fix(eslint-plugin-tsdoc): declare eslint as a peer dependency - #483
Open
Christopher Pruijsen (cpruijsen) wants to merge 1 commit into
Open
fix(eslint-plugin-tsdoc): declare eslint as a peer dependency#483Christopher Pruijsen (cpruijsen) wants to merge 1 commit into
Christopher Pruijsen (cpruijsen) wants to merge 1 commit into
Conversation
Published typings import from 'eslint', so consumers under pnpm and Yarn PnP need it declared for TypeScript to resolve against the consumer's ESLint.
Author
@microsoft-github-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Declare
"peerDependencies": { "eslint": ">=7" }so the consumer's ESLint is linked into the plugin and the published typings bind to the same types the config is checked against.eslintremains adevDependencyfor this repo's tests. Add a test that every non-relative import in the emittedlib/index.d.tsis listed independenciesorpeerDependencies.Published
lib/index.d.tsdoesimport type * as eslint from 'eslint', buteslint-plugin-tsdocdid not declareeslintas a dependency or peer (only adevDependency). Under pnpm and Yarn PnP, TypeScript can resolve that import to a transitive@types/eslintinstead of the consumer's ESLint. A// @ts-checked flat config withplugins: { tsdoc }then fails (IPluginnot assignable toPluginbecauseRuleModuleis notRuleDefinition).Fixes #479
Decision
The range is the one in the issue, which the reporter verified via pnpm
packageExtensions. The issue repro (eslint-plugin-tsdoc@0.5.2,eslint@10.8.0,typescript@6.0.3, pnpm,@types/eslint-scope) istsc --noEmitfailing, then exiting 0 withpackageExtensionseslint: ">=7". That isolated consumer was not re-run in this tree.The other option is to remove the
'eslint'type import from the public surface and type the plugin with@typescript-eslint/utils(already a runtime dependency) or@eslint/core. That is the "more durable" option in the issue and is the direction of draft PR #425. The peer declaration is the smallest reversible packaging change, and it matches how@rushstack/eslint-plugin(already in this repo) declares ESLint. RewritingIPlugincan reintroduce assignability failures against ESLint 10'sPlugintype.Can switch to the types-only approach, or to a tighter peer range (
^8 || ^9 || ^10).>=7may be wider than runtime support: this package already usescontext.filenamewith nogetFilename()fallback, and@typescript-eslint/utils@~8.56.0peers^8.57 || ^9 || ^10.Test plan
rush test --only eslint-plugin-tsdoc(existingtsdoc/syntaxRuleTester cases plus the new packaging invariant).missingPackageNamesis["eslint"]) and passes when it is restored.rush change --verifyagainstmain.rush install/rush updatewith the new peer: nopnpm-lock.yamlchange required (eslintis already a devDependency).eslint; report if ESLint 7 should be dropped from the range.The last box was not run.