chore: bump lint plugins and resolve floating-version fallout#8940
chore: bump lint plugins and resolve floating-version fallout#8940cryptodev-2s wants to merge 1 commit into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution MetaMask internal reviewing guidelines:
|
d61485f to
47f28ca
Compare
345c44b to
7557269
Compare
0xEdouardEth
left a comment
There was a problem hiding this comment.
Approve changes on chain-agnostic-permission
--
import MetaMaskOpenRPCDocument from "./build/openrpc.json";
import MultiChainOpenRPCDocument from "./build/multichain-openrpc.json";
export { MetaMaskOpenRPCDocument, MultiChainOpenRPCDocument };
export default MetaMaskOpenRPCDocument;
mcmire
left a comment
There was a problem hiding this comment.
I'm not totally sure I understand why this is needed. We seem to be bumping two packages by a major version, which is fine if there are new changes we want to take advantage of. But I am not sure how it would make the lint step less deterministic as we are using ^ to indicate ranges, which should tell Yarn to stay within a minor version. Plus we now get two extra peer dependency warnings we didn't get before (I've noted them below).
| "eslint-config-prettier": "^9.1.0", | ||
| "eslint-import-resolver-typescript": "^3.6.3", | ||
| "eslint-plugin-import-x": "^4.3.0", | ||
| "eslint-import-resolver-typescript": "^4.4.4", |
There was a problem hiding this comment.
@metamask/eslint-config-typescript has a peer dependency on eslint-import-resolver-typescript ^3.6.3.
There was a problem hiding this comment.
Sorry I forget to move this to draft it will require the work here MetaMask/eslint-config#444 before
| "eslint-plugin-jest": "^28.8.3", | ||
| "eslint-plugin-jsdoc": "^50.2.4", | ||
| "eslint-plugin-n": "^17.10.3", | ||
| "eslint-plugin-n": "^18.0.1", |
There was a problem hiding this comment.
@metamask/eslint-config-nodejs has a peer dependency on @metamask/eslint-config-nodejs ^17.10.3.
There was a problem hiding this comment.
Sorry I forget to move this to draft it will require the work here MetaMask/eslint-config#444 before
870023d
7557269 to
870023d
Compare
`eslint-plugin-import-x`, `eslint-plugin-n`, `eslint-import-resolver-typescript`, and `prettier` were floating to older versions because `yarn.lock` happened to hold them; the platform- api-docs PR (#8012) added scoped resolutions to keep that frozen. Bumping the four packages to the latest versions matching their semver ranges removes the need for those resolutions and surfaces five pre-existing lint issues that the older plugins didn't flag: - `eslint-plugin-n@18` rejects `import.meta.dirname` in `eslint.config.mjs:132` because the property is only stable in Node 22.16+/24+, while the repo's engines field is `^18.18 || >=20`. Resolve `dirname(fileURLToPath(import.meta.url))` instead so the config works on every supported Node version. - `eslint-plugin-import-x@4.16` enables `no-named-as-default` by default. `@metamask/api-specs` exports `MetaMaskOpenRPCDocument` as both a named export and the default; switch the `chain-agnostic-permission` import to the named form to avoid the ambiguous reference. - `import-x/no-extraneous-dependencies` now catches `@metamask/messenger` in `react-data-query/src/createUIQueryClient.test.ts` and `@metamask/keyring-controller` in `transaction-pay-controller/src/strategy/relay/hyperliquid-withdraw{,.test}.ts`. Both packages already used these symbols at runtime/test time without declaring them. Add them to the relevant `dependencies` / `devDependencies` blocks. `yarn dedupe` and `yarn constraints --fix` also rolled the `messenger-cli` package's `prettier` range forward to match the new root range. No behavior changes in any of the affected packages — their tests all still pass at 100% coverage.
870023d to
002d368
Compare
…aMask#8971) ## Explanation `packages/react-data-query/src/createUIQueryClient.test.ts` imports `Messenger` from `@metamask/messenger`, but the package wasn't listed in `react-data-query`'s `devDependencies`. That's an `import-x/no-extraneous-dependencies` violation — the currently-pinned plugin version misses it but newer versions flag it. This PR adds the missing devDep so the relationship is declared explicitly and the test file lints cleanly under newer plugin versions. Also regenerates `README.md` so the workspace dependency graph reflects the new edge. ## References - Surfaced while investigating the scoped lint-plugin resolutions on MetaMask#8012. - Extracted from the parked MetaMask#8940 so the dep cleanup can land independently of the lint-plugin major bumps that PR also carried. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) — N/A, devDep only, not consumer-facing - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them — N/A
…er as a dependency (MetaMask#8972) ## Explanation `src/strategy/relay/hyperliquid-withdraw{,.test}.ts` imports `@metamask/keyring-controller` at runtime, but the package wasn't listed in `dependencies`. Consumers were relying on transitive hoisting to install it — once a stricter `import-x/no-extraneous-dependencies` runs (or yarn's hoisting layout changes), the missing dep would surface. This PR adds it explicitly so the dep relationship is correct and consumers get the package installed transitively. Also regenerates `README.md` so the workspace dependency graph reflects the new edge. ## References - Surfaced while investigating the scoped lint-plugin resolutions on MetaMask#8012. - Extracted from the parked MetaMask#8940 so the dep cleanup can land independently of the lint-plugin major bumps that PR also carried. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them — N/A
Explanation
PR #8012 (feat/messenger-docs-site) originally carried scoped resolutions for
eslint-plugin-import-x,eslint-plugin-n,eslint-import-resolver-typescript, andprettier. Investigating those (per r3326274335) showed they weren't resolving a transitive-dep conflict —yarn whyconfirms each is consumed only by the root workspace. They existed purely to prevent the lockfile from drifting forward to plugin versions that the codebase hadn't been cleaned against. #8012 has since dropped them.This PR is the follow-up: bump those plugins to the latest versions matching their (newly-widened) semver ranges and fix the two pre-existing lint issues the newer plugins surface, so the lockfile can stay current without the workaround resolutions ever coming back:
eslint-plugin-import-x:4.6.1→^4.16.2eslint-plugin-n:17.15.1→^18.0.1eslint-import-resolver-typescript:3.10.1→^4.4.4prettier:3.4.2→^3.8.3Lint fallout fixed
eslint.config.mjs:import.meta.dirnameis only stable in Node 22.16+/24+, but the repo'senginesfield is^18.18 || >=20.eslint-plugin-n@18'sno-unsupported-featuresrule catches this. Resolve the config dir throughdirname(fileURLToPath(import.meta.url))so the config works across the supported Node range.packages/chain-agnostic-permission/src/scope/constants.ts:eslint-plugin-import-x@4.16enablesno-named-as-defaultby default.@metamask/api-specsexportsMetaMaskOpenRPCDocumentas both a named export and the default; switch to the named-import form so the reference is unambiguous.packages/messenger-cli/package.json:yarn constraints --fixrolled itsprettierrange forward to match the new root range.Shipped separately
Two
import-x/no-extraneous-dependenciesviolations that surface under the bumped plugins were extracted into their own PRs and have already merged:@metamask/messengeronreact-data-query(devDep)@metamask/keyring-controllerontransaction-pay-controller(runtime dep)References
bb6d436d5; this PR keeps the lockfile from drifting back to versions that need themeslint-import-resolver-typescriptto^4.0.0so consumers using@metamask/eslint-config-typescriptdon't see a peer-dep mismatchChecklist