fix(typescript-eslint): gracefully handle invalid flat config objects in `tseslint.config` #10576

abrahamguo · 2024-12-31T07:23:43Z

PR Checklist

Addresses an existing open issue: fixes Bug: tseslint.config hides user-friendly error messages from ESLint #10549
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Gracefully handle invalid flat config objects in tseslint.config.

typescript-eslint · 2024-12-31T07:23:46Z

Thanks for the PR, @abrahamguo!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2024-12-31T07:24:04Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`373e325`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/679cd59bc8456300089377f7
😎 Deploy Preview	https://deploy-preview-10576--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 92 (🔴 down 6 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

nx-cloud · 2024-12-31T07:24:33Z

View your CI Pipeline Execution ↗ for commit 373e325.

Command	Status	Duration	Result
`nx test visitor-keys --coverage=false`	✅ Succeeded	<1s	View ↗
`nx run types:build`	✅ Succeeded	<1s	View ↗
`nx run-many --target=build --exclude website --...`	✅ Succeeded	16s	View ↗
`nx run-many --target=clean`	✅ Succeeded	32s	View ↗
`nx test typescript-eslint --coverage=false`	✅ Succeeded	9s	View ↗
`nx test scope-manager --coverage=false`	✅ Succeeded	<1s	View ↗
`nx test eslint-plugin --coverage=false`	✅ Succeeded	<1s	View ↗
`nx test type-utils --coverage=false`	✅ Succeeded	<1s	View ↗
`Additional runs (24)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2025-01-31 14:14:05 UTC

abrahamguo · 2024-12-31T07:42:23Z

packages/typescript-eslint/src/config-helper.ts

+  the arguments passed to this function at runtime. If any argument is an unexpected type, we should not cause a
+  `TypeError`, and instead, we should silently return the unexpected argument unchanged, because ESLint will later
+   validate the types. */
+  const flattened: unknown[] =


this is the key of this PR: treat the array internally as unknown[]

codecov · 2024-12-31T07:49:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.27%. Comparing base (a94004c) to head (373e325).
Report is 142 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10576   +/-   ##
=======================================
  Coverage   87.26%   87.27%           
=======================================
  Files         450      450           
  Lines       15715    15722    +7     
  Branches     4601     4604    +3     
=======================================
+ Hits        13714    13721    +7     
  Misses       1645     1645           
  Partials      356      356

Flag	Coverage Δ
unittest	`87.27% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/typescript-eslint/src/config-helper.ts	`100.00% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

abrahamguo · 2024-12-31T07:54:20Z

packages/typescript-eslint/src/config-helper.ts

+      ...config
+    }: ObjectWithExtends &
+      Partial<Record<'files' | 'ignores' | 'name', unknown>> =
+      configWithExtends as ObjectWithExtends;


Question: I am following the approach here that any properties that get sent to ESLint (files | ignores | name) are typed as unknown, so that we assume nothing, and ESLint does the validation. What about the extends property, which is unique to tseslint, and is not sent to ESLint?

A. Do we assume that extends always obeys its TS type (i.e. it will always be an array)?
As I mentioned in #10549, there could be several scenarios where a user is not type-checking their config file. This would cause a user-unfriendly error if, for example, they passed a string to extend:`"flat" is not a method on string", and it is inconsistent with all other settings in the config file, which have user-friendly error messages generated by ESLint.

B. Do we pass along invalid values for extends to ESLint?
This doesn't make much sense, because then ESLint would display an error unexpected property 'extends' on user-defined config object, and ESLint never expects the property extends.

C. Do we silently ignore/remove invalid values for extends?
It's inconsistent that this one property in the config file fails silently, whereas any other property with an invalid value would cause an ESLint-generated error message.

D. Do we manufacture an error message consistent with ESLint's error message?
The user would see Key "extends": Expected value to be an array, which would be a consistent user experience as compared to any other property in the config file. Downside is obviously that we have to keep our error message in sync with ESLint's.

