Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: Improve config error messages#17385

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mdjermanovic merged 4 commits intomainfromissue17370
Jul 19, 2023
Merged

feat: Improve config error messages#17385

mdjermanovic merged 4 commits intomainfromissue17370
Jul 19, 2023

Conversation

nzakas
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Includes some keys to catch known eslintrc keys when they appear in flat config. This throws a specific error that the ESLint CLI can then output a more helpful message about.

refs#17370

Is there anything you'd like reviewers to focus on?

Did I miss any top-level keys?

(Note: we can probably do more to do things like catch the different inplugins format, just wanted to get the ball rolling.)

Includes some keys to catch known eslintrc keys when they appear in flatconfig. This throws a specific error that the ESLint CLI can then outputa more helpful message about.fixes#17370
@nzakasnzakas requested a review froma team as acode ownerJuly 18, 2023 15:54
@eslint-github-boteslint-github-botbot added the featureThis change adds a new feature to ESLint labelJul 18, 2023
@netlify
Copy link

netlifybot commentedJul 18, 2023
edited
Loading

Deploy Preview fordocs-eslint canceled.

NameLink
🔨 Latest commit8dccf61
🔍 Latest deploy loghttps://app.netlify.com/sites/docs-eslint/deploys/64b7fef4df8f2a00087ee301

Copy link
Contributor

@matwilkomatwilko left a comment

Choose a reason for hiding this comment

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

Handful of thoughts, can't see any top-level keys you missed.


/* eslint consistent-return: 0 -- no default case */

module.exports = function({ key }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this look slightly nicer as an object rather than a big switch? A little less syntax noise to my eyes 😄

e.g.

const keyErrors = {    env: `A config object is using the "env" key, which is not supported in flat config system.Flat config uses "languageOptions.globals" to define global variables for your files.Please see the following page for information on how to convert your config object into the correct format:https://eslint.org/docs/latest/use/configure/migration-guide#configuring-language-options`,    extends: `A config object is using the "extends" key, which is not supported in flat config system.Instead of "extends", you can include config objects that you'd like to extend from directly in the flat config array.Please see the following page for more information:https://eslint.org/docs/latest/use/configure/configuration-files-new#using-predefined-configurations`,...};module.exports = key => keyErrors[key].trim();

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

ooh nice, I like it.

* @param {string} key The eslintrc key to create a schema for.
* @returns {ObjectPropertySchema} The schema.
*/
function createEslintrcSchema(key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Might suggest a more explicit name thancreateEslintrcSchema - the current name could be confused asadding support for eslintrc config keys at a glance? PerhapsrejectEslintrcKeySchema or similar to spell it out?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Fair point, making it explicit that this is throwing an error makes sense.

//-----------------------------------------------------------------------------
// Full schema
//-----------------------------------------------------------------------------

exports.flatConfigSchema = {

// eslintrc-style keys that should warn
env: createEslintrcSchema("env"),
Copy link
Contributor

@matwilkomatwilkoJul 18, 2023
edited
Loading

Choose a reason for hiding this comment

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

Even though we're unlikely to add any further keys, I might suggest a slight refactor here to remove the duplication of the key name, and not take up half the space in this schema with keys that we don't actually care about in the end:

functionrejectEslintrcKeySchema(key){return{merge:"replace",validate(){thrownewIncompatibleKeyError(key);}};}functionrejectEslintrcKeySchemas(...keys){returnObject.fromEntries(keys.map(key=>[key,rejectEslintrcKeySchema(key)]));}exports.flatConfigSchema={    ...rejectEslintrcKeySchemas("env","extends","globals","ignorePatterns","noInlineConfig","overrides","parser","parserOptions","reportUnusedDisableDirectives","root"),settings: ...linterOptions: ......

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Another nice update, thanks.

@mdjermanovicmdjermanovic added coreRelates to ESLint's core APIs and features acceptedThere is consensus among the team that this change meets the criteria for inclusion labelsJul 18, 2023
@mdjermanovic
Copy link
Member

Did I miss any top-level keys?

No, all eslintrc top-level keys that don't exist as flat config top-level keys are included.

@mdjermanovic
Copy link
Member

This test should be updated:

it("throw an error when env is included in config",()=>{
assert.throws(()=>{
ruleTester.run("no-eval",require("../../fixtures/testers/rule-tester/no-eval"),{
valid:[
{code:"Eval(foo)",env:["es6"]}
],
invalid:[]
});
},/Unexpectedkey"env"found./u);
});

nzakasand others added3 commitsJuly 19, 2023 11:06
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Copy link
Member

@mdjermanovicmdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovicmdjermanovic merged commitcf03104 intomainJul 19, 2023
@mdjermanovicmdjermanovic deleted the issue17370 branchJuly 19, 2023 21:35
bmish added a commit to bmish/eslint that referenced this pull requestJul 23, 2023
* main: (101 commits)  docs: Migrating `eslint-env` configuration comments (eslint#17390)  Merge pull request from GHSA-qwh7-v8hg-w8rh  feat: adds option for allowing empty object patterns as parameter (eslint#17365)  fix: FlatESLint#getRulesMetaForResults shouldn't throw on unknown rules (eslint#17393)  docs: fix Ignoring Files section in config migration guide (eslint#17392)  docs: Update README  feat: Improve config error messages (eslint#17385)  fix: Update no-loop-func to not overlap with no-undef (eslint#17358)  docs: Update README  docs: Update README  8.45.0  Build: changelog update for 8.45.0  chore: package.json update for @eslint/js release  docs: add playground links to correct and incorrect code blocks (eslint#17306)  docs: Expand rule option schema docs (eslint#17198)  docs: Config Migration Guide (eslint#17230)  docs: Update README  fix: Fix suggestion message in `no-useless-escape` (eslint#17339)  docs: Update README  chore: update eslint-config-eslint exports (eslint#17336)  ...
@eslint-github-boteslint-github-botbot locked and limited conversation to collaboratorsJan 16, 2024
@eslint-github-boteslint-github-botbot added the archived due to ageThis issue has been archived; please open a new issue for any further discussion labelJan 16, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@matwilkomatwilkomatwilko left review comments

@mdjermanovicmdjermanovicmdjermanovic approved these changes

Assignees
No one assigned
Labels
acceptedThere is consensus among the team that this change meets the criteria for inclusionarchived due to ageThis issue has been archived; please open a new issue for any further discussioncoreRelates to ESLint's core APIs and featuresfeatureThis change adds a new feature to ESLint
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@nzakas@mdjermanovic@matwilko

[8]ページ先頭

©2009-2025 Movatter.jp