feat: Add types #18854

nzakas · 2024-09-04T16:20:20Z

Prerequisites checklist

I have read the contributing guidelines.

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)

Added the type definitions from @types/eslint to the project:

Added all type definitions in lib/types
Added tests in tests/lib/types
Added new test:types script to run type checking tests
Added npm run test:types to CI to be run on each PR
Added types entries to package.json

For simplicity, I did not enable type checking for the whole repo, just for the type tests.

This is intended to be a base for us to build off of. We can look to make improvements in future PRs.

fixes #18845

Is there anything you'd like reviewers to focus on?

The types are copied over as-is without modification (aside from including the license). We don't need to review their contents right now.

Does the testing setup make sense?

netlify · 2024-09-04T16:20:35Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`a5beaf2`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/66db14c73744780008fdf176

fixes #18845

nzakas · 2024-09-05T20:30:09Z

eslint.config.js

@@ -93,7 +93,8 @@ module.exports = [
            "tests/performance/**",
            "tmp/**",
            "**/test.js",
-            ".vscode"
+            ".vscode",
+            "**/*.ts"


Just for now. 😄

nzakas · 2024-09-05T21:52:49Z

Looks like the webpack plugin is using an older version of @types/eslint, but still seems like there may be an error in the types for formatters?

fasttime · 2024-09-05T22:14:10Z

I can confirm the Formatter type is broken, see #18821 (comment). Would patching that type help to fix the error?

nzakas · 2024-09-05T22:26:23Z

Yes, I think that's all it would take to make both integration tests pass. I'll give it a shot.

nzakas · 2024-09-06T14:51:47Z

Okay, I fixed the incorrect type and we now have three integration tests with other projects.

fasttime · 2024-09-06T19:10:35Z

lib/types/index.d.ts

+    }
+
+    interface Formatter {
+        format(results: LintResult[], data?: LintResultData): string | Promise<string>;


The second argument of LoadedFormatter#format() is just an object with an optional maxWarningsExceeded property:

eslint/docs/src/integrate/nodejs-api.md

Lines 469 to 470 in 6e79ac7

* `format` (`(results: LintResult[], resultsMeta: ResultsMeta) => string | Promise<string>`)<br>

The method to convert the [LintResult] objects to text. `resultsMeta` is an object that will contain a `maxWarningsExceeded` object if `--max-warnings` was set and the number of warnings exceeded the limit. The `maxWarningsExceeded` object will contain two properties: `maxWarnings`, the value of the `--max-warnings` option, and `foundWarnings`, the number of lint warnings.

The cwd and rulesMeta properties are passed directly to the underlying formatter function:

eslint/lib/eslint/eslint.js

Lines 1245 to 1255 in 9bde90c

return formatter(results, {

...resultsMeta,

cwd,

get rulesMeta() {

if (!rulesMeta) {

rulesMeta = eslint.getRulesMetaForResults(results);

}

return rulesMeta;

}

});

Since that type has been broken for so long and the integration tests are passing now, we can merge this change as it is, and I will try to sort up things in a follow-up PR.

fasttime · 2024-09-06T19:13:53Z

tests/lib/types/types.test.ts

+
+sourceCode.getAllComments();
+
+sourceCode.getJSDocComment(AST); // $ExpectType Comment | null


$ExpectType annotations are not working any more. We should also fix this in a follow-up.

fasttime

LGTM, thanks! I'm going to merge this PR now so that it gets included in the ESLint v9.10.0 release.

sam3k · 2024-09-08T00:50:59Z

Per 2024-09-05 TSC meeting, this was released early and we will await feedback from users to make sure it is working as expected.

@nzakas added some integration tests for the types.

See eslint/eslint#18854

dimitropoulos · 2024-09-09T16:50:16Z

thank you for doing this!

nzakas requested a review from a team as a code owner September 4, 2024 16:20

eslint-github-bot bot added the feature This change adds a new feature to ESLint label Sep 4, 2024

github-actions bot added the github actions label Sep 4, 2024

feat: Add types

c9888c9

fixes #18845

nzakas force-pushed the issue18845 branch from 91080c9 to c9888c9 Compare September 4, 2024 16:21

Ignore .ts files

29df12a

nzakas added the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Sep 5, 2024

nzakas commented Sep 5, 2024

View reviewed changes

nzakas added 3 commits September 5, 2024 17:14

Add integration test for types with eslint-webpack-plugin

0001f05

Fix integration test

3ac8a73

Add neostandard integration test

5f707d1

Fix neostandard integration test

5bd2513

nzakas added 2 commits September 5, 2024 18:26

Update LintResultData

4e3b4f4

Add one more integration test

a5beaf2

nzakas mentioned this pull request Sep 6, 2024

Scheduled release for September 6th, 2024 #18819

Closed

28 tasks

fasttime added the accepted There is consensus among the team that this change meets the criteria for inclusion label Sep 6, 2024

fasttime reviewed Sep 6, 2024

View reviewed changes

fasttime removed the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Sep 6, 2024

fasttime approved these changes Sep 6, 2024

View reviewed changes

fasttime merged commit 301b90d into main Sep 6, 2024
23 checks passed

fasttime deleted the issue18845 branch September 6, 2024 19:56

voxpelli added a commit to neostandard/neostandard that referenced this pull request Sep 9, 2024

fix(deps): types are bundled with ESLint >=9.10

53e1179

See eslint/eslint#18854

voxpelli mentioned this pull request Sep 9, 2024

fix(deps): types are bundled with ESLint >=9.10 neostandard/neostandard#164

Merged

tygyh mentioned this pull request Sep 29, 2024

[Snyk] Upgrade eslint from 9.3.0 to 9.10.0 tygyh/godot#4

Closed

Loonz206 mentioned this pull request Sep 30, 2024

[Snyk] Upgrade eslint from 9.9.1 to 9.10.0 Loonz206/todoredux#345

Merged

lholmquist mentioned this pull request Oct 1, 2024

[Snyk] Upgrade eslint from 8.57.0 to 9.10.0 nodeshift/kube-service-bindings#233

Closed

tygyh mentioned this pull request Oct 2, 2024

[Snyk] Upgrade eslint from 9.3.0 to 9.10.0 tygyh/godot#6

Closed

This was referenced Oct 2, 2024

[Snyk] Upgrade eslint from 8.57.0 to 9.10.0 nodeshift/kube-service-bindings#234

Closed

[Snyk] Upgrade eslint from 8.57.0 to 9.10.0 nodeshift/kube-service-bindings#235

Closed

ebiz-markusrissmann mentioned this pull request Oct 4, 2024

[Snyk] Upgrade eslint from 8.57.1 to 9.10.0 MarkusRissmann-private/githubclient#174

Open

qdraw mentioned this pull request Oct 4, 2024

[Snyk] Upgrade eslint from 9.9.1 to 9.10.0 qdraw/starsky#1748

Closed

lholmquist mentioned this pull request Oct 5, 2024

[Snyk] Upgrade eslint from 8.57.0 to 9.10.0 nodeshift/kube-service-bindings#236

Closed

bnichell mentioned this pull request Oct 7, 2024

Change Request: Include types for @eslint/js in the package #18998

Closed

1 task

Loonz206 mentioned this pull request Oct 16, 2024

[Snyk] Upgrade eslint from 9.9.1 to 9.11.1 Loonz206/todoredux#349

Merged

This was referenced Oct 28, 2024

[Snyk] Upgrade eslint from 9.3.0 to 9.12.0 tygyh/godot#11

Closed

[Snyk] Upgrade @eslint/js from 9.10.0 to 9.12.0 tygyh/godot#12

Closed

lholmquist mentioned this pull request Oct 28, 2024

[Snyk] Upgrade eslint from 8.57.0 to 9.12.0 nodeshift/kube-service-bindings#241

Closed

ekmixon mentioned this pull request Oct 28, 2024

[Snyk] Upgrade eslint from 6.8.0 to 9.12.0 ekmixon/aquasecurity-cloudsploit#150

Open

This was referenced Oct 29, 2024

[Snyk] Upgrade eslint from 8.57.1 to 9.12.0 nerdy-tech-com-gitub/images#7

Open

[Snyk] Upgrade eslint from 6.8.0 to 9.12.0 nerdy-tech-com-gitub/foreman#2

Open

Loonz206 mentioned this pull request Oct 30, 2024

[Snyk] Upgrade eslint from 9.9.1 to 9.12.0 Loonz206/todoredux#355

Open

This was referenced Oct 30, 2024

[Snyk] Upgrade eslint from 8.51.0 to 9.12.0 nodeshift/opossum#899

Open

[Snyk] Upgrade eslint from 8.46.0 to 9.12.0 nodeshift/faas-js-runtime#369

Open

zntb mentioned this pull request Nov 1, 2024

[Snyk] Upgrade eslint from 9.10.0 to 9.12.0 zntb/nextjs-template#70

Closed

lholmquist mentioned this pull request Nov 4, 2024

[Snyk] Upgrade eslint from 8.57.0 to 9.12.0 nodeshift/kube-service-bindings#244

Closed

sahilbhelkar mentioned this pull request Nov 5, 2024

[Snyk] Upgrade eslint from 9.0.0 to 9.12.0 sahilbhelkar/space#4

Open

nerdy-tech-com-gitub mentioned this pull request Nov 10, 2024

[Snyk] Upgrade eslint from 8.57.1 to 9.13.0 nerdy-tech-com-gitub/tailwindcss#7

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add types #18854

feat: Add types #18854

nzakas commented Sep 4, 2024 •

edited

Loading

netlify bot commented Sep 4, 2024 •

edited

Loading

nzakas Sep 5, 2024

nzakas commented Sep 5, 2024

fasttime commented Sep 5, 2024

nzakas commented Sep 5, 2024

nzakas commented Sep 6, 2024

fasttime Sep 6, 2024

fasttime Sep 6, 2024

fasttime left a comment

sam3k commented Sep 8, 2024

dimitropoulos commented Sep 9, 2024

	* `format` (`(results: LintResult[], resultsMeta: ResultsMeta) => string \| Promise<string>`)<br>
	The method to convert the [LintResult] objects to text. `resultsMeta` is an object that will contain a `maxWarningsExceeded` object if `--max-warnings` was set and the number of warnings exceeded the limit. The `maxWarningsExceeded` object will contain two properties: `maxWarnings`, the value of the `--max-warnings` option, and `foundWarnings`, the number of lint warnings.

	return formatter(results, {
	...resultsMeta,
	cwd,
	get rulesMeta() {
	if (!rulesMeta) {
	rulesMeta = eslint.getRulesMetaForResults(results);
	}

	return rulesMeta;
	}
	});


		sourceCode.getAllComments();

		sourceCode.getJSDocComment(AST); // $ExpectType Comment \| null

feat: Add types #18854

feat: Add types #18854

Conversation

nzakas commented Sep 4, 2024 • edited Loading

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Sep 4, 2024 • edited Loading

✅ Deploy Preview for docs-eslint canceled.

nzakas Sep 5, 2024

Choose a reason for hiding this comment

nzakas commented Sep 5, 2024

fasttime commented Sep 5, 2024

nzakas commented Sep 5, 2024

nzakas commented Sep 6, 2024

fasttime Sep 6, 2024

Choose a reason for hiding this comment

fasttime Sep 6, 2024

Choose a reason for hiding this comment

fasttime left a comment

Choose a reason for hiding this comment

sam3k commented Sep 8, 2024

dimitropoulos commented Sep 9, 2024

nzakas commented Sep 4, 2024 •

edited

Loading

netlify bot commented Sep 4, 2024 •

edited

Loading