-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 TS config files #18134
base: main
Are you sure you want to change the base?
Conversation
Hi @aryaemami59!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
|
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
eslint.config.ts
, eslint.config.mts
, eslint.config.cts
)eslint.config.ts
, eslint.config.mts
, eslint.config.cts
)
Hi @aryaemami59!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
eslint.config.ts
, eslint.config.mts
, eslint.config.cts
)
Wouldn't it be better to use Jiti for this? I think it's perfect for this, it's used in Nuxt and TailwindCSS to load configuration from Typescript files. It is much smaller and faster than Typescript. |
the first-class ts config support has been discussed in #12078 eslint/rfcs#50, and seems we didn't come to an agreement to accept it. |
it should be possible to use ts config in the current config. The easiest way I can think of is to use NODE_OPTIONS=--loader=tsx eslint --config=eslint.config.ts xxx |
@cany748 I had to research it to make sure it doesn't create import side effects. But it seems like you were right, it does what I wanted |
That doesn't work you'll get: Error: tsx must be loaded with --import instead of --loader And if you use NODE_OPTIONS=--import=tsx eslint --config=eslint.config.ts you'll get a Error: ESLint configuration in --config is invalid:
- Property "" is the wrong type |
I mean flat config (which is the default in eslint v9). If you are using eslint v8, the default is eslintrc, and you need to specify |
That still won't work with Out of curiosity what is your opinion in the current solution this PR provides? |
Thanks for working on this, @aryaemami59! In order to move forward with this pull request, we will need some unit tests to verify that TypeScript config files are being loaded as expected. My suggestion would be to start looking at these tests and add similar tests to check the behavior with packages that have config files with |
Thank you so much, I appreciate it, this was the reason why I haven't written any tests yet, thankfully the problem is resolved thanks to @antfu. I will start writing a bunch of tests now! |
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
- This is not only done to reduce some potential redundancy down the line, but to ensure that TS config files are able to handle `type` imports as that is something most people are likely going to do.
; branch 'main' of https://github.com/eslint/eslint into eslint.config.ts
Note: `isRunningInBun` and `isRunningInDeno` are functions to make treeshaking for consuming libraries easier.
- This is done for mainly 2 reasons: 1. We don't know how many runtime environments are going to support loading TS files natively in the future, so this saves us having to check for every single one. 2. This also ensures that we give the user the option of passing their own TS loader of choice through `NODE_OPTIONS` in CLI for example: `NODE_OPTIONS=--import=tsx/esm`, without ESLint getting in the way and potentially causing conflicts between multiple loaders.
I believe the first approach is more predictable. In the second case, |
@aryaemami59 @antfu you're both working on PRs for this feature. It would be helpful if you could collaborate to see which approach would be best. |
@nzakas I'd love nothing more |
ci failing has been fixed in the main branch, can you please rebase it, thanks! |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[X] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
This PR:
eslint.config.ts
,eslint.config.mts
,eslint.config.cts
)jiti
as a dependency.jiti
based on whether the config file's extension ends withts
.I went through a lot of different possible ways of implementing it, and this by far seems to be the simplest way of doing it without changing too much of the internals of the package or adding too many dependencies.
Is there anything you'd like reviewers to focus on?