fix(#199): Auto-tag react fragments

[roycrippen4]

Auto-closes react fragment tags in js, jsx, and tsx files.

[PriceHiller]

Assigning myself as a reminder to look at this in a bit when I'm available.

[roycrippen4]

@PriceHiller No problem. I'm writing a few more tests to ensure it's working correctly.

I know there's a possible issue in .js files if there isn't any jsx specific syntax to match against. If you just open a new .js file and the first jsx syntax is a react fragment it won't work. I don't think this is necessarily a bad thing as it'll be almost impossible to handle such a case.

This shouldn't be an issue in .jsx or .tsx files since the file-type is checked.

[roycrippen4]

I found a couple issues when running the tests. I'm working on a fix now.

[PriceHiller]

If the tests fail due to incompatibility between this and a less important feature (like automatically adding a closing tag for an opening tag without a closing partner) and it proves to be too difficult to resolve the compatilibty between them just list out the compatability issues and we can decide if it's worth losing something for this.

Fragment closing seems to be quite popular in terms of requested features so I'm ok with losing lesser known/used features for it.

If that turns out to be the case though, I'll be marking this as the start of autotag 1.0.0 and yanking some things that have been marked as deprecated for a while.

[roycrippen4]

It should all be compatible. I just made an error with boolean logic and need one extra check function and (hopefully) everything should work. There are a couple of tests failing on the main branch. I'm making my measure of success as getting the same tests that fail on main to match the PR's tests.

[roycrippen4]

I'm pretty sure those are the same tests that are already failing on the main branch. Are those tests related to #190 and #196?

[PriceHiller]

I'm pretty sure those are the same tests that are already failing on the main branch. Are those tests related to #190 and #196?

Yeah, those tests appear to be failing due to an update upstream in nvim-treesitter for the js/ts parsers.

Probably not your fault based on what I'm seeing locally. Entirely separate issue (though it is a problem).

[roycrippen4]

We should make an issue to get them fixed so at least it's documented.

[PriceHiller]

We should make an issue to get them fixed so at least it's documented.

Already done 😉 (#205)

[PriceHiller]

Ping me when you're good to go over here (or convert this PR to a draft and then promote it when you're ready) and I'll take a look

[roycrippen4]

It's good to go

[roycrippen4]

I mentioned it in #199, but this PR doesn't fix #185.

[PriceHiller]

Overall, it looks great!

Can you run make format for me and push those changes? You'll need stylua installed for the formatting.

I really need to add a CI check for formattingn now that I've noticed this 😅

[PriceHiller]

Also, for clarification's sake, do I need to close #185 once this is merged?

[roycrippen4]

No, #185 will get it's own PR. I probably won't get it done today, but in the near future.

[PriceHiller]

No, #185 will get it's own PR. I probably won't get it done today, but in the near future.

Gotcha, no rush on it. You're doing god's work right now. I'll leave that issue alone then.

Just hit it with a make format and I think we're good to go.

[PriceHiller]

Awesome, thanks again @roycrippen4 -- great work!

[roycrippen4]

Happy to help!

[abilkhan024]

Is there a way to opt out from this feature, or potentially change current behavior. Right now it causes problems when i want to pass type arguments by adding </>. For example:
typing: useState<>
results: useState<></>
I didn't find work around for that, pretty sure that was not intentional behaviour.