Question: I am following the approach here that any properties that get sent to ESLint (files | ignores | name) are typed as unknown, so that we assume nothing, and ESLint does the validation. What about the extends property, which is unique to tseslint, and is not sent to ESLint?

A. Do we assume that extends always obeys its TS type (i.e. it will always be an array)? As I mentioned in #10549, there could be several scenarios where a user is not type-checking their config file. This would cause a user-unfriendly error if, for example, they passed a string to extend:`"flat" is not a method on string", and it is inconsistent with all other settings in the config file, which have user-friendly error messages generated by ESLint.

No; we should not assume that the extends property has valid shape

B. Do we pass along invalid values for extends to ESLint? This doesn't make much sense, because then ESLint would display an error unexpected property 'extends' on user-defined config object, and ESLint never expects the property extends.

Agreed - no.

C. Do we silently ignore/remove invalid values for extends? It's inconsistent that this one property in the config file fails silently, whereas any other property with an invalid value would cause an ESLint-generated error message.

D. Do we manufacture an error message consistent with ESLint's error message? The user would see Key "extends": Expected value to be an array, which would be a consistent user experience as compared to any other property in the config file. Downside is obviously that we have to keep our error message in sync with ESLint's.

Yes, I think we should report a user-friendly error for invalid values. I don't think it's particularly essential that it remain identical to eslint core's error message wording (in other words, no need to stress about minor drift), but I do think it's essential that we conspicuously identify it as coming from the tseslint.config() function, not from eslint core.

@kirkwaiblinger I've updated both the existing error message about invalid values inside extends, as well as the new error message about invalid extends itself, to be consistent and to match ESLint. What do you think?

abrahamguo · 2024-12-31T07:55:38Z

packages/typescript-eslint/src/config-helper.ts

+        config.name != null
+          ? `, named "${
+              // eslint-disable-next-line @typescript-eslint/no-base-to-string,@typescript-eslint/restrict-template-expressions
+              config.name


Even though config.name is unknown, it should still be fine to concatenate it

packages/typescript-eslint/src/config-helper.ts

abrahamguo · 2024-12-31T07:59:33Z

Ready for review/discussion!

abrahamguo · 2024-12-31T08:01:52Z

packages/typescript-eslint/src/config-helper.ts

          ...(name && { name }),
        };
      }),
-      config,
+      // `config` could be any object, but we'll assume it's a `Config` object for TS purposes.
+      config as Config,


Inside this function, config should be unknown, but outside this function, config should be Config even if it's really an invalid value

abrahamguo · 2024-12-31T08:02:55Z

packages/typescript-eslint/src/config-helper.ts

  return flattened.flatMap((configWithExtends, configIndex) => {
-    const { extends: extendsArr, ...config } = configWithExtends;
-    if (extendsArr == null || extendsArr.length === 0) {


null check was updated to use in, to make TS happy.

length check was unnecessary.

packages/typescript-eslint/src/config-helper.ts

…onfig-helper-unknown

abrahamguo · 2025-01-03T13:53:16Z

packages/typescript-eslint/src/config-helper.ts

+    const extendsError = (message: string) =>
+      new TypeError(
+        `tseslint.config(): Config ${
+          nameIsString ? `"${name}"` : '(unnamed)'


this matches ESLint's logic, where it only uses the name if it is a string.

…onfig-helper-unknown

kirkwaiblinger · 2025-01-16T20:13:59Z

packages/typescript-eslint/tests/config-helper.test.ts

@@ -247,4 +245,16 @@ describe('config helper', () => {
      { rules: { rule: 'error' } },
    ]);
  });
