Build: Detect stale node_modules at build/dev time#77995
Open
manzoorwanijk wants to merge 2 commits intotrunkfrom
Open
Build: Detect stale node_modules at build/dev time#77995manzoorwanijk wants to merge 2 commits intotrunkfrom
manzoorwanijk wants to merge 2 commits intotrunkfrom
Conversation
Replace bin/packages/validate-typescript-version.js with a generic install-drift check that catches stale node_modules from any dependency, not just typescript. The new script (tools/validation/check-installed-deps.mjs) compares package-lock.json against npm's hidden lockfile (node_modules/.package-lock.json) and verifies installed integrity per registry-fetched entry, ignoring workspace links and platform-skipped optional deps. Set up tools/validation/ as a workspace (@wordpress/validation-tools, picked up by the existing tools/* glob). Build, dev, and build:profile-types invoke the check via `npm run check-installed-deps --workspace @wordpress/validation-tools`. Aligns with the validation-tools workspace direction from #77522 and #75041, sitting alongside the existing check-*/validate-* scripts. Also extends the CLI/bin/env eslint override (no-console: off) to cover tools/validation/**, mirroring the same change in #77522.
manzoorwanijk
requested review from
Mamaduka,
aduth,
ciampo,
jsnajdr,
mirka,
sirreal,
t-hamano and
tyxla
manzoorwanijk
added
the
[Type] Build Tooling
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in d5ae5b8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25429739923
|
|
Size Change: 0 B Total Size: 7.95 MB ℹ️ View Unchanged
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a generic “stale node_modules” guard for Gutenberg’s local build/dev flows by comparing the declared lockfile (package-lock.json) against npm’s installed-tree lockfile (node_modules/.package-lock.json). It replaces the previous TypeScript-only drift check so developers get a fast, actionable failure (“run npm install”) when dependencies are out of sync.
Changes:
- Add a new validation workspace (
tools/validation) with acheck-installed-depsscript that checks lockfile vs installed-tree integrity. - Run the new dependency sync check as Step 0 in
bin/build.mjsandbin/dev.mjs, and inbuild:profile-types. - Expand the ESLint “allow console” override to include
tools/**/*.js|mjs.
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/validation/package.json | Adds a new private workspace package to host validation scripts. |
| tools/validation/check-installed-deps.mjs | Implements the lockfile vs installed-tree drift detection logic. |
| tools/eslint/config.mjs | Allows console usage in tools/** scripts (consistent with CLI/tooling usage). |
| package.json | Switches build:profile-types to run the new dependency sync check. |
| package-lock.json | Registers @wordpress/validation-tools as a workspace link. |
| bin/build.mjs | Runs dependency sync validation as Step 0 before the rest of the build. |
| bin/dev.mjs | Runs dependency sync validation as Step 0 before starting the dev build/watch. |
| bin/packages/validate-typescript-version.js | Removes the legacy TypeScript-only drift guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three small fixes from #77995's review: - Track total mismatch count separately from the printed list, so the reported count is accurate when more than MAX_REPORTED packages drift. Verbose output now appends "... and N more." when truncated. - Rename the verbose label `version mismatch` -> `integrity mismatch`, since the comparison is on the integrity hash (not the semver string). - Move and reword the optional-deps comment to sit alongside the branch it describes, and clarify that real drift on optional deps is still caught by the integrity check below.
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.
What?
Inspired by #77950.
Replace the TypeScript-only install-drift guard (
bin/packages/validate-typescript-version.js, called frombin/build.mjs/bin/dev.mjs) with a generic lockfile-sync check that catches drift from any dependency, not justtypescript.The new script lives at
tools/validation/check-installed-deps.mjsand is invoked frombin/build.mjs,bin/dev.mjs, and thebuild:profile-typesscript.Why?
In the review of #77950 (this comment, finding 5), @ciampo flagged that removing
validate-typescript-version.jsloses a local-dev guardrail. Syncpack only checks declared values across workspaces — it does not detectnode_modulesbeing stale relative topackage.json/package-lock.json. A developer who pulls a branch that bumps TypeScript without re-runningnpm installwould now hit cryptictsgoerrors instead of the previous clear "Detected vs Required" message.The follow-up discussion settled on the right shape:
This PR is that follow-up — a wholistic check that flags any out-of-sync
node_modules, not just TypeScript.How?
The script compares
package-lock.json(declared) againstnode_modules/.package-lock.json(npm's hidden lockfile, rewritten on every install to record the actual installed tree). For every registry-fetched entry, it checks that both files agree onintegrity; mismatches and missing entries exit non-zero with a hint to runnpm install. Workspace links and platform-skipped optional deps are excluded to avoid false positives.It runs as the first step of
npm run buildandnpm run dev, so a stale install fails fast with a clear message instead of producing confusing downstream errors.Testing Instructions
All cases run locally; no wp-env required.
1. Happy path (in-sync install) — should pass
Expected: build prints
🔍 Checking dependencies...as Step 0 followed by✔ All good., then runs through the remaining steps and finishes with🎉 Build completed successfully!.2. Stale install — missing
node_modules/.package-lock.jsonmv node_modules/.package-lock.json node_modules/.package-lock.json.bak npm run build # Restore: mv node_modules/.package-lock.json.bak node_modules/.package-lock.jsonExpected: build aborts at Step 0 with:
3. Stale install — a package was removed since last install
Simulate without actually breaking your
node_modules:Expected: build aborts at Step 0 with:
4. Stale install — a package's version changed (integrity mismatch)
Expected: build aborts at Step 0 with:
5. Verbose mode — show technical details
The script supports a
--verboseflag for debugging that shows which specific packages are out of sync (or which path was missing). With one of the failure scenarios from tests 2-4 in place:Expected (e.g. for a single missing package):
6. Dev mode — check also runs as Step 0
npm run dev # (Ctrl-C once you see "🔍 Checking dependencies..."; no need to wait for the full watch.)Expected: prints
🔍 Checking dependencies...immediately after the "Starting development build..." banner.7. Lint
Expected: passes.
Screenshots or screencast
N/A — CLI-only change.
Use of AI Tools
This PR was authored with assistance from Claude Code (Anthropic). The script logic, workspace scaffolding, and PR description were drafted with AI assistance; all changes were reviewed, tested locally, and verified against a clean install plus the failure simulations described in the testing instructions above.