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.

Sign up for GitHub

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

Jump to bottom

feat: Add support for TypeScript config files #117

Merged
merged 23 commits into from
May 24, 2024
+341 −0
Merged

feat: Add support for TypeScript config files #117

Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
5e5fc4c
feat: Add support for TypeScript config files
aryaemami59 Mar 9, 2024
50676fa
Update start date in `README.md`
aryaemami59 Mar 9, 2024
9ab30b5
Add RFC PR
aryaemami59 Mar 12, 2024
c0c0714
Link prior RFC and issue regarding support for `.eslintrc.ts` files.
aryaemami59 Mar 12, 2024
247393f
Add `External References` section
aryaemami59 Mar 12, 2024
d13d556
Add alternatives considered for parsing TypeScript configuration files
aryaemami59 Mar 12, 2024
5e68795
Add links to TypeScript documentation in `External References` section
aryaemami59 Mar 12, 2024
24dac1d
Add some basic examples
aryaemami59 Mar 12, 2024
3863834
Convert `Related Discussions` to a list.
aryaemami59 Mar 12, 2024
d5e16db
Inform about the possibility of type checking in `.js` config files
aryaemami59 Mar 12, 2024
0e09c10
Add question related to `defineConfig` to `Open Questions`
aryaemami59 Mar 17, 2024
3c37a94
Include Docusaurus in the list of frameworks that are using `jiti`
aryaemami59 Mar 17, 2024
e0da6c3
Add new open question about how the feature interacts with `--config`
aryaemami59 Mar 28, 2024
24b0893
Add answer for question about caching
aryaemami59 Mar 28, 2024
ef591fc
Add answer about interoperability question
aryaemami59 Mar 28, 2024
7dfea39
Add how and where we use `jiti` in the Detailed Design` section
aryaemami59 Mar 28, 2024
de7e87a
Add missing section related to `importedConfigFileModificationTime`
aryaemami59 Apr 2, 2024
fb612ed
Enable `esmResolve` for `jiti`
aryaemami59 Apr 2, 2024
f60a15c
Update `Detailed Design` section
aryaemami59 Apr 4, 2024
fef64ec
Add disclaimer about `jiti` not supporting the top-level `await` syntax
aryaemami59 Apr 4, 2024
6e8440f
Remove section about `type` field in `package.json`
aryaemami59 May 14, 2024
2a2c9f8
Slightly modify `Detailed Design` section
aryaemami59 May 14, 2024
263c467
Slightly modify `Summary` section
aryaemami59 May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions designs/2024-support-ts-config-files/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
- Repo: eslint/eslint
- Start Date: 2023-03-09
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
- RFC PR: (leave this empty, to be filled in later)
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
- Authors: [Arya Emami](https://github.com/aryaemami59)

# Add Support for TypeScript Config Files

## Summary

Add support for TypeScript config files (`eslint.config.ts`, `eslint.config.mts`, `eslint.config.cts`)
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

<!-- One-paragraph explanation of the feature. -->

## Motivation

<!-- Why are we doing this? What use cases does it support? What is the expected
outcome? -->

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
Copy link
Member

@bmish bmish Mar 12, 2024 •
edited
Loading

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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?

aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

<!--
This is the bulk of the RFC.

Explain the design with enough detail that someone familiar with ESLint
can implement it by reading this document. Please get into specifics
of your approach, corner cases, and examples of how the change will be
used. Be sure to define any new terms in this section.
-->

The goal is to seamlessly support TypeScript configuration files in ESLint. To achieve this, ESLint will need to recognize and parse TypeScript configuration files in the same way it does for JavaScript configuration files. This will involve creating the configuration file resolution logic to recognize `.ts`, `.mts`, and `.cts` files as valid ESLint configuration files. We will need to treat `eslint.config.mts` files as ECMAScript Modules (ESM) and `eslint.config.cts` files as CommonJS (CJS). The module resolution for `eslint.config.ts` files will depend on the `type` field in the nearest `package.json`. The maintainers of ESLint have raised some valid concerns some of which include:
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

- There should not be extra overhead for JavaScript users. This means this change should not have a significant impact (if any at all) affecting users who use plain JavaScript config files.
- The external tools that are used to parse the config files written in TypeScript should not create side effects. Specifically, it is imperative that these tools do not interfere with Node.js's native module resolution system by hooking into or altering the standard `import/require` mechanisms. This means tools like [`ts-node`](https://github.com/TypeStrong/ts-node) and [`tsx`](https://github.com/privatenumber/tsx) might not be suitable for this purpose.
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

So far the tool that seems to be the most suitable for this purpose is [`jiti`](https://github.com/unjs/jiti). It does not introduce side effects and performs well, demonstrating its reliability. It also seems to be more battle-tested given some established frameworks such as [Nuxt](https://github.com/nuxt/nuxt) and [Tailwind CSS](https://github.com/tailwindlabs/tailwindcss) have been using it to load their configuration files.
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

## Documentation

<!--
How will this RFC be documented? Does it need a formal announcement
on the ESLint blog to explain the motivation?
-->

The documentation for this feature will be added to the [ESLint User Guide](https://eslint.org/docs/user-guide/configuring) page. The documentation will explain how to use TypeScript configuration files and the differences between JavaScript and TypeScript configuration files.

## Drawbacks

<!--
Why should we *not* do this? Consider why adding this into ESLint
might not benefit the project or the community. Attempt to think
about any opposing viewpoints that reviewers might bring up.

Any change has potential downsides, including increased maintenance
burden, incompatibility with other tools, breaking existing user
experience, etc. Try to identify as many potential problems with
implementing this RFC as possible.
-->

This change will most likely require at least one external tool as either a dependency or a peer dependency.

## Backwards Compatibility Analysis

<!--
How does this change affect existing ESLint users? Will any behavior
change for them? If so, how are you going to minimize the disruption
to existing users?
-->

This goal is to minimize the disruption to existing users. The primary focus is to ensure that the existing JavaScript configuration files continue to work as expected. The changes will only affect TypeScript users who are using TypeScript configuration files. The changes will not affect the existing JavaScript configuration files.
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

## Alternatives
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

<!--
What other designs did you consider? Why did you decide against those?

This section should also include prior art, such as whether similar
projects have already implemented a similar feature.
-->

Copy link
Member

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

Copy link

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.

Copy link

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.

Copy link

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.

Copy link

@silverwind silverwind Apr 11, 2024 •
edited
Loading

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.

Copy link
Member

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.

Copy link

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@privatenumber I tried to use tsx tsImport as explained here to load a .ts config file, but didn't succeed. This is my code:

let config;
const { tsImport, register } = require("tsx/esm/api");
const unregister = register();
try {
    config = (await tsImport(fileURL.href, __filename)).default;
} finally {
    unregister();
}

where fileURL is the URL of this config file:

// eslint.config.mts
export default [];

The error stack trace indicates that the config file is being loaded as CommonJS:

SyntaxError: Unexpected token 'export'
    at wrapSafe (node:internal/modules/cjs/loader:1389:18)
    at Module._compile (node:internal/modules/cjs/loader:1425:20)
    at Module._extensions..js (node:internal/modules/cjs/loader:1564:10)
    at Module.load (node:internal/modules/cjs/loader:1287:32)
    at Module._load (node:internal/modules/cjs/loader:1103:12)
    at cjsLoader (node:internal/modules/esm/translators:318:15)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:258:7)
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:474:24)

I guess I'm doing something wrong?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not the best place to discuss this. Can you send me a reproduction link in the tsx repo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not the best place to discuss this. Can you send me a reproduction link in the tsx repo?

Thanks! I think the problem is that I wasn't calling register from require("tsx/cjs/api"). It works if I add that call before tsImport. Another problem is that there is seemingly no way to completely undo the changes done by tsImport to the Node.js loader internals. Particularly, I think there is no way to unregister the ESM Module loader. I didn't even find out how to do that programmatically, so this is possibly a limitation of Node.js.

## Open Questions
Copy link
Member

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?


<!--
This section is optional, but is suggested for a first draft.

What parts of this proposal are you unclear about? What do you
need to know before you can finalize this RFC?

List the questions that you'd like reviewers to focus on. When
you've received the answers and updated the design to reflect them,
you can remove this section.
-->

1. How is caching going to work with TypeScript config files?
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
2. Should we look at the nearest `tsconfig.json` file to determine the module resolution for `eslint.config.ts` files? Most likely not, but it's worth considering.
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
3. Should we allow some sort of interoperability between JavaScript and TypeScript configuration files? For example, should we allow a TypeScript configuration file to extend a JavaScript configuration file and vice versa?
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
4. Should we allow `eslint.config.ts` to be able to use `export default` as well as `module.exports` (might be related to [TypeScript's automatic Module Detection](https://www.typescriptlang.org/tsconfig#moduleDetection))?
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

## Help Needed

<!--
This section is optional.

Are you able to implement this RFC on your own? If not, what kind
of help would you need from the team?
-->

I will be implementing this feature. I will need help from the team to review the code and provide feedback.

## Frequently Asked Questions

<!--
This section is optional but suggested.

Try to anticipate points of clarification that might be needed by
the people reviewing this RFC. Include those questions and answers
in this section.
-->

## Related Discussions
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved

<!--
This section is optional but suggested.

If there is an issue, pull request, or other URL that provides useful
context for this proposal, please include those links here.
-->

[This PR](https://github.com/eslint/eslint/pull/18134) is related to this RFC.