+
+  it.each([undefined, null])(


(testing) maybe add also like one string and/or number or something 🤷‍♂️

Suggested change

it.each([undefined, null])(

it.each([undefined, null, "invalid config object", 42])(

kirkwaiblinger · 2025-01-16T20:34:38Z

packages/typescript-eslint/tests/config-helper.test.ts

@@ -81,8 +81,7 @@ describe('config helper', () => {
        },
      ),
    ).toThrow(
-      'Your config at index 1, named "my-config-2", contains undefined ' +
-        'extensions at the following indices: 0, 2',
+      'tseslint.config(): Config "my-config-2": Key "extends": Expected array to only contain objects at user-defined index 1.',


This is just a regression, right?

We have lost the index of my-config-2 in the error message

extension, at index 1, is an object.

No, not a regression. I was trying to match ESLint's error message format exactly.

The index of my-config-2 (1) was moved to the end of the message to be consistent with ESLint.
However, I can revert this.

kirkwaiblinger · 2025-01-16T20:35:09Z

packages/typescript-eslint/tests/config-helper.test.ts

@@ -107,8 +106,7 @@ describe('config helper', () => {
        },
      ),
    ).toThrow(
-      'Your config at index 1 (anonymous) contains undefined extensions at ' +
-        'the following indices: 0, 2',
+      'tseslint.config(): Config (unnamed): Key "extends": Expected array to only contain objects at user-defined index 1.',


Why have we changed the naming of (anonymous) to (unnamed)? Revert?

I did this to more closely match ESLint's error convention, but I'm happy to revert it.

packages/typescript-eslint/src/config-helper.ts

kirkwaiblinger · 2025-01-16T20:40:45Z

packages/typescript-eslint/src/config-helper.ts

-    const { extends: extendsArr, ...config } = configWithExtends;
-    if (extendsArr == null || extendsArr.length === 0) {
-      return config;
+    const isObject = (


Please move this and any other inner functions that don't capture variables to be top-level named variables. This should look like

function isObject(value: unknown): value is object { // eslint-disable-next-line @typescript-eslint/internal/eqeq-nullish, eqeqeq return typeof value === 'object' && value !== null; }

Technically a correct type-predicate here would also have typeof value === 'function' since const o: object = () => {} is valid, but probably not worth thinking about.

kirkwaiblinger · 2025-01-16T20:44:45Z

packages/typescript-eslint/src/config-helper.ts

-      Infinity,
-    ) as ConfigWithExtends[];
-
-    const undefinedExtensions = extendsArrFlattened.reduce<number[]>(


Why not just change this to be invalidExtensions and report more errors than just the indices?

packages/typescript-eslint/src/config-helper.ts

…onfig-helper-unknown

JoshuaKGoldberg · 2025-03-03T13:17:56Z

👋 @abrahamguo just checking in, did you mean to re-request review on this? Anything we can help with?

JoshuaKGoldberg · 2025-04-07T13:19:13Z

Closing this PR as it's been stale for a couple of months without activity. Feel free to reopen @abrahamguo if you have time - but no worries if not! If anybody wants to drive it forward, please do post your own PR. If you use this as a start, please add a co-author attribution at the end. Cheers! ✨

add failing tests

cd10811

abrahamguo added 4 commits December 31, 2024 01:32

use unknown type

25175a1

extract constant

9345ab7

fix comment

df40d93

change assertion to be safer

93b8c8f

abrahamguo commented Dec 31, 2024

View reviewed changes

packages/typescript-eslint/src/config-helper.ts Show resolved Hide resolved

remove unnecessary check

e1e7436

abrahamguo commented Dec 31, 2024

View reviewed changes

kirkwaiblinger reviewed Jan 2, 2025

View reviewed changes

packages/typescript-eslint/src/config-helper.ts Outdated Show resolved Hide resolved

kirkwaiblinger added the awaiting response Issues waiting for a reply from the OP or another party label Jan 2, 2025

abrahamguo added 3 commits January 2, 2025 22:55

Merge branch 'main' of github.com:abrahamguo/typescript-eslint into c…

2c12396

…onfig-helper-unknown

remove assumptions about extends

4f58a0f

tweak messages

0002341

abrahamguo commented Jan 3, 2025

View reviewed changes

abrahamguo added 3 commits January 3, 2025 07:53

small tweak

fc4c692

Merge branch 'main' of github.com:abrahamguo/typescript-eslint into c…

587c8de

…onfig-helper-unknown

fix test message

5918b80

JoshuaKGoldberg requested a review from kirkwaiblinger January 7, 2025 12:57

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 7, 2025

kirkwaiblinger requested changes Jan 16, 2025

View reviewed changes

kirkwaiblinger added the awaiting response Issues waiting for a reply from the OP or another party label Jan 24, 2025

abrahamguo added 6 commits January 25, 2025 07:32

Merge branch 'main' of github.com:abrahamguo/typescript-eslint into c…

c913b4b

…onfig-helper-unknown

Merge branch 'main' of github.com:abrahamguo/typescript-eslint into c…

aa4f897

…onfig-helper-unknown

some small tweaks

a654208

add additional test

9c2a031

tweak messages

1127362

report all indices

373e325

JoshuaKGoldberg closed this Apr 7, 2025

JoshuaKGoldberg added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Apr 7, 2025

kirkwaiblinger mentioned this pull request Apr 14, 2025

fix(typescript-eslint): gracefully handle invalid flat config objects in config helper #11070

Merged

3 tasks

github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(typescript-eslint): gracefully handle invalid flat config objects in `tseslint.config` #10576

fix(typescript-eslint): gracefully handle invalid flat config objects in `tseslint.config` #10576

abrahamguo commented Dec 31, 2024

typescript-eslint bot commented Dec 31, 2024

netlify bot commented Dec 31, 2024 •

edited

Loading

nx-cloud bot commented Dec 31, 2024 •

edited

Loading

abrahamguo Dec 31, 2024 •

edited

Loading

codecov bot commented Dec 31, 2024 •

edited

Loading

abrahamguo Dec 31, 2024

kirkwaiblinger Jan 2, 2025

abrahamguo Jan 3, 2025

abrahamguo Dec 31, 2024

abrahamguo commented Dec 31, 2024

abrahamguo Dec 31, 2024

abrahamguo Dec 31, 2024

abrahamguo Jan 3, 2025

kirkwaiblinger Jan 16, 2025

kirkwaiblinger Jan 16, 2025

abrahamguo Jan 31, 2025

kirkwaiblinger Jan 16, 2025

abrahamguo Jan 31, 2025

kirkwaiblinger Jan 16, 2025

kirkwaiblinger Jan 16, 2025

JoshuaKGoldberg commented Mar 3, 2025

JoshuaKGoldberg commented Apr 7, 2025

	it.each([undefined, null])(
	it.each([undefined, null, "invalid config object", 42])(

fix(typescript-eslint): gracefully handle invalid flat config objects in tseslint.config #10576

fix(typescript-eslint): gracefully handle invalid flat config objects in tseslint.config #10576

Conversation

abrahamguo commented Dec 31, 2024

PR Checklist

Overview

typescript-eslint bot commented Dec 31, 2024

netlify bot commented Dec 31, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Dec 31, 2024 • edited Loading

abrahamguo Dec 31, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Dec 31, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abrahamguo commented Dec 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshuaKGoldberg commented Mar 3, 2025

JoshuaKGoldberg commented Apr 7, 2025

fix(typescript-eslint): gracefully handle invalid flat config objects in `tseslint.config` #10576

fix(typescript-eslint): gracefully handle invalid flat config objects in `tseslint.config` #10576

netlify bot commented Dec 31, 2024 •

edited

Loading

nx-cloud bot commented Dec 31, 2024 •

edited

Loading

abrahamguo Dec 31, 2024 •

edited

Loading

codecov bot commented Dec 31, 2024 •

edited

Loading