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

Upgrading from 2.22.0 to 2.23.0 causes parsing to fail miserably #1753

Closed
kaiyoma opened this issue Mar 17, 2020 · 22 comments
Closed

Upgrading from 2.22.0 to 2.23.0 causes parsing to fail miserably #1753

kaiyoma opened this issue Mar 17, 2020 · 22 comments
Labels
awaiting response Issues waiting for a reply from the OP or another party package: parser Issues related to @typescript-eslint/parser

Comments

@kaiyoma
Copy link

kaiyoma commented Mar 17, 2020

I'm not sure what's going on, but when I upgrade to the latest version of the typescript-eslint packages (including 2.24.0), linting fails on my entire codebase at a very low level. With version 2.22.0 or earlier, everything is fine. The errors all look like this:

Parse errors in imported module './Foo': Unexpected token = (49:21)

What happened?

Versions

package version
@typescript-eslint/parser 2.23.0
TypeScript 3.8.3
ESLint 6.8.0
node 12.16.1
npm 6.13.4
@kaiyoma kaiyoma added package: parser Issues related to @typescript-eslint/parser triage Waiting for maintainers to take a look labels Mar 17, 2020
@bradzacher
Copy link
Member

Not a lot I can do with this.

Could you please provide some more information?
what is the output of an eslint --debug run?
What does your config look like?
What does the failing code look like?
etc

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look labels Mar 17, 2020
@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

One thing that could be relevant: we have a codebase that's mixed language (some TypeScript, mostly JavaScript). The parser is struggling with all our JS files.

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

I ran eslint --debug and here was the tail end of it:

2020-03-17T01:30:57.133Z eslint:cli-engine Lint index.js
2020-03-17T01:30:57.134Z eslint:linter Linting code for index.js (pass 1)
2020-03-17T01:30:57.134Z eslint:linter Verify
2020-03-17T01:30:57.134Z eslint:linter With ConfigArray: index.js
2020-03-17T01:30:57.177Z eslint:linter Parsing error: Unexpected token =
SyntaxError: Unexpected token =
    at Espree.raise (...\node_modules\espree\lib\espree.js:190:25)
    at Espree.unexpected (...\node_modules\espree\lib\espree.js:235:18)
    at Espree.pp.expect (...\node_modules\acorn\dist\acorn.js:683:28)
    at Espree.pp$3.parseMethod (...\node_modules\acorn\dist\acorn.js:2624:10)
    at Espree.pp$1.parseClassMethod (...\node_modules\acorn\dist\acorn.js:1387:25)
    at Espree.pp$1.parseClassElement (...\node_modules\acorn\dist\acorn.js:1376:10)
    at Espree.pp$1.parseClass (...\node_modules\acorn\dist\acorn.js:1315:26)
    at Espree.pp$1.parseStatement (...\node_modules\acorn\dist\acorn.js:833:19)
    at Espree.pp$1.parseTopLevel (...\node_modules\acorn\dist\acorn.js:746:23)
    at Espree.parseTopLevel (...\node_modules\espree\lib\espree.js:178:26)
2020-03-17T01:30:57.177Z eslint:linter Generating fixed text for index.js (pass 1)
2020-03-17T01:30:57.177Z eslint:source-code-fixer Applying fixes
2020-03-17T01:30:57.177Z eslint:source-code-fixer shouldFix parameter was false, not attempting fixes
2020-03-17T01:30:57.178Z eslint:file-enumerator Complete iterating files: ["index.js"]
2020-03-17T01:30:57.179Z eslint:cli-engine Linting complete in: 1833ms

index.js
  346:23  error  Parsing error: Unexpected token =

✖ 1 problem (1 error, 0 warnings)

It's getting confused by this kind of notation:

class Foo extends Component {
  handleSomeEvent = (event) => ... // <= the equals sign is confusing the parser
}

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

What I don't quite understand is why typescript-eslint/parser is running for our JS files, since that's not in our config. I printed the config for our JS files and the word "typescript" doesn't appear in it at all. We apply TypeScript plugins/rules/parsers/etc for our TypeScript files only.

@bradzacher
Copy link
Member

(...\node_modules\espree\lib\espree.js:178:26)

