Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw syntax error for } and > in JSX text #36636

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Throw syntax error for } and > in JSX text #36636

merged 2 commits into from
Feb 11, 2020

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: throw syntax error for } and > in JSX text.
5 participants