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

feat: support TypeScript config using importx #18440

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/src/use/configure/configuration-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ The ESLint configuration file may be named any of the following:
* `eslint.config.js`
* `eslint.config.mjs`
* `eslint.config.cjs`
* `eslint.config.ts` (requires [TypeScript setup](#typescript-support))
* `eslint.config.mts` (requires [TypeScript setup](#typescript-support))
* `eslint.config.cts` (requires [TypeScript setup](#typescript-support))

It should be placed in the root directory of your project and export an array of [configuration objects](#configuration-objects). Here's an example:

Expand Down Expand Up @@ -453,3 +456,19 @@ npx eslint --config some-other-file.js **/*.js
```

In this case, ESLint does not search for `eslint.config.js` and instead uses `some-other-file.js`.

## TypeScript Support

::: warning
Loading TypeScript configuration files is an experimental feature and may change in future releases.
:::

ESLint supports loading configuration files written in TypeScript optionally. The TypeScript support is powered by [`importx`](https://github.com/antfu/importx), which does not come out-of-the-box with ESLint and you will need to install it as a dev dependency first:

```shell
npm install --save-dev importx
```

Note that `importx` only strips off type annotations and does not perform type checking.

When both `eslint.config.js` and `eslint.config.ts` are present in the same directory, JavaScript versions always take precedence unless provided specifically via `--config` option.
5 changes: 4 additions & 1 deletion knip.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@
"rollup-plugin-node-polyfills",

// FIXME: not sure why is eslint-config-eslint reported as unused
"eslint-config-eslint"
"eslint-config-eslint",

// Optional dependencies for loading TypeScript configs
"importx"
]
},
"docs": {
Expand Down
29 changes: 27 additions & 2 deletions lib/eslint/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ const { Retrier } = require("@humanwhocodes/retry");
const FLAT_CONFIG_FILENAMES = [
"eslint.config.js",
"eslint.config.mjs",
"eslint.config.cjs"
"eslint.config.cjs",
"eslint.config.ts",
"eslint.config.mts",
"eslint.config.cts"
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
];
const debug = require("debug")("eslint:eslint");
const privateMembers = new WeakMap();
Expand Down Expand Up @@ -270,6 +273,17 @@ function findFlatConfigFile(cwd) {
);
}

/**
* Check if the file is a TypeScript file.
* @param {string} filePath The file path to check.
* @returns {boolean} `true` if the file is a TypeScript file, `false` if it's not.
*/
function isFileTS(filePath) {
const fileExtension = path.extname(filePath);

return fileExtension.endsWith("ts");
}

/**
* Load the config array from the given filename.
* @param {string} filePath The filename to load from.
Expand Down Expand Up @@ -313,7 +327,18 @@ async function loadFlatConfigFile(filePath) {
delete require.cache[filePath];
}

const config = (await import(fileURL)).default;
let config;

if (isFileTS(filePath)) {
Copy link
Member

Choose a reason for hiding this comment

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

Because Node.js is the only runtime that can't load TypeScript natively, maybe we can also check that globalThis.Deno and globalThis.Bun are undefined before loading importx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With register/module-loader like ts-node or tsx (the cli), or testing frameworks like Vitest, Node.js can load TypeScript - the goal for importx is to cover those details, where ESLint or other users can be agnostic to runtime like Deno and Bun, while also could automatically work if we have yet another runtime in the future.

importx already doing lazying loading internally - so importing importx itself should be fairly lightweight.

Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure I'm understanding, you're saying importx will already check for Deno and Bun and not do anything additional in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly.

Copy link
Member

@fasttime fasttime Jun 2, 2024

Choose a reason for hiding this comment

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

I tried to install importx on Deno (marked as a dev dependency in package.json if that matters) and it still downloaded a bunch of packages, including jiti, tsx and typescript. It seems unnecessary to require users to install all those packages if their runtime handles TS files out of the box.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah that's a good point. In that case, I think a line to check the Deno and Bun in ESLint won't hurt, yes - Will do.

config = await import("importx")
.then(r => r.import(fileURL, __filename));
} else {
config = await import(fileURL);
}

if (config.default) {
config = await config.default;
}

importedConfigFileModificationTime.set(filePath, mtime);

Expand Down
9 changes: 9 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@
"strip-ansi": "^6.0.1",
"text-table": "^0.2.0"
},
"peerDependencies": {
"importx": "*"
},
"peerDependenciesMeta": {
"importx": {
"optional": true
}
},
"devDependencies": {
"@babel/core": "^7.4.3",
"@babel/preset-env": "^7.4.3",
Expand Down Expand Up @@ -162,6 +170,7 @@
"semver": "^7.5.3",
"shelljs": "^0.8.5",
"sinon": "^11.0.0",
"importx": "^0.3.5",
"typescript": "^5.3.3",
"vite-plugin-commonjs": "^0.10.0",
"webpack": "^5.23.0",
Expand Down
15 changes: 15 additions & 0 deletions tests/fixtures/config-ts/cts/eslint.config.cts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
interface FakeFlatConfigItem {
plugins?: Record<string, unknown>;
name?: string;
rules?: Record<string, unknown>;
}

const config: FakeFlatConfigItem[] = [
{
rules: {
"no-undef": "error" as string
}
},
]

module.exports = config;
15 changes: 15 additions & 0 deletions tests/fixtures/config-ts/custom/eslint.custom.config.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
interface FakeFlatConfigItem {
plugins?: Record<string, unknown>;
name?: string;
rules?: Record<string, unknown>;
}

const config: FakeFlatConfigItem[] = [
{
rules: {
"no-undef": "error" as string
}
},
]

export default config;
5 changes: 5 additions & 0 deletions tests/fixtures/config-ts/js-ts-mixed/eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
rules: {
"no-undef": "error"
}
};
5 changes: 5 additions & 0 deletions tests/fixtures/config-ts/js-ts-mixed/eslint.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
rules: {
"no-undef": "warn" as string
}
};
1 change: 1 addition & 0 deletions tests/fixtures/config-ts/js-ts-mixed/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo;
15 changes: 15 additions & 0 deletions tests/fixtures/config-ts/mts/eslint.config.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
interface FakeFlatConfigItem {
plugins?: Record<string, unknown>;
name?: string;
rules?: Record<string, unknown>;
}

const config: FakeFlatConfigItem[] = [
{
rules: {
"no-undef": "error" as string
}
},
]

export default config;
17 changes: 17 additions & 0 deletions tests/fixtures/config-ts/top-level-await/eslint.config.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@

interface FakeFlatConfigItem {
plugins?: Record<string, unknown>;
name?: string;
rules?: Record<string, unknown>;
}

const config: FakeFlatConfigItem[] = [
// Top-level await
await Promise.resolve({
rules: {
"no-undef": "error" as string
}
}),
]

export default config;
5 changes: 5 additions & 0 deletions tests/fixtures/config-ts/ts-const-enum/enum.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const enum Level {
Error = 2,
Warn = 1,
Off = 0,
}
9 changes: 9 additions & 0 deletions tests/fixtures/config-ts/ts-const-enum/eslint.config.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { Level } from "./enum.mts";

export default [
{
rules: {
"no-undef": Level.Error
}
},
] as const;
9 changes: 9 additions & 0 deletions tests/fixtures/config-ts/ts-namespace/eslint.config.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { LocalNamespace } from "./namespace.mts";

export default [
{
rules: {
"no-undef": LocalNamespace.Level.Error
}
},
] as const;
7 changes: 7 additions & 0 deletions tests/fixtures/config-ts/ts-namespace/namespace.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export namespace LocalNamespace {
export const enum Level {
Error = 2,
Warn = 1,
Off = 0,
}
}
15 changes: 15 additions & 0 deletions tests/fixtures/config-ts/ts/eslint.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
interface FakeFlatConfigItem {
plugins?: Record<string, unknown>;
name?: string;
rules?: Record<string, unknown>;
}

const config: FakeFlatConfigItem[] = [
{
rules: {
"no-undef": "error" as string
}
},
]

module.exports = config;
21 changes: 13 additions & 8 deletions tests/lib/cli-engine/lint-result-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,21 @@ describe("LintResultCache", () => {
it("contains node version during hashing", () => {
const version = "node-=-version";

sandbox.stub(process, "version").value(version);
const NewLintResultCache = proxyquire("../../../lib/cli-engine/lint-result-cache.js", {
"./hash": hashStub
});
const newLintResultCache = new NewLintResultCache(cacheFileLocation, "metadata");
const versionStub = sandbox.stub(process, "version").value(version);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't correctly restored, causing process.version to always be the mocked value for sub-sequence tests.

Choose a reason for hiding this comment

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

Oh my gosh, thank you so much for figuring this one out, I was banging my head on the wall for quite some time trying to figure this one out.


newLintResultCache.getCachedLintResults(filePath, fakeConfig);
try {
const NewLintResultCache = proxyquire("../../../lib/cli-engine/lint-result-cache.js", {
"./hash": hashStub
});
const newLintResultCache = new NewLintResultCache(cacheFileLocation, "metadata");

assert.ok(hashStub.calledOnce);
assert.ok(hashStub.calledWithMatch(version));
newLintResultCache.getCachedLintResults(filePath, fakeConfig);

assert.ok(hashStub.calledOnce);
assert.ok(hashStub.calledWithMatch(version));
} finally {
versionStub.restore();
}
});
});

Expand Down