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

[utils] [new] parse: support flat config #2714

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

DMartens
Copy link
Contributor

Flat ESLint no longer supports context.parserPath which is required for utils/parse.
This PR uses context.languageOptions.parse which has the loaded parser as an alternative.
Some notes:

  • Backwards compatible as it only checks if context.parserPath is unset
  • Currently it does check if parse or parseForESLint in context.languageOptions.parser is a function (I do not if "constructed" context objects are officially supported)
  • I added a test to the rule to no-unused-modules for testing with flat eslint rule tester

utils/parse.js Show resolved Hide resolved
utils/parse.js Outdated
Comment on lines 124 to 125
const isFlat = context.languageOptions &&
typeof context.languageOptions.parser === 'object'; // May be a string
Copy link
Member

Choose a reason for hiding this comment

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

will this test still work with the approach we've taken to have a config shared between flat and legacy? Meaning, the legacy config object still has languageOptions, I just mark it non-enumerable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand it is that has nothing to do with the read config file, as context.languageOptions.parserOptions is also set for legacy configs (relevant code). But if the linter runs in "flat" mode it no longer sets the context.parserPath, context.parserOptions ... .

Copy link
Member

Choose a reason for hiding this comment

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

ok - it's not that languageOptions is set that IDs it as the flat config, it's that it's set and has an object parser property?

Are we sure it can't be null, only a string or an object?

Copy link
Contributor Author

@DMartens DMartens Feb 10, 2023

Choose a reason for hiding this comment

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

No as I understand it in the current eslint both properties are set: context.parserOptions and context.languageOptions.parserOptions (same with other properties of languageOptions). But in "flat" mode the top-level properties are not set.

To the type of languageOptions.parser:

  • null: I created an extra test case for this case.
  • string: is from @types/eslint
  • object: is already an object for the current eslint

Copy link
Member

Choose a reason for hiding this comment

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

Let's also add runtime checks that throw errors if the types aren't what we expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added runtime checks that context.languageOptions.parser either has a function parse or parseForESLint

@DMartens
Copy link
Contributor Author

Added support for context.languageOptions.parserOptions (which is preferred over context.parserOptions).
Maybe that was the cause of the confusion but I forgot to add this because as I said context.parserOptions is not set in "flat" mode.

@ljharb ljharb force-pushed the parse-use-language-options-parser branch from 4d70c98 to 9ee5904 Compare April 11, 2023 23:09
@ljharb ljharb force-pushed the parse-use-language-options-parser branch from 9ee5904 to dc596a2 Compare April 11, 2023 23:27
@ljharb
Copy link
Member

ljharb commented Apr 11, 2023

Did some tweaks for style, but also additional ones to handle that FlatRuleTester isn't available in eslint < 8.

@ljharb ljharb changed the title feat: support context.languageOptions.parser for utils/parse [utils] [new] parse: support flat config Apr 12, 2023
@ljharb ljharb merged commit dc596a2 into import-js:main Apr 12, 2023
@DMartens DMartens deleted the parse-use-language-options-parser branch April 12, 2023 17:31
@azat-io
Copy link

azat-io commented Apr 26, 2023

I have a lot of errors like this: error Parse errors in imported module 'vite': parserPath or languageOptions.parser is required! (undefined:undefined) import/namespace.
I use flat config, can I fix it?

@ljharb
Copy link
Member

ljharb commented Apr 27, 2023

@azat-io meaning, with eslint-module-utils 2.7 you don't see those, but with 2.8 you do?

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

Successfully merging this pull request may close these issues.

None yet

3 participants