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
feat: Add support for TypeScript config files #117
base: main
Are you sure you want to change the base?
Conversation
Worth linking the prior art in this area: #50 |
I think it makes sense and is low cost to support this in a runtime that supports ts natively. Like deno and bun, we can just import them. |
@bradzacher Yes thank you!!! |
|
||
The primary motivation for adding support for TypeScript configuration files to ESLint is to enhance the developer experience and accommodate the evolving JavaScript ecosystem. As TypeScript's popularity continues to grow, more projects are adopting TypeScript not only for their source code but also for their configuration files. This shift is driven by TypeScript's ability to provide compile-time type checks and IntelliSense. By supporting `eslint.config.ts`, `eslint.config.mts`, and `eslint.config.cts`, ESLint will offer first-class support to TypeScript users, allowing them to leverage these benefits directly within their ESLint configuration. | ||
|
||
## Detailed Design |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the feature interact with the CLI option --config
for specifying a config file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested it, so far it seems to work pretty well actually, especially with v9. I'm probably going to write a bunch of tests as well to see if there are any edge cases but so far so good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to explain what the behavior is when specifying TS or non-TS config files of varying file extensions through that option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't behave any differently, same as before. You can do eslint . --config=eslint.config.ts
or eslint . -c eslint.config.ts
and they just work. Same as with a eslint.config.js
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add that into the RFC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it to the open questions, is that fine?
Which version of TypeScript can the config file be written in? With what tsconfig settings? Does this mean eslint would need to depend on typescript, causing it to be installed for non-TS users? if not, then would eslint be able to consume typescript in pnpm and yarn pnp in order to transpile the eslint config? |
- [Reference to the question](eslint#117 (comment))
It's possible to declare an implicit, optional peer dependency in a way that both yarn and pnpm will respect. {
"peerDependenciesMeta": {
"typescript": {
"optional": true
}
},
} You don't even need to declare an explicit peer dependency with this config as it implicitly declares a dep on For context this is how the |
We are going to be using
As far as I know,
I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together. I left some answers to the open questions. Also, we like to have a detailed design right in the RFC explaining where you'd make changes and how Jiti would be used.
|
||
The primary motivation for adding support for TypeScript configuration files to ESLint is to enhance the developer experience and accommodate the evolving JavaScript ecosystem. As TypeScript's popularity continues to grow, more projects are adopting TypeScript not only for their source code but also for their configuration files. This shift is driven by TypeScript's ability to provide compile-time type checks and IntelliSense. By supporting `eslint.config.ts`, `eslint.config.mts`, and `eslint.config.cts`, ESLint will offer first-class support to TypeScript users, allowing them to leverage these benefits directly within their ESLint configuration. | ||
|
||
## Detailed Design |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add that into the RFC?
Thanks for this RFC @aryaemami59! Could it be that Jiti doesn't transpile top-level // eslint.config.mts
const { default: globals } = await import('globals');
export default {
languageOptions: {
globals: globals.node
},
rules: {
'no-undef': 'error'
}
};
Would it be possible to add a note to the RFC to state this limitation - or if I'm overlooking something, explain how to make top-level It would be good to have this noted either way to make sure that we don't forget to add unit tests during the implementation. |
@fasttime Yeah I mentioned it in this comment but I can go ahead and include it in the RFC as well. To my knowledge top level awaits are pretty much the only limitation |
Yes, the lack of support for top-level |
This section should also include prior art, such as whether similar | ||
projects have already implemented a similar feature. | ||
--> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a community project: https://github.com/antfu/eslint-ts-patch to support it. would like to hear the author :) / @antfu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It uses jiti to support ts file. Which already mentioned #117 (comment) that it doesn't currently support top-level await.
I would personally recommend using https://github.com/egoist/bundle-require instead which is more robust and will respect tsconfig.json. The downside is that it would introduce esbuild
into the dependency. If the install size is a concern, I guess we could have an optional package like @eslint/config-loader-ts
that only requires when users use ts version of config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like @eslint/config-inspector uses bundle-require as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the inspector uses that because we need to know the dependencies of the config to do automatic reloads. Supporting TS was a free side-effect.
Even if ESLint doesn't need information on dependencies, I think it's still a good way to support TS. Vite uses the same approach to load vite.config.ts
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does bundle-require
write a temp file to disk like vite does?
These temp files are a major source of problems with vite, see vitejs/vite#9470.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I see TLA as a language spec requirement, and I don't think whether to support it should be up to whether we can imagine use-cases.
But if we want evaluate usage, here are examples of TLA in configs (Vite, Rollup, Next, Jest, esbuild, etc):
https://github.com/search?q=path%3A%2F%5C.config%5C.%28js%7Cts%29%24%2F+%2F%5Eawait+%2F&type=code
tsx is mentioned as a non-viable alternative because it uses custom loaders. Does that also hold when
tsImport()
is used?
tsImport()
is isolated and won't add TypeScript support to the rest of the environment.
But just to be clear, I'm only sharing it as an option. Personally, I would like to have popular compilers supported as optional peer dependencies to accommodate different preferences and environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I see TLA as a language spec requirement, and I don't think whether to support it should be up to whether we can imagine use-cases.
But if we want evaluate usage, here are examples of TLA in configs (Vite, Rollup, Next, Jest, esbuild, etc): https://github.com/search?q=path%3A%2F%5C.config%5C.%28js%7Cts%29%24%2F+%2F%5Eawait+%2F&type=code
Interesting. There are also ESLint configs that use TLA: https://github.com/search?q=path%3A%2Feslint%5C.config%5C.m%3Fjs%24%2F+%2F%5Econst+.%2B+%3D+await+%2F&type=code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a workaround for TLA available:
export default (async () => {
await whatever();
}());
We await
the export of the config file. We ran into this when discussing CommonJS support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jiti won't recognize TLA not only in the config file itself, but also in imported modules, because they are all treated as CommonJS. I did a quick test using a patched version of ESLint with the changes from #18134 and with this config:
// eslint.config.ts
export { default } from './recommended.mjs';
// recommended.mjs
import { readFile } from 'node:fs/promises';
const json = await readFile('package.json', 'utf-8');
const { name } = JSON.parse(json);
export default [{ name, rules: { 'no-undef': 'error' } }];
When running eslint
, I got an error as expected:
Oops! Something went wrong! :(
ESLint: 9.2.0
ReferenceError: await is not defined
at ../project/recommended.mjs:4:14
at evalModule (../project/node_modules/eslint/node_modules/jiti/dist/jiti.js:1:256443)
at jiti (../project/node_modules/eslint/node_modules/jiti/dist/jiti.js:1:254371)
at ../project/eslint.config.ts:2:43
at evalModule (../project/node_modules/eslint/node_modules/jiti/dist/jiti.js:1:256443)
at jiti (../project/node_modules/eslint/node_modules/jiti/dist/jiti.js:1:254371)
at loadFlatConfigFile (../project/node_modules/eslint/lib/eslint/eslint.js:335:24)
at async calculateConfigArray (../project/node_modules/eslint/lib/eslint/eslint.js:421:28)
at async ESLint.lintFiles (../project/node_modules/eslint/lib/eslint/eslint.js:840:25)
at async Object.execute (../project/node_modules/eslint/lib/cli.js:500:23)
at async main (../project/node_modules/eslint/bin/eslint.js:153:22)
The workaround of using a dynamic import didn't help either, and resulted in the same error:
// eslint.config.ts
export default (async () =>
(await import('./recommended.mjs')).default
)();
This means that if we decide to use Jiti, plugin developers should be advised to avoid TLA in their shared configs (including rules and transitive dependencies), or else those configs will not work for users who have a eslint.config.ts
in their project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added support for all the ts loaders we mentioned in eslint-ts-patch
and listed their trade-offs, where you can give them a try today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the jiti approach makes sense to me. It's fairly clean and even though it doesn't support top-level await, I think we can live with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm conflicted about this requirement. While it may burden non-TypeScript users with an unnecessary dependency, TypeScript users are a significant part of the ESLint community. I'll defer to the team's judgment on this one.
|
||
3. Using [TypeScript's `transpileModule()`](https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#a-simple-transform-function) to parse TypeScript configuration files. This approach proved to be problematic because it requires a significant amount of overhead and is not suitable for this purpose. | ||
|
||
## Open Questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using the ts configs, does it check the ts type, or is it just erasure typings?
projects have already implemented a similar feature. | ||
--> | ||
|
||
While developing this feature, we considered the following alternatives: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A feasible alternative is leaving it to the community, e.g. eslint-ts-patch
. ts users just need to install it, and it just works.
I agree. As a user, I wouldn't want a TS compiler installed in non-TS projects, and I wouldn't want a second TS compiler installed if I already have one installed. As a reference in |
@privatenumber I think that's good feedback. We could definitely specify TS-related stuff as an optional dependency and ask folks to manually install. |
Seems like a good call to require opt-in to these dependencies. Some runtimes like bun and deno support typescript out of the box so won't need this compiler. I think it's also likely that Node.js will gain native typescript support eventually, see nodejs/node#43816. |
That's true. In those runtimes the |
This is consistent with what I mentioned in #117 (comment). We can check the runtime and import it directly, otherwise keep looking for the ts loader |
Summary
Add support for TypeScript config files (
eslint.config.ts
,eslint.config.mts
,eslint.config.cts
).Related Issues
This PR is related to this RFC.