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

Running infinitely: ${f({ a: ${1} })}; '' ? ${2} : <em /> #1523

Closed
scastiel opened this issue Nov 8, 2017 · 8 comments
Closed

Running infinitely: ${f({ a: ${1} })}; '' ? ${2} : <em /> #1523

scastiel opened this issue Nov 8, 2017 · 8 comments

Comments

@scastiel
Copy link

scastiel commented Nov 8, 2017

On a big file I have, ESLint is going into an infinite loop while linting. It doesn't throw any error, just never terminates. I reproduced the problem in a very short example.

You can reproduce the problem with this minimalistic repo I created.

Here is the file I want to run ESLint on:

`${f({ a: `${1}` })}`; '' ? `${2}` : <em />

I know it looks pretty weird, but the syntax is totally correct (with ES2015 and JSX). I simplified it as much as possible; if I change something (replace a template string with a plain string for instance) the problem doesn't reproduce.

Here is the package.json:

{
  "devDependencies": {
    "babel-eslint": "^8.0.2",
    "eslint": "^4.10.0",
    "eslint-plugin-react": "^6.10.0"
  }
}

And the .eslintrc.js:

module.exports = {
  parser: 'babel-eslint',
  plugins: [
    'react',
  ],
  rules: {
    'react/jsx-indent': 'error'
  }
}

And finally what happens when I run ESLint:

$ npx eslint --debug index.js
  eslint:cli Running on files +0ms
  eslint:glob-util Creating list of files to process. +0ms
  eslint:ignored-paths Looking for ignore file in /Users/sebastien/Downloads/test-eslint +0ms
  eslint:ignored-paths Could not find ignore file in cwd +0ms
  eslint:cli-engine Processing /Users/sebastien/Downloads/test-eslint/index.js +0ms
  eslint:cli-engine Linting /Users/sebastien/Downloads/test-eslint/index.js +0ms
  eslint:config Constructing config file hierarchy for /Users/sebastien/Downloads/test-eslint +0ms
  eslint:config Using .eslintrc and package.json files +0ms
  eslint:config Loading /Users/sebastien/Downloads/test-eslint/.eslintrc.js +1ms
  eslint:config-file Loading JS config file: /Users/sebastien/Downloads/test-eslint/.eslintrc.js +0ms
  eslint:config Using /Users/sebastien/Downloads/test-eslint/.eslintrc.js +85ms
  eslint:config-ops Using config from partial cache +0ms
  eslint:config-ops Apply environment settings to config +1ms
  eslint:linter Linting code for /Users/sebastien/Downloads/test-eslint/index.js (pass 1) +0ms
@scastiel
Copy link
Author

scastiel commented Nov 8, 2017

Maybe this piece of code is easier to understand to reproduce the issue:

const s = `${'' + { a: `${1}` }}`;
const t = '' ? `${2}` : <em />;

The problem happens when following is involved:

  • a first template string;
  • in this template string, an object (with {} syntax);
  • still in this template string, another template string inside the referenced object;
  • an expression using ternary operator;
  • in this expression, some JSX in one of the operands;
  • still in this expression, a template string in another operand.

The problem does not happen if:

  • the JSX is removed or put outside the ternary expression;
  • the template string or the JSX is in the first operand of the ternary expression, nor if they're both in the same operand.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2017

Thanks for the update; the first example was just two throwaway statements :-)

This seems like a (totally obscure, strange, edge, but valid) bug. Thanks for the report and the repro repo!

@jomasti
Copy link
Contributor

jomasti commented Nov 9, 2017

After a quick investigation, it looks to be an issue with using babel-eslint as the parser. Using the example and the default parser, there is no problem. When using babel-eslint as the parser instead, it hangs.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2017

Interesting - it'd be useful to figure out if it's a bug in babel-eslint (which is possible), or a bug in this plugin when using babel-eslint (which is also possible).

@scastiel
Copy link
Author

scastiel commented Nov 9, 2017

Some investigation later…

The infinite loop comes from here:
https://github.com/yannickcr/eslint-plugin-react/blob/f36b0fa995a2bbe84ab28b035fc6aacbfcb11734/lib/rules/jsx-indent.js#L232-L234

It seems that in some cases, getTokenBefore returns the same token as before. This is confirmed when I add this into the loop:

let prevPrevToken = null;
do {
  prevToken = sourceCode.getTokenBefore(prevToken);
  if (prevToken === prevPrevToken) {
    throw new Error('Woops, entering into an infinite loop!');
  }
  prevPrevToken = prevToken;
} while (prevToken.type === 'Punctuator');

I added some console.log to see the content of sourceCode:

console.log(sourceCode.ast.tokens.map(t => t.value).join('·'));

This is the result for non working code (entering infinite loop) (line return added for clarity):

const·s·=·`${·''·+·{·a·:·`·template·${·1·}·template·`·}·}·template·`·;
const·t·=·''·?·`·template·${·2·}·template·`·:·<·em·/·>·;

And the result for working code (not entering infinite loop, I just inverted the first and second operand of the ternary expression:

const·s·=·`${·''·+·{·a·:·`·template·${·1·}·template·`·}·}·template·`·;
const·t·=·`·template·${·2·}·template·`·?·''·:·<·em·/·>·;

As you can see there doesn’t seem to be a lot of differences, and the tokens seem okay (I don't know what template stands for, but it's here in working example too).

So maybe the issue is in the getTokenBefore method provided by ESLint… I guess no matter what, there's no reason that the returned token is the same as the one provided as parameter. What do you think? Should I investigate more in this direction and eventually submit an issue in ESLint project?

@ljharb
Copy link
Member

ljharb commented Nov 9, 2017

If it’s only with babel-eslint, then i think you’d need to file an issue there.

@scastiel
Copy link
Author

After a lot of investigation on eslint-plugin-react, eslint, babel-eslint, babylon, etc., I confirmed the problem is in babel-eslint parser. I created the issue and submitted the PR for the fix.

I think this issue can now be closed. Thanks a lot for your help!

@ljharb
Copy link
Member

ljharb commented Nov 10, 2017

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants