8000 Enhancement: Error if configuration options aren't provided as expected · Issue #6403 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

Enhancement: Error if configuration options aren't provided as expected #6403

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
Open
4 tasks done
JoshuaKGoldberg opened this issue Jan 31, 2023 · 9 comments
Open
4 tasks done

Enhancement: Error if configuration options aren't provided as expected #6403

JoshuaKGoldberg opened this issue Jan 31, 2023 · 9 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released enhancement New feature or request package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@JoshuaKGoldberg
Copy link
Member

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

typescript-estree

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Spinning out from #6367 (comment): we have logic in typescript-estree to ensure provided configuration options are the expected types. But we don't show any indication about incorrect types to users who configure us incorrectly.

Fail

{
  parserOptions: {
    project: 123
  }
}

Pass

{
  parserOptions: {
    project: ['tsconfig.eslint.json']
  }
}

Additional Info

Proposal:

  • console.warn in v6?
  • throw an error in v7?
@JoshuaKGoldberg JoshuaKGoldberg added enhancement New feature or request triage Waiting for team members to take a look package: typescript-estree Issues related to @typescript-eslint/typescript-estree labels Jan 31, 2023
@JoshuaKGoldberg
Copy link
Member Author

ping @typescript-eslint/triage-team - any thoughts on this?

@bradzacher
Copy link
Member

We should just throw? Logging is bad because people don't look at them and don't action them.

@JoshuaKGoldberg JoshuaKGoldberg added breaking change This change will require a new major version to be released accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Apr 8, 2023
@JoshuaKGoldberg JoshuaKGoldberg added this to the 6.0.0 milestone Apr 8, 2023
@JoshuaKGoldberg
Copy link
Member Author
JoshuaKGoldberg commented Apr 8, 2023 •
edited
Loading

👍 . Adding to the v6 milestone, but we can always push to v7 if this ends up taking too long.

Edit: v6 already has a lot in it, this isn't critical, and I'm focusing on other things. Moving to v7 v8 just to get it out of my v6 queue. But if someone wants to implement it, please go ahead!

@JoshuaKGoldberg JoshuaKGoldberg modified the milestones: 6.0.0, 7.0.0 May 13, 2023
@Josh-Cena
Copy link
Member

Is this a breaking change? Won't it just cause undefined behavior downstream anyway?

@JoshuaKGoldberg
Copy link
Member Author

Yeah agreed the warning log is a much lighter breaking change than most. But I'd still think it qualifies. If something was previously passing some incorrect or unknown property, this will cause it to newly spit out output. I think that could break tooling if it was expecting things in a particular format.

@bradzacher
Copy link
Member
bradzacher commented Dec 18, 2023 •
edited
Loading

When we log we have to ensure its to stderr. A non-trivial number of consumers assume that eslint output comes in certain formats and stdout logs can break that.

I think that just jumping straight to throwing is fine for us. We could leverage a json schema for automated validation if we want.

@JoshuaKGoldberg JoshuaKGoldberg changed the title Enhancement: Log a warning if configuration options aren't provided as expected types? Enhancement: Error if configuration options aren't provided as expected Jan 5, 2024
@Zamiell
Copy link
Contributor
Zamiell commented Jan 6, 2024 •
edited
Loading

How are the new proposed errors going to be generated for the user? Will they have to add a @type line something like this?

/** @type {import("eslint").Linter.Config} */
const config = {
  /** @type {import("@typescript-eslint/eslint-plugin").Linter.Config.ParserOptions} */
  parserOptions: {
    sourceType: "module",
    ecmaVersion: "latest",
    tsconfigRootDir: REPO_ROOT,
    project: true,
  },

  ignorePatterns: ["**/dist/**"],
};

@bradzacher
Copy link
Member

@Zamiell nope - hard crashes at runtime.
The new flat config work would provide a typed function for people that might help but still not entirely likely to help as we need to support untyped keys for other parsers..

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Jan 7, 2024
Closed
4 tasks
@JoshuaKGoldberg JoshuaKGoldberg removed this from the 8.0.0 milestone Jul 5, 2024
@JoshuaKGoldberg
Copy link
Member Author

We talked internally: v8 has a lot of changes in it and nobody has taken ownership of this non-trivial change. Removing it from the v8 milestone. We still want this, but it'll have to come in a future major version.

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Aug 19, 2024
Closed
4 tasks
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Dec 18, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released enhancement New feature or request package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@JoshuaKGoldberg @Zamiell @bradzacher @Josh-Cena
0