Looks like this error is coming from espree (eslint's default parser), not from typescript-eslint?

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

Looks like it, but like I said before, this wasn't a problem with earlier versions of typescript-eslint. Something changed in 2.23.0, because that's the only thing I changed in between the good behavior and the bad behavior.

@bradzacher
Copy link
Member

I don't know what to say - the stack points clearly at another package.
Our stack has zero say over how, when, or by whom a file gets parsed - this is controlled entirely by ESLint.

Upgrading the version of our tooling has no bearing on what ESLint does internally, so there must be something else that's changed.

If you provide more information, maybe I can help you look into this.
Help me to help you.

@bradzacher
Copy link
Member

bradzacher commented Mar 17, 2020

It's getting confused by this kind of notation:

Also - this is because class properties are not supported by espree.
You cannot use them in your JS files without babel-eslint, or our parser.

See repro in eslint's online example

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

You cannot use them in your JS files without babel-eslint, or our parser.

Yup, we use babel-eslint too. I'll keep digging here, but my results are very reproducible. I downgraded typescript-eslint back to version 2.22.0 and now linting works correctly again.

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

Not sure if this is related, but I see a bunch of these messages every time I lint:

`parseForESLint` from parser `.../node_modules/@typescript-eslint/parser/dist/parser.js` is invalid and will just be ignored

@bradzacher
Copy link
Member

That line is from eslint-plugin-import. This plugin does its own parsing outside of the standard ESLint cycle (which is why we recommend not using certain rules from the plugin: https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/FAQ.md#eslint-plugin-import).

The plugin uses the configured parser to parse the dependencies of each and every file (including files in dependencies in node_modules).
Something is not parsing correctly for whatever reason. Maybe due to #1746.

See previous thread on this message: #1333, which you commented on it looks like.

@bradzacher
Copy link
Member

but my results are very reproducible

If you can throw up a repro repo, then I might be able to help you investigate, but it's hard to provide any guidance or help as it stands.

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

Yeah, we've already disabled some the import rules based on your doc. Thanks for writing that up.

It looks like the parser is getting tripped up by lines that look like this:

export info from './moduleInfo';

I think this falls into the "TypeScript 3.8 isn't fully supported" bucket?

@bradzacher
Copy link
Member

I'm assuming that was a typo, as that code is not valid.
The only valid export from statements are:

export * as foo from './foo';
export * from './foo';
export { bar } from './foo';

3.8's export * as foo are supported, as of 2.24.0

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

It wasn't a typo (we've had this code for years) but maybe this is a case of Babel supporting something that TypeScript doesn't. That line is taking the default export from another file and exporting it with a new name.

@bradzacher
Copy link
Member

bradzacher commented Mar 17, 2020

That is not yet valid JS.
It's a stage 1 feature, so still very, very early stages, and hasn't been updated in almost a year: https://github.com/tc39/proposal-export-default-from

The "correct" way to do this with valid ES2020 (and valid TS) code is export { default } from './foo'

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

Hmm, I need the export to be named info though. I tried this:

export { default as info } from './moduleInfo';

But now I'm getting even worse errors:

Error while parsing index.js
Line undefined, column undefined: Cannot read property 'map' of undefined
`parseForESLint` from parser `.../@typescript-eslint/parser/dist/parser.js` is invalid and will just be ignored

@vladimiry

This comment has been minimized.

@bradzacher
Copy link
Member

But now I'm getting even worse errors

Any time you see the "... is invalid and will just be ignored" message, that's coming from eslint-plugin-import.
It might be a case of making the same changes I made in #1333 to log out extra info from the import plugin to help with debugging.

I would consider just using our parser for everything, so that you don't have the inconsistent behaviour of using babel-eslint and typescript-eslint. But that would probably take a little bit of time to ensure you fix up anything non-standard that babel was handling for you (like the export default from).

@kaiyoma
Copy link
Author

kaiyoma commented Mar 17, 2020

That's actually a pretty good idea. I'll dig into that today and see what I find.

@kaiyoma
Copy link
Author

kaiyoma commented Mar 18, 2020

After digging all day, I feel pretty confident that the root issue is that we have a mixed-language codebase, where our JS files also have (dormant) Flow types. That's why we have both @typescript-eslint and babel-eslint in our codebase. I don't like it, but that's how it is.

After some clean installs and clean package upgrades, I got the number of linting errors down to something very reasonable. All the errors were related to Flow rules being incorrectly applied to TS files. Once I updated our eslint config to not do that, everything passed. Sorry for the trouble and thanks a ton for your suggestions!

@kaiyoma kaiyoma closed this as completed Mar 18, 2020
@bradzacher
Copy link
Member

no problem, glad we were able to get it all sorted!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP or another party package: parser Issues related to @typescript-eslint/parser
Projects
None yet
Development

No branches or pull requests

3 participants