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

ESLint: Workaround failure to parse files with template literals #15241

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Apr 1, 2020

Changes proposed in this Pull Request:

  • Add a workaround for ESLint failing to parse files containing template literals, throwing the following error: TypeError: Cannot read property 'range' of null.

This error has been reported several other times in the various ESLint repos:

babel/babel-eslint#530 (I think babel-eslint is the origin of the problem, but don't quote me on this)
eslint/eslint#9872
jsx-eslint/eslint-plugin-react#2321
...and so on.

I've tried a few workarounds, but this is both the one that actually fixed the issue, and also the simplest one.

The template-curly-spacing rule enforces our traditional spacing inside template literal variables (e.g. ${ foo }).
Not only it's just a styling rule, but it's also one that is automatically fixed by Prettier.

Note: I've intentionally left the original rule in the config file. The workaround overrides it, and this way it would be easier to just remove the workaround whenever babel-eslint fixes this issue.

Testing instructions:

  • Before applying this PR, try opening a file containing template literals (example).
  • Open your editor and, while monitoring the ESLint log, try causing a parse error. For example, in VSCode, I did this:
    • Open the ESLint output log (typically by clicking on "ESLint" in the bottom toolbar).
    • The log should report this parsing error Cannot read property 'range' of null Occurred while linting /path/to/file.js:123 whenever the file is parsed (in my case, it parses on type, so it happens very often; if you have set ESLint to only parse on save, please save the file every each step 😄). Note that the reported line is where the first template literal is (in the example it would be line 42).
    • In the file, try adding a space in the middle of a variable name.
      E.g. defaultAttributes (at line 10 of the example) -> defa ultAttributes.
    • Note that only the second half of the variable name is marked as error with the red wiggly line.
    • Delete the space (aka: remove the intentional error).
    • Now the entire variable is marked as error with the red line, and there's no way to "fix" it, except closing and reopening the file.
  • Apply the PR. You might want to close and reopen VSCode just to be sure.
    • Notice that there are no parsing errors in the ESLint log.
    • Try causing a parse error again by adding a space in the variable name. Make sure the red line appears on the second half of the name.
    • Delete the space.
    • Now the error is cleared, and the ESLint log is clean.

Proposed changelog entry for your changes:

  • N/A

@Copons Copons added [Status] Needs Review To request a review from Crew. Label will be renamed soon. Build labels Apr 1, 2020
@Copons Copons requested review from getdave, jeryj, WunderBart and a team April 1, 2020 18:23
@Copons Copons self-assigned this Apr 1, 2020
@jetpackbot
Copy link

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against d02eb54

Copy link
Contributor

@jeryj jeryj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this change to the branch I was having trouble with, and this fixes it so I can use template literals again! 🙌

Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked for me. Let's get this in to keep dev moving.

@kraftbj kraftbj added this to the 8.5 milestone Apr 1, 2020
@kraftbj kraftbj merged commit 76eac07 into master Apr 1, 2020
@kraftbj kraftbj deleted the fix/eslint-template-literals branch April 1, 2020 22:07
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants