Skip to content

fix(core): emit shadowed deep duplicates and avoid over-elevation in …#35573

Open
xavier-dfns wants to merge 1 commit intonrwl:masterfrom
xavier-dfns:fix/npm-lockfile-stringify-shadowing
Open

fix(core): emit shadowed deep duplicates and avoid over-elevation in …#35573
xavier-dfns wants to merge 1 commit intonrwl:masterfrom
xavier-dfns:fix/npm-lockfile-stringify-shadowing

Conversation

@xavier-dfns
Copy link
Copy Markdown

Heavily vibe coded fix for #29593. I will forgive you if you close it but I hope it can bring some attention to it.

Bugs

stringifyNpmLockfile produces dist lockfiles that npm ci rejects when the root tree contains intermediate-version shadowing:

  1. Shadowed same-version copies dropped. The project graph dedups by (packageName, version), so when the same version exists at multiple paths in root because an intermediate sibling holds a different version, only one path is emitted. The deep copy is missing and walk-up from the consumer hits the wrong-version sibling.
  2. Over-eager elevation creates shadows. elevateNestedPaths hoisted nested entries into any empty ancestor slot, inventing intermediate-level entries that didn't exist in root and shadowing walk-up resolution for any deeper consumer of a different version.
    Standalone POC reproducing both against nx@22.7.0 below.

Fixes

  • emitRootDuplicates (after mapSnapshots) re-emits root paths whose (name, version) matches a graph node and whose parent is reachable in the dist tree, iterated to a fixpoint.
  • wouldShadowDescendant gates empty-slot elevation in elevateNestedPaths — only elevate if no descendant of the slot's parent declares a different version of the same package.

Tests

New shadowing and over-elevation regressions suite covers each bug with a synthetic lockfile (both verified to fail without the fix).
Two __fixtures__/ lockfiles updated; both encoded the buggy output (npm-hoisting was missing required deep picomatch@2.3.1 copies, auxiliary-packages had over-elevated entries).

POC

mkdir nx-bug-repro && cd nx-bug-repro
npm init -y
npm install --save-dev nx@22.7.0
# copy repro.js into this directory
node repro.js

repro.js:

/**
 * Reproduces two bugs in `stringifyNpmLockfile` (Nx 22.7.0).
 *
 * Both bugs cause the dist lockfile produced by `@nx/esbuild:esbuild` /
 * `@nx/js:tsc` (with `generatePackageJson: true` and `generateLockfile: true`)
 * to differ from what npm itself would produce — `npm ci` then refuses to
 * install in the dist directory because walk-up resolution lands on the
 * wrong version.
 *
 * Each scenario builds a synthetic root `package-lock.json` in memory,
 * feeds it through Nx's parser/pruner/stringifier, and verifies a specific
 * structural invariant is broken in the output.
 *
 *   $ node repro.js
 */

// Nx's `package.json#exports` doesn't expose its lock-file internals, so
// resolve them by absolute path through node_modules.
const path = require('path');
const nxRoot = path.dirname(require.resolve('nx/package.json'));
const {
  stringifyNpmLockfile,
  getNpmLockfileNodes,
  getNpmLockfileDependencies,
} = require(path.join(nxRoot, 'dist/src/plugins/js/lock-file/npm-parser'));
const { pruneProjectGraph } = require(
  path.join(nxRoot, 'dist/src/plugins/js/lock-file/project-graph-pruning'),
);
const { ProjectGraphBuilder } = require(
  path.join(nxRoot, 'dist/src/project-graph/project-graph-builder'),
);

let hashCounter = 0;
function stringify(rootLockFile, packageJson) {
  const hash = `mock-${++hashCounter}`;
  const { nodes: externalNodes, keyMap } = getNpmLockfileNodes(
    JSON.stringify(rootLockFile),
    hash,
  );
  const ctx = {
    projects: {},
    externalNodes,
    fileMap: { nonProjectFiles: [], projectFileMap: {} },
    filesToProcess: { nonProjectFiles: [], projectFileMap: {} },
    nxJsonConfiguration: null,
    workspaceRoot: '/virtual',
  };
  const deps = getNpmLockfileDependencies(
    JSON.stringify(rootLockFile),
    hash,
    ctx,
    keyMap,
  );
  const builder = new ProjectGraphBuilder({
    nodes: {},
    dependencies: {},
    externalNodes,
  });
  for (const d of deps) builder.addDependency(d.source, d.target, d.type, null);
  const graph = builder.getUpdatedProjectGraph();
  const pruned = pruneProjectGraph(graph, packageJson);
  return JSON.parse(stringifyNpmLockfile(pruned, JSON.stringify(rootLockFile), packageJson));
}

