-
Notifications
You must be signed in to change notification settings - Fork 39
feat: add plugin meta and support typescript-eslint@7 #86
feat: add plugin meta and support typescript-eslint@7 #86
Conversation
@JoshuaKGoldberg - thanks for doing this. I'll have to disable the deprecation plugin for now, just to get the rest of our ecosystem onto eslint 9, but I'll be following this so we can bring it back once it's merged! |
👋 since it's been a month, ping @gund - any chance you'd have time to look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! I'm so sorry it took so long to review this but I finally made some time to review it.
Thanks so much for the upgrade, it's great! Just left one small comment to resolve, otherwise looks good!
btw what about these dev dependencies, shouldn't they also be updated to v7? "@typescript-eslint/eslint-plugin": "^6.0.0",
"@typescript-eslint/rule-tester": "^6.0.0",
"@typescript-eslint/parser": "^6.0.0", |
Co-authored-by: Alex Malkevich <[email protected]>
@@ -7,6 +7,7 @@ | |||
"declaration": true, | |||
"noEmitHelpers": true, | |||
"importHelpers": true, | |||
"skipLibCheck": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this:
$ npm run build
> [email protected] prebuild
> rimraf dist
> [email protected] build
> tsc --build tsconfig.build.json
node_modules/ts-api-utils/lib/index.d.cts:1:8 - error TS1259: Module '"/Users/josh/repos/eslint-plugin-deprecation/node_modules/typescript/lib/typescript"' can only be default-imported using the 'esModuleInterop' flag
1 import ts from 'typescript';
~~
node_modules/typescript/lib/typescript.d.ts:7159:1
7159 export = ts;
~~~~~~~~~~~~
This module is declared with 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.
Found 1 error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for the updates!
Seems like this breaks backwards compatibility with ESlint v7... eslint-plugin-deprecation/package.json Line 32 in ae8ca95
And also dropping the BC test for v7 from the matrix here: eslint-plugin-deprecation/.github/workflows/test.yml Lines 47 to 55 in ae8ca95
What do you think @JoshuaKGoldberg? |
I'm ok with that! ESLint v8 has been out for ... quite a while. |
}, | ||
"peerDependencies": { | ||
"eslint": "^7.0.0 || ^8.0.0", | ||
"eslint": "^8.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're in the area ... we could do this, I think? So swapping (v7, v8) with (v8, v9)?
"eslint": "^8.0.0", | |
"eslint": "^8.0.0 || ^9.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, heh, I suppose that's already covered in #79.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🎉 This PR is included in version 3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adds
@typescript-eslint/utils@7
as an allowed version, as both 6 and 7 work with this package.Adds a
meta
per https://eslint.org/docs/latest/extend/plugin-migration-flat-config#adding-plugin-meta-information as a non-breaking change. Adding in themeta
actually fixes #85, since this plugin doesn't expose any preset config anyway.Also swaps from the long-inactive
tsutils
to its successor,ts-api-utils
. That's what@typescript-eslint/*
packages use.