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: Support arbitrary config loaders #17211

Closed
1 task done
matwilko opened this issue May 23, 2023 · 3 comments
Closed
1 task done

Change Request: Support arbitrary config loaders #17211

matwilko opened this issue May 23, 2023 · 3 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@matwilko
Copy link
Contributor

matwilko commented May 23, 2023

ESLint version

8.40.0

What problem do you want to solve?

Now that flat config is in place, and is going to become the default in v9 (as I understand it), I'd like to discuss re-opening supporting non-JS config files again, a la #12078 and eslint/rfcs/pull/50.

From reading through the various comments made at the time, I think the main objection was pulling external support into ESLint to handle non-JS files, effectively being a "kingmaker" by making anything else first-class - and a more general system that doesn't favour any particular language or tooling was the preferred way to go, but proposals stalled with flat config going on at the time.

I'm not suggesting specifically supporting Typescript config files, or any other language, but I think we can capitalise on the fact that flat config has now simplified how configuration is handled.

(full disclosure, supporting typescript configuration files is my main goal here, so that I can get type checked configurations and rule options, but I'm trying to show due consideration to the issues raised previously 😄)

What do you think is the correct solution?

From my understanding of flat config in lib/eslint/flat-eslint.js, configuration is now only loaded from a single entry-point by loading a single file as a module via import() (eslint.config.js or --config), and there's no longer any complex file traversal to resolve additional configs (except to traverse upwards to find the nearest config file.)

Given this, I think it's now relatively easy to support other config loaders.

My solution would be to widen what flat config will load from - since it already import()'s a JS config file you specify, it seems easy enough to loosen that to "load any specified module", and expect that module to resolve to a flat config array for processing. This allows for arbitrary loading semantics, but ESLint itself maintains the simple API of "load a flat config array from X".

For example, Typescript (or whatever language) support could be plugged in by creating an npm package that looks for eslint.config.ts and does the necessary Typescript compilation/execution and resolves as its export default a flat config array. ESLint would then just import() that package and process as normal the flat config array it gets handed.

This also eliminates all the potential problems of peerDependencies and magic behaviour, at least from ESLint's point of view - the external package would be responsible for handling dependencies and any magic behaviour is opt-in, since you need to tell ESLint to load the package.

The current logic of traversing for eslint.config.js and loading it can be encapsulated separately from the generic loading logic, and it would just be the default config module to be loaded if no custom one is specified.

Another possible scenario could be the creation of some enforced shared config within an organization - publish the enforced shared config as an npm package and tell ESLint to load it with no possibility of locally overriding (because there's no local config file at all!)

I'm not sure yet how you'd specify this alternative module in a way you can commit to git (perhaps an eslintConfigModule property in package.json?)

I'm aware that you could emulate this today by creating eslint.config.js and putting some equivalent to export default import('some-module'), but that's very untidy if you then want to have a proper eslint.config.xx right next to it?

I know this sort of change will likely require an RFC, and I'm willing to put one together if there's interest. If what I'm proposing isn't entirely clear, I'm also happy to put together a proof of concept to demonstrate.

Participation

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

Additional comments

No response

@matwilko matwilko added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint labels May 23, 2023
@mdjermanovic
Copy link
Member

Thanks for the proposal!

I'm not sure yet how you'd specify this alternative module in a way you can commit to git (perhaps an eslintConfigModule property in package.json?)

I'm aware that you could emulate this today by creating eslint.config.js and putting some equivalent to export default import('some-module'), but that's very untidy if you then want to have a proper eslint.config.xx right next to it?

I personally think the benefit of avoiding an extra eslint.config.js file that would simply invoke the loader, while still having to configure eslint somewhere to use the loader doesn't justify this functionality. Curious what other team members think about this.

@matwilko
Copy link
Contributor Author

I personally think the benefit of avoiding an extra eslint.config.js file that would simply invoke the loader, while still having to configure eslint somewhere to use the loader

Sure, that did occur to me, which is why I don't have a concrete proposal for how to do that. It's along the same lines as using an alternative filename to eslint.config.js at the moment, you always need to specify -c <filename>, and I assume that will usually get enshrined in a script in package.json.

I don't want to derail the whole proposal on this point, so if we split that train of thought out (i.e. can we specify an alternative filename/module to eslint.module.js in a way that ESLint will pick up without needing to provide it on the command line?), would a proposal where -c XXX is expanded to allow any module name rather than requiring a file path be an acceptable starting point?

That way, we can set a script to e.g. { "eslint": "npm run eslint -c eslint-configloader-typescript ..." }, and it can go away and do whatever custom logic it wants as proposed above.

@nzakas
Copy link
Member

nzakas commented May 26, 2023

I'm not a fan of this idea at the moment. There's a lot of use case extrapolation in your description that I don't think we currently have any data to support.

I'd much rather wait until flat config is released and is the format that most people are using and then look at what people are actually doing to see if there is a way to streamline the config experience further. The benefit of the eslint.config.js approach is that people can implement config loading however they want inside of that file and we can learn from what people are doing.

So, closing this for now as I think it's a distraction from finishing up the flat config release and working through the existing issues.

@nzakas nzakas closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2023
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 23, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion 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