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: Flat config - allow nested arrays by automatically flattening a flat config array #18040

Closed
1 task
bradzacher opened this issue Jan 27, 2024 · 7 comments
Labels
core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@bradzacher
Copy link
Contributor

bradzacher commented Jan 27, 2024

ESLint version

v9.0.0-alpha.2

What problem do you want to solve?

ESLint currently requires you to spread flat config arrays from shared configs to create a 2d array.
This creates a decent sized devx problem for flat configs:

the user needs to be aware of whether or not a shared config is an array or an object - the former must be spread and the latter must not. Failing to spread an array causes a cryptic linter crash:

Error: Unexpected key "0" found.
    at ObjectSchema.validate (/node_modules/@humanwhocodes/object-schema/src/object-schema.js:270:23)
    at /node_modules/@humanwhocodes/object-schema/src/object-schema.js:239:18
    at Array.reduce (<anonymous>)
    at ObjectSchema.merge (/node_modules/@humanwhocodes/object-schema/src/object-schema.js:237:24)
    at /node_modules/@humanwhocodes/config-array/api.js:935:42
    at Array.reduce (<anonymous>)
    at FlatConfigArray.getConfig (/node_modules/@humanwhocodes/config-array/api.js:934:39)
    at FlatConfigArray.isFileIgnored (/node_modules/@humanwhocodes/config-array/api.js:962:15)
    at /node_modules/eslint/lib/eslint/eslint-helpers.js:514:38
    at Array.forEach (<anonymous>)

it also creates a bit of ugliness in configs where there's a mish-mash of ...'s eg:

export default [
  eslint.configs.recommended,
  ...compat.config(eslintPluginPlugin.configs.recommended),
  ...tseslint.configs.strictTypeChecked,
  ...tseslint.configs.stylisticTypeChecked,
  jsdocPlugin.configs['flat/recommended-typescript-error'],
];

What do you think is the correct solution?

I'd like ESLint to automatically flatten the config array from an n-dimensional array to a 2d one.

This would mean that I would no longer need to care about how a shared config exports - I could just consume everything in exactly the same way. It would standardise the appearance, eg the above snippet would become:

export default [
  eslint.configs.recommended,
  compat.config(eslintPluginPlugin.configs.recommended),
  tseslint.configs.strictTypeChecked,
  tseslint.configs.stylisticTypeChecked,
  jsdocPlugin.configs['flat/recommended-typescript-error'],
];

Failing that - at least the flat config validator should have explicit detection for arrays so that it can provide a better error message. EG "Unexpected array - did you forget to spread?"

Participation

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

Additional comments

No response

@bradzacher bradzacher added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint labels Jan 27, 2024
@fasttime
Copy link
Member

fasttime commented Jan 27, 2024

Just a tip. When not sure, I use Array.prototype.concat. It works regardless of whether the params are arrays or plain objects, e.g.:

export default [].concat(
  eslint.configs.recommended,
  compat.config(eslintPluginPlugin.configs.recommended),
  tseslint.configs.strictTypeChecked,
  tseslint.configs.stylisticTypeChecked,
  jsdocPlugin.configs['flat/recommended-typescript-error'],
);

Not less ugly, just less thinking.

@fasttime
Copy link
Member

I agree that the error message could be improved. @eslint/eslint-team

@bradzacher
Copy link
Contributor Author

@fasttime yeah it's entirely possible to solve this from the user side - it would just be nice to not have to so everyone can be standardised.

Eg from the perspective of a plugin + shared config author - I'd much rather document to users saying "add tseslint.configs.strictTypeChecked to your config" rather than "add ...tseslint.configs.strictTypeChecked or if you use a flattening util use tseslint.configs.strictTypeChecked"

@bradzacher
Copy link
Contributor Author

Also one thing I didn't mention above - consider sheared configs which compose other shared configs.

Right now the shared configs have to also spread their things, then the user has to spread their shared config. The result is that you lose the ability to relate the original config to errors.

For example an error saying "the config at element 4" is meaningless when everything is spread becuase the indices no longer line up.

But if we allowed for nested arrays then eslint could actually validate before it flattens and so it could have some semblance of relation. Eg you could say "the config at element [4][3][2]` and it would make sense (as long as the user chose not to do any spreading).

@nzakas
Copy link
Member

nzakas commented Jan 29, 2024

While flattening arrays was part of the original RFC, we decided against flattening arrays inside of the config array because it basically creates a situation where you never quite know what you are inserting into the config array. For instance, you could have an array whose elements are both object and arrays, and that makes it impossible to do things like apply an imported config to a subset of files.

The solution to the debugging problem is to identify configs and then output that information in the error message. I think it's a much better user experience to say something like "Config 'typescript-eslint/recommended' has an error" than asking people to search through multiple nested arrays.

@bradzacher
Copy link
Contributor Author

and that makes it impossible to do things like apply an imported config to a subset of files.

It's not impossible -- [config].flatten().map(...) would do exactly what you want with only a dozen extra characters!

But I do see your point in that it makes things a little more deterministic/predictable for the user.

Though you still do kind of have this problem because there's no required standard for shared configs. They could export an array, an object or a function that returns either - you don't know!

There is a potential pain point in that a user might write ...config only to find that it was an object not an array -- oops! It's a minor inconvenience as ofc it would be an immediate runtime error for the lint run.

@nzakas
Copy link
Member

nzakas commented Feb 15, 2024

We're not planning on doing this, so closing. We will, of course, keep an eye on conventions that develop in the ecosystem and reserve the right to revisit the idea of flattening arrays in the future.

@nzakas nzakas closed this as not planned Won't fix, can't repro, duplicate, stale Feb 15, 2024
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

No branches or pull requests

3 participants