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.

Sign up for GitHub

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

Jump to bottom

Change Request: explicitly handle undefined being passed as a config value #18259

Closed
1 task done
G-Rath opened this issue Apr 2, 2024 · 4 comments · Fixed by #18357
Closed
1 task done

Change Request: explicitly handle undefined being passed as a config value #18259

G-Rath opened this issue Apr 2, 2024 · 4 comments · Fixed by #18357
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@G-Rath
Copy link
Contributor

G-Rath commented Apr 2, 2024 •
edited

ESLint version

next

What problem do you want to solve?

Currently flat config does not have any special handling for undefined being passed in causing an "accessed property on undefined" error:

Oops! Something went wrong! :(

ESLint: 8.56.0

TypeError: Cannot read properties of undefined (reading 'ignores')
    at FlatConfigArray.get ignores [as ignores] ([redacted]/node_modules/@humanwhocodes/config-array/api.js:648:15)
    at FlatConfigArray.isDirectoryIgnored ([redacted]/node_modules/@humanwhocodes/config-array/api.js:1012:10)
    at FlatConfigArray.getConfig ([redacted]/node_modules/@humanwhocodes/config-array/api.js:806:12)
    at FlatConfigArray.isFileIgnored ([redacted]/node_modules/@humanwhocodes/config-array/api.js:962:15)
    at [redacted]/node_modules/eslint/lib/eslint/eslint-helpers.js:312:49
    at Array.reduce (<anonymous>)
    at entryFilter ([redacted]/node_modules/eslint/lib/eslint/eslint-helpers.js:299:28)
    at Object.isAppliedFilter ([redacted]/node_modules/@nodelib/fs.walk/out/readers/common.js:12:31)
    at AsyncReader._handleEntry ([redacted]/node_modules/@nodelib/fs.walk/out/readers/async.js:86:20)
    at [redacted]/node_modules/@nodelib/fs.walk/out/readers/async.js:65:22

This is confusing because undefined is commonly expected if you try to use a config that doesn't exist but the error message looks more like a potential bug in ESLint:

const jestPlugin = require('eslint-plugin-jest');

module.exports = [
  {
    // enable jest rules on test files
    files: ['test/**'],
    ...jestPlugin.configs['flat/recommended'],
  },
  // explodes, as we're removing this in our next major!
  jestPlugin.configs['flat/snapshots'],
];

This is a real-world example of where I hit this as I was testing for eslint-plugin-jest's upcoming major in which we're removing the flat/snapshots config.

What do you think is the correct solution?

I'd like ESLint to explicitly check for undefined and raise an error along the lines of "you've passed undefined as a config - this usually happens when you're trying to reference a config that doesn't exist"

Participation

  • I am willing to submit a pull request for this change.

Additional comments

Born from #18094 & #18094 (comment)
@G-Rath G-Rath added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint labels Apr 2, 2024
@mdjermanovic
Copy link
Member

It makes sense to me to improve error reporting in this case, as referencing a config name that doesn't exist in a plugin might be a common mistake.

@eslint/eslint-team thoughts?

@nzakas
Copy link
Member

nzakas commented Apr 2, 2024

@G-Rath can you please update your description with a specific example that results in the error?

@G-Rath
Copy link
Contributor Author

G-Rath commented Apr 2, 2024

@nzakas done (I was in the middle of doing that when you posted 😅)

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Apr 3, 2024
@nzakas
Copy link
Member

nzakas commented Apr 3, 2024

Thanks.

Yeah, I agree we should have a nicer error message in this case.

G-Rath added a commit to G-Rath/eslint that referenced this issue Apr 5, 2024
@G-Rath
feat: show an explicit message when undefined is included as a config
af9507e 
Fixes eslint#18259
@G-Rath G-Rath mentioned this issue Apr 5, 2024
Closed
1 task
@nzakas nzakas mentioned this issue Apr 15, 2024
Merged
nzakas added a commit that referenced this issue Apr 17, 2024
@nzakas
feat: Provide helpful error message for nullish configs
4deb68e 
fixes #18259
nzakas added a commit that referenced this issue Apr 17, 2024
@nzakas
feat: Provide helpful error message for nullish configs
d9baea2 
fixes #18259
@nzakas nzakas mentioned this issue Apr 17, 2024
Merged
1 task
fasttime added a commit that referenced this issue Apr 19, 2024
@nzakas @fasttime
feat: Provide helpful error message for nullish configs (#18357)
03068f1 
* feat: Provide helpful error message for nullish configs

fixes #18259

* Update lib/config/flat-config-array.js

Co-authored-by: Francesco Trotta <[email protected]>

* Update lib/config/flat-config-array.js

Co-authored-by: Francesco Trotta <[email protected]>

---------

Co-authored-by: Francesco Trotta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Triage
Status: Complete
Milestone
No milestone
3 participants
@nzakas @G-Rath @mdjermanovic