fix(js): include transitive workspace deps in pruned pnpm lockfile#35532
fix(js): include transitive workspace deps in pruned pnpm lockfile#35532FrozenPandaz merged 13 commits intomasterfrom
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit 7460849
☁️ Nx Cloud last updated this comment at |
When app -> lib-a -> lib-b, prune-lockfile produced a lockfile missing the lib-b importer, missing lib-b's npm deps from packages:, and kept 'workspace:*' as the specifier for lib-b in lib-a's importer. Since copy-workspace-modules rewrites lib-a/package.json to "file:../lib-b", pnpm install --frozen-lockfile failed with ERR_PNPM_OUTDATED_LOCKFILE. - traverseWorkspaceNode now recurses into workspace->workspace edges with a visited set, so transitive workspace npm deps reach the pruned graph. - stringifyPnpmLockfile BFS-collects transitive workspace importers and rewrites workspace-package references to the flat workspace_modules/ layout via file:<rel> / link:<rel>, matching what copy-workspace-modules writes to disk. Fixes #34655
Co-authored-by: FrozenPandaz <FrozenPandaz@users.noreply.github.com>
…agers Fold the standalone pnpm transitive-workspace-dep test into the existing describe.each so pnpm/yarn/npm all exercise transitive workspace deps. Replace YAML-regex assertions with a real install contract: run the package manager's frozen-lockfile install against the pruned dist/.
…pace Copy the pruned dist to a fresh tmp dir outside the e2e workspace before running install. Previously the install ran inside the workspace, where parent pnpm-workspace.yaml / hoisted node_modules could mask missing deps and let a broken pruned lockfile pass. The dist is the deployment artifact; install it the way it actually gets deployed.
…ilure runCommand silently swallows non-zero exits by default — pass failOnError: true so the install step is a real assertion. The prior CI green didn't actually validate that pnpm/yarn/npm install succeeded against the pruned dist.
Same class of bug as the pnpm fix: mapWorkspaceModules only iterated the root packageJson.dependencies, so app -> lib-a -> lib-b only emitted lib-a's entries. `npm ci` against the pruned dist then failed with "Missing: <lib-b> from lock file". BFS through workspace deps so transitive workspace packages get their node_modules + workspace_modules entries too, with a visited guard to handle cycles.
…ansitives - pnpm-parser: walk peerDependencies in addition to dependencies and optionalDependencies during the transitive workspace BFS, so workspace packages referenced only as peers still get importer blocks. Initialize importer.specifiers when missing (lockfile v6+ may omit it) so the rewrite to file:<rel> always lands. - npm-parser: walk optionalDependencies and peerDependencies in the transitive BFS for the same reason. - Add unit tests for the npm-parser transitive workspace dep + cycle cases mirroring the pnpm-parser coverage.
app -> lib-a <-> lib-b across pnpm/yarn/npm. Without the visited guard in the parser BFS or in traverseWorkspaceNode, this hangs at prune-lockfile time.
- Extract WORKSPACE_DEP_TYPES constant (used 4x across both parsers) - Pre-build name->snapshot map in npm-parser to avoid O(n*m) scan inside the BFS - Drop redundant defensive checks (workspaceModules.has() already guards) - Trim narrative comments that restated the code; keep only WHY notes - Tighten test assertions
Native algorithm, no string round-trip — slightly faster and idiomatic on Node 17+.
…t the task graph level Cycle protection is still covered by the parser unit tests, which bypass Nx's task scheduler and exercise the BFS visited guard directly.
cafcf0b to
7460849
Compare
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud has identified a possible root cause for your failed CI:
We determined this failure is unrelated to the PR changes. The e2e-nx:e2e-ci--src/cache-no-daemon.test.ts task fails with EADDRINUSE :::3000 — port 3000 is already occupied in the CI environment when the remote cache fixture tries to bind to it. Since this project is not in the set of touched projects and the port conflict is a pre-existing infrastructure issue, no code changes are needed.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
Current Behavior
@nx/js:prune-lockfiledoes not recursively walk workspace→workspace dependencies. Given anapp → @myorg/lib-a → @myorg/lib-b → lodashchain (wherelib-aandlib-bare workspace packages), the executor produces a prunedpnpm-lock.yamlthat:workspace_modules/@myorg/lib-b(the transitive workspace dep).lodash(lib-b's npm dep) from thepackages:section.specifier: workspace:*forlib-binsidelib-a's importer block, even though@nx/js:copy-workspace-modulesrewriteslib-a/package.jsonto"@myorg/lib-b": "file:../lib-b".The specifier mismatch causes
pnpm install --frozen-lockfileto fail in the deployed output:Expected Behavior
@nx/js:prune-lockfilerecursively discovers transitive workspace dependencies and produces a lockfile that:importersblock for every workspace module thatcopy-workspace-moduleswrites to disk.packages:section.workspace_modules/layout (specifier: file:<rel>/version: link:<rel>), matching whatcopy-workspace-moduleswrites to each package'spackage.json.pnpm install --frozen-lockfilesucceeds in the pruned output directory.Implementation
Two coordinated changes in lockfile-side code:
project-graph-pruning.ts—traverseWorkspaceNodenow recurses into workspace→workspace dependency edges with avisitedset, so transitive workspace npm deps reach the pruned graph.pnpm-parser.ts—stringifyPnpmLockfileBFS-collects transitive workspace importers, deep-clones each importer block, and rewrites workspace-package references to the flatworkspace_modules/layout.Tests
pnpm-parser.spec.tscovering the canonical chain, dependency cycles, and diamond shapes.e2e/js/src/js-executor-prune-lockfile.test.tsexercising the canonical chain end-to-end.pnpm install --frozen-lockfilenow succeeds in the pruned output where it previously failed withERR_PNPM_OUTDATED_LOCKFILE.Credit
Diagnosis and original fix sketch by @estevaolucas in #35347 — this PR carries forward the recursion + specifier rewrite portions in a focused change. The other concerns from #35347 (devDep/peerDep stripping, catalog reference resolution in
copy-workspace-modules) are real but separable and intentionally left for follow-up issues.Related Issue(s)
Fixes #34655