Skip to content

Commit aef9453

Browse files
kaigritunFrozenPandaz
authored andcommitted
fix(js): guard against undefined closest node in rehoistNodes (#34347)
## Current Behavior When running `@nx/js:prune-lockfile` on a monorepo with transitive dependencies that have multiple versions where neither version is reachable from a direct dependency in package.json, the executor throws: ``` NX An error occurred while creating pruned lockfile Original error: Cannot read properties of undefined (reading 'name') TypeError: Cannot read properties of undefined (reading 'name') at switchNodeToHoisted (node_modules/nx/src/plugins/js/lock-file/project-graph-pruning.js:165:31) ``` ## Expected Behavior The lockfile pruning should complete without crashing, even when some transitive dependencies cannot be traced back to a direct dependency. ## Root Cause In `rehoistNodes()`, when there are multiple nested nodes for a package, the code finds the "closest" node by computing `pathLengthToIncoming()` for each. However, when none of the nested nodes have a path to any direct dependency in package.json, `pathLengthToIncoming()` returns `undefined` for all of them. Since `undefined < Infinity` is `false` in JavaScript, `closest` remains `undefined`, and then `switchNodeToHoisted(undefined, ...)` crashes. ## Fix Add a guard to only call `switchNodeToHoisted()` when a closest node was actually found: ```typescript if (closest) { switchNodeToHoisted(closest, builder, invBuilder); } ``` This allows the pruning to continue - the nested nodes simply won't be rehoisted if no closest node can be determined. ## Related Issue Fixes #34322 ## Test Added Added a unit test that verifies `rehoistNodes()` doesn't crash when nested nodes have no path to package.json dependencies. (cherry picked from commit b46de60)
1 parent 1b1bcb7 commit aef9453

File tree

2 files changed

+45
-1
lines changed

2 files changed

+45
-1
lines changed

packages/nx/src/plugins/js/lock-file/project-graph-pruning.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,48 @@ describe('project-graph-pruning', () => {
424424
'4.17.20'
425425
);
426426
});
427+
428+
it('should not crash when nested nodes have no path to package.json deps', () => {
429+
// This tests the fix for https://github.com/nrwl/nx/issues/34322
430+
// When transitive dependencies have multiple versions where neither version
431+
// is reachable from a direct dependency, the code should not crash.
432+
433+
// Setup: Add multiple nested versions that are NOT in packageJsonDeps
434+
builder.addExternalNode({
435+
type: 'npm',
436+
name: 'npm:lodash@4.17.20',
437+
data: {
438+
packageName: 'lodash',
439+
version: '4.17.20',
440+
},
441+
});
442+
builder.addExternalNode({
443+
type: 'npm',
444+
name: 'npm:lodash@4.17.19',
445+
data: {
446+
packageName: 'lodash',
447+
version: '4.17.19',
448+
},
449+
});
450+
// Connect them as nested dependencies
451+
builder.addStaticDependency('npm:lodash@4.17.20', 'npm:lodash@4.17.19');
452+
453+
// packageJsonDeps does NOT include lodash, so pathLengthToIncoming
454+
// will return undefined for both nested nodes
455+
const packageJsonDeps = {
456+
typescript: '5.0.0',
457+
};
458+
459+
// Should NOT throw "Cannot read properties of undefined (reading 'name')"
460+
expect(() => rehoistNodes(graph, packageJsonDeps, builder)).not.toThrow();
461+
462+
const result = builder.getUpdatedProjectGraph();
463+
// Nested nodes should remain as-is (not rehoisted)
464+
expect(result.externalNodes?.['npm:lodash@4.17.20']).toBeDefined();
465+
expect(result.externalNodes?.['npm:lodash@4.17.19']).toBeDefined();
466+
// npm:lodash should NOT be created since we couldn't determine which to hoist
467+
expect(result.externalNodes?.['npm:lodash']).toBeUndefined();
468+
});
427469
});
428470

429471
describe('pruneProjectGraph', () => {

packages/nx/src/plugins/js/lock-file/project-graph-pruning.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,9 @@ export function rehoistNodes(
247247
closest = node;
248248
}
249249
});
250-
switchNodeToHoisted(closest, builder, invBuilder);
250+
if (closest) {
251+
switchNodeToHoisted(closest, builder, invBuilder);
252+
}
251253
}
252254
});
253255
}

0 commit comments

Comments
 (0)