function entry(version) {
  return {
    version,
    resolved: `https://registry.npmjs.org/x/-/x-${version}.tgz`,
    integrity: `sha512-${'a'.repeat(86)}==`,
  };
}

// ─── Bug 1: shadowed same-version deep duplicates are dropped ───────────────
//
// Root layout:
//   node_modules/pkg@1.0.0                                                   (top)
//   node_modules/wrapper@1.0.0                            deps: pkg 2.0.0, consumer 1.0.0
//   node_modules/wrapper/node_modules/pkg@2.0.0
//   node_modules/wrapper/node_modules/consumer@1.0.0      deps: pkg 1.0.0 (exact)
//   node_modules/wrapper/node_modules/consumer/
//                       node_modules/pkg@1.0.0                               (deep dup)
//
// npm nested the deep `pkg@1.0.0` under `consumer` because `wrapper`'s sibling
// `pkg@2.0.0` shadows walk-up resolution. Nx's project graph deduplicates by
// `(name, version)`, so it collapses the two `pkg@1.0.0` paths into one node
// and emits only the top-level placement. The deep copy is missing from the
// dist lockfile, and walk-up from `consumer` finds `wrapper/pkg@2.0.0` instead
// — `npm ci` rejects with "Invalid: lock file's pkg@2.0.0 does not satisfy
// pkg@1.0.0".
function bug1() {
  const root = {
    name: 'workspace',
    version: '0.0.0',
    lockfileVersion: 3,
    requires: true,
    packages: {
      '': {
        name: 'workspace',
        version: '0.0.0',
        dependencies: { app: '1.0.0', pkg: '1.0.0' },
      },
      'node_modules/app': {
        ...entry('1.0.0'),
        dependencies: { wrapper: '1.0.0' },
      },
      'node_modules/wrapper': {
        ...entry('1.0.0'),
        dependencies: { pkg: '2.0.0', consumer: '1.0.0' },
      },
      'node_modules/wrapper/node_modules/pkg': entry('2.0.0'),
      'node_modules/wrapper/node_modules/consumer': {
        ...entry('1.0.0'),
        dependencies: { pkg: '1.0.0' },
      },
      'node_modules/wrapper/node_modules/consumer/node_modules/pkg': entry('1.0.0'),
      'node_modules/pkg': entry('1.0.0'),
    },
  };
  const pkg = {
    name: 'dist-app',
    version: '0.0.0',
    dependencies: { app: '1.0.0', pkg: '1.0.0' },
  };
  const out = stringify(root, pkg);
  const deep = 'node_modules/wrapper/node_modules/consumer/node_modules/pkg';
  const sibling = 'node_modules/wrapper/node_modules/pkg';

  console.log('Bug 1 — shadowed same-version deep duplicate');
  console.log('  consumer pins  pkg @ 1.0.0');
  console.log(`  walk-up hits   ${out.packages[sibling].version} at ${sibling}`);
  console.log(
    `  deep copy at   ${deep}: ${out.packages[deep] ? out.packages[deep].version : 'MISSING ❌'}`,
  );
  console.log(
    out.packages[deep] && out.packages[deep].version === '1.0.0'
      ? '  ✅ FIXED'
      : '  ❌ npm ci would fail: "lock file\'s pkg@2.0.0 does not satisfy pkg@1.0.0"',
  );
}

