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

Change Request: Provide config key when config validation fails #17960

Closed
1 task done
martypdx opened this issue Jan 5, 2024 · 11 comments · Fixed by #17980
Closed
1 task done

Change Request: Provide config key when config validation fails #17960

martypdx opened this issue Jan 5, 2024 · 11 comments · Fixed by #17980
Assignees
Labels
core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@martypdx
Copy link

martypdx commented Jan 5, 2024

ESLint version

8.55.0

What problem do you want to solve?

When the config file fails validation there is very little information to go on. In connection with #17959, using "error" for reportUnusedDisableDirectives causes the load to fail with error below which has no information on which config caused the error or call stack entry with line numbers.

% npm run lint -- src/www/main.js

> vite-poc@0.0.0 lint
> eslint src/www/main.js


Oops! Something went wrong! :(

ESLint: 8.55.0

TypeError: Expected a Boolean.
    at Object.boolean [as validate] (/Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/object-schema/src/validation-strategy.js:36:19)
    at ObjectSchema.validate (/Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/object-schema/src/object-schema.js:285:35)
    at Object.validate (/Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/object-schema/src/object-schema.js:180:32)
    at ObjectSchema.validate (/Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/object-schema/src/object-schema.js:285:35)
    at /Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/object-schema/src/object-schema.js:239:18
    at Array.reduce (<anonymous>)
    at ObjectSchema.merge (/Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/object-schema/src/object-schema.js:237:24)
    at /Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/config-array/api.js:966:42
    at Array.reduce (<anonymous>)
    at FlatConfigArray.getConfig (/Users/marty/gitmllc/vite-poc/node_modules/@humanwhocodes/config-array/api.js:965:39)

What do you think is the correct solution?

Report:

  • the configuration setting that failed validation
  • the found value for the setting that failed validation
  • line number would be nice

Participation

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

Additional comments

Is there a type definition for new config file? I could see implementing this as type validation as well.

@martypdx martypdx added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint labels Jan 5, 2024
@martypdx martypdx changed the title Change Request: Provide config key and value when config validation fails Change Request: Provide config key when config validation fails Jan 5, 2024
@martypdx
Copy link
Author

martypdx commented Jan 6, 2024

I upgraded to 8.56 and the the new message about the failed value is 🤩

TypeError: Expected one of: "error", "warn", "off", 0, 1, 2, or a boolean.

Will there be more additions or changes to the validation errors?

Are there plans for a type def file for the eslint.config.js?

@martypdx martypdx closed this as completed Jan 6, 2024
@bmish
Copy link
Sponsor Member

bmish commented Jan 6, 2024

I do agree we need to at least indicate what property is involved.

@bmish
Copy link
Sponsor Member

bmish commented Jan 6, 2024

Why was this closed?

@fasttime
Copy link
Member

fasttime commented Jan 6, 2024

I also think that it would be good to include the name of the option in the error message. Reopening to get more feedback from the team @eslint/eslint-team.

@fasttime fasttime reopened this Jan 6, 2024
@martypdx
Copy link
Author

martypdx commented Jan 6, 2024

I waffled on it and closed because it seemed like the messages were improving and maybe already on the roadmap. But agree if opening is the thing to do to keep on the radar.

@nzakas
Copy link
Member

nzakas commented Jan 9, 2024

@martypdx that's very strange. The flat config does typically tell you not just the key but the path to the key when there's a validation error. Can you create a Stackblitz that reproduces the issue you're seeing? I'd like to see it in action to understand what is happening here because this is unexpected.

@martypdx
Copy link
Author

martypdx commented Jan 9, 2024

@nzakas here is a Stackblitz using these dep versions:

  "dependencies": {
    "@eslint/js": "^8.56.0",
    "eslint": "^8.56.0",
    "globals": "^13.24.0"
  }

If you run npm run lint from the terminal, you get details about the value, but no property info or info that a failed config is the issue.

image

This is same error you would see in OUTPUT ESlint tab of VSCode console

@nzakas
Copy link
Member

nzakas commented Jan 10, 2024

Awesome ,thanks so much. I'll take a look.

@nzakas nzakas self-assigned this Jan 10, 2024
@nzakas
Copy link
Member

nzakas commented Jan 10, 2024

I was able to reproduce this locally. Interestingly, the unit tests are passing with key information, so somehow we are skipping over the logic that adds the key information to the message in this case.

@nzakas
Copy link
Member

nzakas commented Jan 10, 2024

Okay, so it turns out the error is being produced with the key information, but our error reporting functionality is swallowing all of the details.

@nzakas
Copy link
Member

nzakas commented Jan 10, 2024

This was actually caused by a change I made to object-schema. This was apparently a little too clever because it was interfering with the normal processing of errors, including modifying the stack as it went.

nzakas added a commit that referenced this issue Jan 11, 2024
* fix: Ensure config keys are printed for config errors

fixes #17960

* Remove unnecessary test
snitin315 pushed a commit that referenced this issue Feb 1, 2024
* fix: Ensure config keys are printed for config errors

fixes #17960

* Remove unnecessary test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants