Skip to content

build(npm): make the manifest forward-compatible with npm 12 - #30455

Open
caugner wants to merge 2 commits into
mainfrom
fred-1868-npm-v12-forward-compat
Open

build(npm): make the manifest forward-compatible with npm 12#30455
caugner wants to merge 2 commits into
mainfrom
fred-1868-npm-v12-forward-compat

Conversation

@caugner

@caugner caugner commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Add an allowScripts allowlist for lefthook and unrs-resolver, and drop the engines.npm: "^11" constraint.

Test results and supporting details

  • npm 12 no longer runs dependency install scripts unless the root package.json allowlists them. lefthook's postinstall registers the git hooks, so without the entry contributors silently lose them.
  • Combined with engine-strict=true in .npmrc, engines.npm: "^11" makes any install here fail outright with EBADENGINE under npm 12. devEngines.packageManager (npm >=11.8.0) already states what local development expects, and it accepts npm 12.
  • Downstream consumers were never affected: the published package is built from a generated build/package.json carrying only name and version.
  • Entries are name-only (--no-allow-scripts-pin), so Dependabot bumps need no re-approval commits, at the cost of trusting future versions of the listed packages.
  • Verified: npm ci --strict-allow-scripts passes on npm 12.0.2.

Related issues

Part of mdn/fred#1868.

npm 12 no longer runs `preinstall`/`install`/`postinstall` from
dependencies unless the root `package.json` lists them in `allowScripts`.
`npm ci` still succeeds, so a missing entry only surfaces later as a
missing binary.

Generated with `npm approve-scripts --all --no-allow-scripts-pin`, so the
entries are name-only and keep matching after a dependency bump.
Combined with `engine-strict=true` in `.npmrc`, `engines.npm: "^11"`
makes `npm install` in this repo fail with `EBADENGINE` under npm 12.

`devEngines.packageManager` already states the npm version expected for
local development, and it accepts npm 12.
@github-actions github-actions Bot added infra Infrastructure issues (npm, GitHub Actions, releases) of this project size:s [PR only] 7-24 LoC changed labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs).

@caugner
caugner marked this pull request as ready for review September 8, 2026 17:04
@caugner
caugner requested review from a team and mdn-bot as code owners September 8, 2026 17:04
@caugner
caugner requested a review from LeoMcA September 8, 2026 17:04
@ddbeck

ddbeck commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

I have no idea what's going on here. Can you add some context? Why should I approve this PR?

@caugner

caugner commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

I have no idea what's going on here. Can you add some context? Why should I approve this PR?

I don't understand your question. There is context in the commit messages, in the PR description, and in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra Infrastructure issues (npm, GitHub Actions, releases) of this project size:s [PR only] 7-24 LoC changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants