Skip to content

Conversation

bradzacher
Copy link
Contributor

Fixes #36341

This is my first PR for typescript, so please LMK if there's a better way to do this.

@msftclas
Copy link

msftclas commented Feb 5, 2020

CLA assistant check
All CLA requirements met.

@orta orta self-assigned this Feb 5, 2020
@orta
Copy link
Contributor

orta commented Feb 5, 2020

This is great, I don't have any code recommendations (given the simplicity of the implementation) and the tests look good. I went to check babels implementation details only to find my way back to the linked issue.

My only question is do you want to add a codefix for these too?

@bradzacher
Copy link
Contributor Author

only to find my way back to the linked issue

Yeah sorry, I raised issues in acorn, babel and typescript together (and PR'd it into babel) 😄

My only question is do you want to add a codefix for these too?

Sure, I'd love to!
Just triple checking my understanding of the codebase is right - is this the correct place to learn about, and add code fixes?
https://github.com/microsoft/TypeScript/tree/master/src/services/codefixes

@a-tarasyuk
Copy link
Contributor

is this the correct place to learn about, and add code fixes?

Yes, it is the right place (:

@bradzacher
Copy link
Contributor Author

Took me a minute to figure everything out, but I got there in the end.
LMK if the codefix implementation looks good.

@orta
Copy link
Contributor

orta commented Feb 11, 2020

This looks perfect, thanks @bradzacher 👍

@orta orta merged commit 348c4dd into microsoft:master Feb 11, 2020
@bradzacher bradzacher deleted the 36341-dangling-jsx branch February 11, 2020 16:45
@alloy
Copy link
Member

alloy commented Feb 12, 2020

Nice one, immediately caught an error in a DT test case 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants
close