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

Fixing > character in JSX text should have a "fix all in file" #37409

Closed
DanielRosenwasser opened this issue Mar 16, 2020 · 5 comments · Fixed by #37436
Closed

Fixing > character in JSX text should have a "fix all in file" #37409

DanielRosenwasser opened this issue Mar 16, 2020 · 5 comments · Fixed by #37436
Assignees
Labels
Domain: Quick Fixes Editor-provided fixes, often called code actions. Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 16, 2020

We introduced a breaking change in #36636 so that > in JSX text would become an error as per the JSX spec:

Unexpected token. Did you mean `{'>'}` or `>`?

I hit this several times on the new website's src/templates/pages/download.tsx even though the code was just working. Users shouldn't have to fix them one-by-one, so it'd probably be a good idea to auto-fix them all.

@DanielRosenwasser DanielRosenwasser added Domain: Quick Fixes Editor-provided fixes, often called code actions. Experience Enhancement Noncontroversial enhancements labels Mar 16, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.9.1 milestone Mar 16, 2020
@DanielRosenwasser DanielRosenwasser added Good First Issue Well scoped, documented and has the green light Help Wanted You can do this labels Mar 16, 2020
@DanielRosenwasser DanielRosenwasser changed the title Fixing '>' character should have a "fix all in file" Fixing > character in JSX text should have a "fix all in file" Mar 16, 2020
@nmain
Copy link

nmain commented Mar 16, 2020

Could this fix choose {'>'} or {">"} intelligently based on other quote usage in the file?

@orta
Copy link
Contributor

orta commented Mar 16, 2020

I think we already have checks for that sort of thing - so it's likely, yes

@a-tarasyuk
Copy link
Contributor

@orta I tried to add a test/fix for the Fix all QF and got the following error message

No available code fix has the expected id. Fix All is not available if there is only one potentially fixable diagnostic present

The code which I tried to test

let a = <div>>{"foo"}</div>;
let b = <div>>{"foo"}</div>;
let c = <div>>{"foo"}</div>;

Are there any special cases that need to be considered when processing scanner errors in QF services?

I think we already have checks for that sort of thing - so it's likely, yes

You are right, however, it seems that the current service doesn't use quote preferences

@andrewbranch
Copy link
Member

@a-tarasyuk I worked on the fix-all changes that generate that message somewhat recently; seems like there might be a bug there in codeFixProvider. I’m happy to help take a look if you need a hand.

@a-tarasyuk
Copy link
Contributor

@andrewbranch Thanks. I've created PR, with the one change in the codeFixProvider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Quick Fixes Editor-provided fixes, often called code actions. Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants