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

fix: make plugin-kit types usable in CommonJS #143

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
11 changes: 9 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
CI: true

test_types:
name: Test Types (core)
name: Test Types
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -65,13 +65,20 @@ jobs:
with:
node-version: "lts/*"

- name: npm install and test types
- name: npm install and test types (core)
working-directory: packages/core
run: |
npm install
npm run build
npm run test:types

- name: npm install and test types (plugin-kit)
working-directory: packages/plugin-kit
run: |
npm install
npm run build
npm run test:types

jsr_test:
name: Verify JSR Publish
runs-on: ubuntu-latest
Expand Down
10 changes: 10 additions & 0 deletions packages/core/tests/types/cjs-import.test.cts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* @fileoverview CommonJS type import test for ESLint Core.
* @author Francesco Trotta
*/

//-----------------------------------------------------------------------------
// Imports
//-----------------------------------------------------------------------------

import "@eslint/core";
2 changes: 1 addition & 1 deletion packages/core/tests/types/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
"rootDir": "../..",
"strict": true
},
"files": ["../../dist/esm/types.d.ts", "types.test.ts"]
"include": [".", "../../dist"]
}
8 changes: 8 additions & 0 deletions packages/plugin-kit/build-cts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { readFile, writeFile } from "node:fs/promises";
fasttime marked this conversation as resolved.
Show resolved Hide resolved

const oldSourceText = await readFile("dist/esm/index.d.ts", "utf-8");
const newSourceText = oldSourceText.replaceAll(
'import("./types.ts")',
'import("./types.ts", { with: { "resolution-mode": "import" } })',
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious: Will this work with TypeScript 4? And if not, does it matter?

Copy link
Member Author

@fasttime fasttime Jan 1, 2025

Choose a reason for hiding this comment

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

It will not work because TypeScript 4 doesn't support import attributes. The alternative would be renaming types.ts to types.cts in dist/cjs and updating the imports in index.d.cts accordingly.

Update:

Copy link
Member

Choose a reason for hiding this comment

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

I think renaming to types.cts and updating the references seems like a safer option for best compatibility. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. I switched to importing from types.cts in 38bc8e9. We don't have policy about TypeScript versions support like we have for Node.js versions, but since the import attributes proposal is still not finalized, types.cts looks like a safer choice.

);
await writeFile("dist/cjs/index.d.cts", newSourceText);
15 changes: 8 additions & 7 deletions packages/plugin-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,25 @@
"homepage": "https://github.com/eslint/rewrite#readme",
"scripts": {
"build:dedupe-types": "node ../../tools/dedupe-types.js dist/cjs/index.cjs dist/esm/index.js",
"build:cts": "node -e \"fs.copyFileSync('dist/esm/index.d.ts', 'dist/cjs/index.d.cts')\"",
"build:cts": "node ./build-cts.js",
"build": "rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && npm run build:cts",
"test:jsr": "npx jsr@latest publish --dry-run",
"pretest": "npm run build",
"test": "mocha tests/",
"test:coverage": "c8 npm test"
"test:coverage": "c8 npm test",
"test:jsr": "npx jsr@latest publish --dry-run",
"test:types": "tsc -p tests/types/tsconfig.json"
},
"keywords": [
"eslint",
"eslintplugin",
"eslint-plugin"
],
"license": "Apache-2.0",
"devDependencies": {
"dependencies": {
"@eslint/core": "^0.9.1",
"levn": "^0.4.1"
},
"devDependencies": {
"@types/levn": "^0.4.0",
"c8": "^9.1.0",
"mocha": "^10.4.0",
Expand All @@ -56,8 +60,5 @@
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"dependencies": {
"levn": "^0.4.1"
}
}
10 changes: 10 additions & 0 deletions packages/plugin-kit/tests/types/cjs-import.test.cts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* @fileoverview CommonJS type import test for ESLint Plugin Kit.
* @author Francesco Trotta
*/

//-----------------------------------------------------------------------------
// Imports
//-----------------------------------------------------------------------------

import "@eslint/plugin-kit";
9 changes: 9 additions & 0 deletions packages/plugin-kit/tests/types/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"noEmit": true,
"rootDir": "../..",
"strict": true
},
"include": [".", "../../dist"]
}
104 changes: 104 additions & 0 deletions packages/plugin-kit/tests/types/types.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* @fileoverview Type tests for ESLint Plugin Kit.
* @author Francesco Trotta
*/

//-----------------------------------------------------------------------------
// Imports
//-----------------------------------------------------------------------------

import {
BooleanConfig,
CallMethodStep,
ConfigCommentParser,
Directive,
DirectiveType,
RulesConfig,
SourceLocation,
SourceRange,
StringConfig,
TextSourceCodeBase,
VisitNodeStep,
} from "@eslint/plugin-kit";

//-----------------------------------------------------------------------------
// Tests
//-----------------------------------------------------------------------------

// CallMethodStep
class TestCallMethodStep extends CallMethodStep {
constructor({ target, args }: { target: string; args: [string, number] }) {
super({ target, args });
}
}
const step2 = new TestCallMethodStep({ target: "foo", args: ["foo", 42] });
step2.args satisfies unknown[];
step2.kind satisfies 2;
step2.target satisfies string;
step2.type satisfies "call";

// ConfigCommentParser
const configCommentParser = new ConfigCommentParser();
configCommentParser.parseDirective("foo") satisfies
| { label: string; value: string; justification: string }
| undefined;
const jsonLikeConfig = configCommentParser.parseJSONLikeConfig("bar");
if (jsonLikeConfig.ok) {
jsonLikeConfig.config satisfies RulesConfig;
} else {
jsonLikeConfig.error.message satisfies string;
}
configCommentParser.parseListConfig("baz") satisfies BooleanConfig;
configCommentParser.parseStringConfig("qux") satisfies StringConfig;

// Directive
void ((type: "disable" | "enable" | "disable-next-line" | "disable-line") => {
const directive = new Directive({
type,
node: {},
value: "foo",
justification: "bar",
});
directive.justification satisfies string;
directive.node satisfies unknown;
directive.type satisfies DirectiveType;
directive.value satisfies string;
});

// TextSourceCodeBase
class TestTextSourceCode extends TextSourceCodeBase {
declare ast: { foo: string; bar: number };
constructor({
text,
ast,
}: {
text: string;
ast: { foo: string; bar: number };
}) {
super({ text, ast, lineEndingPattern: /\r\n|[\r\n\u2028\u2029]/u });
}
}
const sourceCode = new TestTextSourceCode({
text: "text",
ast: { foo: "ABC", bar: 123 },
});
sourceCode.ast satisfies { foo: string; bar: number };
sourceCode.getAncestors({}) satisfies object[];
sourceCode.getLoc({}) satisfies SourceLocation;
sourceCode.getParent({}) satisfies object | undefined;
sourceCode.getRange({}) satisfies SourceRange;
sourceCode.getText() satisfies string;
sourceCode.getText({}, 0, 1) satisfies string;

// VisitNodeStep
class TestVisitNodeStep extends VisitNodeStep {
constructor({ target, phase }: { target: object; phase: 1 | 2 }) {
super({ target, phase, args: ["foo", 42] });
}
}
const step1 = new TestVisitNodeStep({ target: { foo: "bar" }, phase: 2 });
step1.args satisfies unknown[];
step1.kind satisfies 1;
step1.phase satisfies 1 | 2;
step1.target satisfies object;
step1.type satisfies "visit";
Loading