// ─── Bug 2: over-eager elevation invents shadowing intermediate entries ─────
//
// Root layout:
//   node_modules/pkg@1.0.0                                                   (top)
//   node_modules/parent@1.0.0                                  deps: wrapper, child
//   node_modules/parent/node_modules/wrapper@1.0.0             deps: pkg ^2.0.0
//   node_modules/parent/node_modules/wrapper/
//                node_modules/pkg@2.0.0                                      (3-deep)
//   node_modules/parent/node_modules/child@1.0.0               deps: pkg 1.0.0 (exact)
//
// In root, `parent` has no `node_modules/pkg` of its own — `child`'s walk-up
// reaches the top-level `pkg@1.0.0` correctly. Nx's `elevateNestedPaths`
// hoists the deep `pkg@2.0.0` up to `parent/node_modules/pkg` because that
// slot is empty in `result`, inventing an intermediate-level entry that
// didn't exist in root. That entry now shadows walk-up for `parent/child`,
// which finds `pkg@2.0.0` instead of the top-level `pkg@1.0.0` — `npm ci`
// rejects with "Invalid: lock file's pkg@2.0.0 does not satisfy pkg@1.0.0".
function bug2() {
  const root = {
    name: 'workspace',
    version: '0.0.0',
    lockfileVersion: 3,
    requires: true,
    packages: {
      '': {
        name: 'workspace',
        version: '0.0.0',
        dependencies: { app: '1.0.0', pkg: '1.0.0' },
      },
      'node_modules/app': {
        ...entry('1.0.0'),
        dependencies: { parent: '1.0.0' },
      },
      'node_modules/parent': {
        ...entry('1.0.0'),
        dependencies: { wrapper: '1.0.0', child: '1.0.0' },
      },
      'node_modules/parent/node_modules/wrapper': {
        ...entry('1.0.0'),
        dependencies: { pkg: '^2.0.0' },
      },
      'node_modules/parent/node_modules/wrapper/node_modules/pkg': entry('2.0.0'),
      'node_modules/parent/node_modules/child': {
        ...entry('1.0.0'),
        dependencies: { pkg: '1.0.0' },
      },
      'node_modules/pkg': entry('1.0.0'),
    },
  };
  const pkg = {
    name: 'dist-app',
    version: '0.0.0',
    dependencies: { app: '1.0.0', pkg: '1.0.0' },
  };
  const out = stringify(root, pkg);
  const elevated = 'node_modules/parent/node_modules/pkg';
  const deep = 'node_modules/parent/node_modules/wrapper/node_modules/pkg';

  console.log('Bug 2 — over-eager elevation creates a shadowing entry');
  console.log(`  child pins  pkg @ 1.0.0   (at node_modules/parent/node_modules/child)`);
  console.log(
    `  elevated:   ${elevated}: ${out.packages[elevated] ? out.packages[elevated].version : 'absent'}` +
      `   ${out.packages[elevated] ? '  (root has no entry here — invented by Nx)' : ''}`,
  );
  console.log(
    `  original:   ${deep}: ${out.packages[deep] ? out.packages[deep].version : 'absent'}`,
  );
  console.log(
    !out.packages[elevated]
      ? '  ✅ FIXED'
      : `  ❌ npm ci would fail: walk-up from parent/child finds ${out.packages[elevated].version}, expected 1.0.0`,
  );
}

console.log('Nx version:', require('nx/package.json').version);
console.log('');
bug1();
console.log('');
bug2();

…stringifyNpmLockfile

Two cases produced lockfiles `npm ci` rejects when the root tree contains
intermediate-version shadowing:

1. The project graph dedups by `(packageName, version)`, so when the same
   version appears at multiple lockfile paths in root because an intermediate
   sibling holds a different version, only one path is emitted. The deep copy
   is missing and walk-up from the consumer hits the wrong-version sibling.

2. `elevateNestedPaths` hoisted nested entries into any empty ancestor slot.
   That invented intermediate-level entries that didn't exist in root and
   shadowed walk-up resolution for any deeper consumer of a different version.

Add `emitRootDuplicates` (after `mapSnapshots`) and `wouldShadowDescendant`
(gating empty-slot elevation). The two updated `__fixtures__` files encoded
the pre-fix layouts: the npm-hoisting fixture was missing required deep
copies of `picomatch@2.3.1`, the auxiliary-packages fixture had over-elevated
entries.
@xavier-dfns xavier-dfns requested a review from a team as a code owner May 5, 2026 14:02
@xavier-dfns xavier-dfns requested a review from leosvelperez May 5, 2026 14:02
@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

👷 Deploy request for nx-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 0f84dfa

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

👷 Deploy request for nx-dev pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 0f84dfa

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant