chore: stop installing with --legacy-peer-deps - #215
Open
adamjohnwright wants to merge 7 commits into
Open
adamjohnwright wants to merge 7 commits into
adamjohnwright wants to merge 7 commits into
Conversation
It was on every install here -- CI, deploy, release verification, the Dockerfile, the local setup guide -- and the tree stopped needing it long ago. Measured 2026-09-16: `npm ci` on main resolves and installs strictly, 2109 packages, and a strict `npm install` leaves the lockfile untouched. It was not free while it stayed. It accepts an inconsistent tree, so a dependency bump that cannot resolve at all passed every check: Angular's packages peer-depend on one another at exact versions, dependabot's group moved eleven of them and left `platform-browser-dynamic` and `compiler-cli` behind, and CI was green. The only thing that objected was this repo's pre-push hook, which runs `npm ci` without the flag -- so the gate existed locally and not where it mattered. Removing it makes CI the check it was meant to be. A bump that cannot resolve now fails where it is raised rather than where someone happens to push from. CONTRIBUTING says why, so it is not added back the next time an install fails on peers. If an install fails on peer dependencies the tree is wrong; the fix is the versions, not telling npm to stop looking. Left alone deliberately: the Dockerfile still uses `npm install` rather than `npm ci`. Making container builds reproducible is worth doing and is a different change from this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 16, 2026
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
1 participant
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.
You were right that it reads like a workaround. It is one, and the tree stopped
needing it a long time ago.
It is not needed
Measured today on main, in a clean directory:
No flag, no failure. It appears to have come in with the first CI pipeline and was
never revisited.
It was not free
The flag accepts an inconsistent dependency tree, so a bump that cannot resolve
at all passed every check. Angular's packages peer-depend on one another at
exact versions; dependabot's group (#161) moved eleven of them and left
platform-browser-dynamicandcompiler-clibehind:Six green checks. The only thing that objected was this repo's pre-push hook,
which runs
npm ciwithout the flag — so the gate existed on a developer'smachine and not in CI, which is exactly backwards.
What changes
--legacy-peer-depsis removed from CI, deploy, release verification, theDockerfile and the local setup guide. CONTRIBUTING says why, so it is not added
back the next time an install fails on peers:
What this will do
It will make some future dependabot PRs fail, including #161 as it stands. That
is the point — they fail now at the PR that introduces them, rather than passing
and leaving a tree that only installs when you ask npm not to look.
Deliberately not in this PR
The Dockerfile still uses
npm installrather thannpm ci. Reproduciblecontainer builds are worth having and are a separate change from this one — I
started making it, noticed it was scope creep, and put it back.
🤖 Generated with Claude Code