Update TypeScript-related packages and rules #171
Conversation
sjinks
force-pushed
the
update/typescript
branch
from
e60bf15
to
437c312
Compare
package.json
Outdated
| "@typescript-eslint/eslint-plugin": "^5.55.0 || ^6.0.0", | ||
| "@typescript-eslint/parser": "^5.55.0 || ^6.0.0", |
There was a problem hiding this comment.
Why the ||? these aren't peerDeps, so they will resolve the left hand side of the operator and will be stuck there via package-lock. (In this case, @typescript-eslint/parser gets 5.62)
There was a problem hiding this comment.
This was because of eslint-plugin-jest :-( It should usually resolve to the highest possible version.
There was a problem hiding this comment.
Updated to 6.0.0 and eslint-plugin-jest from 27.2.1 to 27.1.3.
package.json
Outdated
| @@ -32,13 +32,13 @@ | |||
| }, | |||
| "homepage": "https://github.com/Automattic/eslint-config-wpvip#readme", | |||
| "dependencies": { | |||
| "@babel/eslint-parser": "7.21.3", | |||
| "@babel/eslint-parser": "^7.22.10", | |||
There was a problem hiding this comment.
Can we stick to exact refs?
There was a problem hiding this comment.
We can, but we have copies of @babel/eslint-parser in our projects (say, in vip-cli). All babel-related packages are updated together, and we will end up having multiple copies of @babel/eslint-parser.
There was a problem hiding this comment.
We can remove @babel/eslint-parser from VIP CLI, as well as any other peer dependencies leftover from older versions of this plugin. We now have a module resolution adapter that pulls dependencies from this project.
I have done this for other projects I've updated and it works very well. The only two dependencies needed when using this plugin are eslint and @automattic/eslint-plugin-wpvip.
https://github.com/Automattic/eslint-config-wpvip#configuration
There was a problem hiding this comment.
OK, but if we stick to the exact version, will it be safe to update other Babel-related packages? I would like to avoid publishing a new package version every time we update Babel.
There was a problem hiding this comment.
If we align on major version (7, currently) then I have not seen any issues with minor version drift.
There was a problem hiding this comment.
OK, I have removed the caret.
| { | ||
| "column": 121, | ||
| "endColumn": 121, | ||
| "endLine": 45, | ||
| "fix": { | ||
| "range": [ | ||
| 894, | ||
| 894, | ||
| ], | ||
| "text": " | ||
| ", | ||
| }, | ||
| "line": 45, | ||
| "message": "Insert \`⏎\`", | ||
| "messageId": "insert", | ||
| "nodeType": null, | ||
| "ruleId": "prettier/prettier", | ||
| "severity": 2, | ||
| }, |
There was a problem hiding this comment.
This is because of this change:
https://github.com/Automattic/eslint-config-wpvip/pull/171/files#diff-0df5474287e5a16dc96f268437aaa2035aa854b4d43249f0ac20de228ca943eeR45 (removed EOL in __fixtures__/javascript.js)
| { | ||
| "column": 48, | ||
| "endColumn": 61, | ||
| "endLine": 1, | ||
| "line": 1, | ||
| "message": "'three' is defined but never used.", | ||
| "messageId": "unusedVar", | ||
| "nodeType": "Identifier", | ||
| "ruleId": "@typescript-eslint/no-unused-vars", | ||
| "severity": 2, | ||
| }, |
There was a problem hiding this comment.
It looks like the @typescript-eslint/no-unused-vars rule now catches unused parameters as well.
export function add( one: number, two: number, three: String ): number {
return one + two;
}
| { | ||
| "column": 55, | ||
| "endColumn": 61, | ||
| "endLine": 1, | ||
| "fix": { | ||
| "range": [ | ||
| 54, | ||
| 60, | ||
| ], | ||
| "text": "string", | ||
| }, | ||
| "line": 1, | ||
| "message": "Don't use \`String\` as a type. Use string instead", | ||
| "messageId": "bannedTypeMessage", | ||
| "nodeType": "Identifier", | ||
| "ruleId": "@typescript-eslint/ban-types", | ||
| "severity": 2, | ||
| }, |
There was a problem hiding this comment.
@typescript-eslint/ban-types was added in 6.0.0: https://github.com/typescript-eslint/typescript-eslint/blob/v6.0.0/packages/eslint-plugin/src/configs/strict-type-checked.ts#L13
| { | ||
| "column": 1, | ||
| "endColumn": 14, | ||
| "endLine": 5, | ||
| "line": 5, | ||
| "message": "Do not use "@ts-ignore" because it alters compilation errors.", | ||
| "messageId": "tsDirectiveComment", | ||
| "nodeType": "Line", | ||
| "ruleId": "@typescript-eslint/ban-ts-comment", | ||
| "severity": 2, | ||
| }, |
There was a problem hiding this comment.
@typescript-eslint/ban-ts-comment added in 6.0: https://github.com/typescript-eslint/typescript-eslint/blob/v6.0.0/packages/eslint-plugin/src/configs/strict-type-checked.ts#L12
| "severity": 1, | ||
| "severity": 2, |
There was a problem hiding this comment.
@typescript-eslint/prefer-ts-expect-error is an error in 6.0.0: https://github.com/typescript-eslint/typescript-eslint/blob/v6.0.0/packages/eslint-plugin/src/configs/strict-type-checked.ts#L65
| { | ||
| "column": 49, | ||
| "endColumn": 54, | ||
| "endLine": 29, | ||
| "line": 29, | ||
| "message": "'value' is defined but never used.", | ||
| "messageId": "unusedVar", | ||
| "nodeType": "Identifier", | ||
| "ruleId": "@typescript-eslint/no-unused-vars", | ||
| "severity": 2, | ||
| }, |
There was a problem hiding this comment.
Added to strict-type-checked in 6.0.0: https://github.com/typescript-eslint/typescript-eslint/blob/v6.0.0/packages/eslint-plugin/src/configs/strict-type-checked.ts#L65
| '@typescript-eslint/no-shadow': 'error', | ||
| 'import/no-duplicates': 'error', |
There was a problem hiding this comment.
sjinks
force-pushed
the
update/typescript
branch
from
19ffcf7
to
357ac98
Compare
| @@ -42,4 +42,4 @@ export function shadow() { | |||
|
|
|||
| nonExistent(); | |||
|
|
|||
| // There is intentionally no new line at the end of this file to hit [eol-last](https://eslint.org/docs/rules/eol-last). | |||
| // There is intentionally no new line at the end of this file to hit [eol-last](https://eslint.org/docs/rules/eol-last). | |||
There was a problem hiding this comment.
thanks, this one keeps getting clobbered
@typescript-eslint(this adds support for TypeScript 5.1+)eslint-plugin-importto 2.28.1 (necessary for this)@babel/eslint-parserto 7.22.10