Skip to content

Commit 74311e7

Browse files
committed
fix(core): omit peer dependencies when installing packages to a temp dir (#36295)
`installPackageToTmp` (behind devkit's `ensurePackage`) fetches a package into an **empty** temp directory. npm and bun **auto-install that package's peer dependencies** there. So a loose peer range — e.g. `@phenomnomnominal/tsquery`'s `typescript: >3.0.0` — pulls the **newest** major, TypeScript 7, into the temp dir. tsquery reads `ts.SyntaxKind` at module load, which TS 7 no longer exposes as a top-level CommonJS export, so it crashes: ``` NX Cannot convert undefined or null to object at Object.keys (<anonymous>) at .../@phenomnomnominal/tsquery/dist/src/syntax-kind.js:8:27 ``` Peer dependencies are the **host's** responsibility, not something a throwaway fetch should decide. `ensurePackage` already loads the package from the temp dir with the workspace's `node_modules` on `NODE_PATH`, so its peers resolve from the workspace — the correct provider. This omits peers from the temp install so nothing incompatible gets pulled: - **npm** / **bun**: `--omit=peer` - **pnpm**: `--config.auto-install-peers=false` - **Yarn** (classic & Berry): never auto-installs peers, so no flag needed Verified locally: with `--omit=peer` the temp dir no longer contains TypeScript 7, and loading the package resolves `typescript@6.0.3` from the workspace via `NODE_PATH`. Unit tests cover the emitted install command for every package manager. Hardening for the `ensurePackage` path, surfaced while investigating the TypeScript 7 / tsquery crash. Complements bounding tsquery's `typescript` peer range at the source. <!-- polygraph-session-start --> --- [View session information ↗](https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Migrate-nrwl-repos-to-nx-23.1.0-rc.0-b8c94700) <!-- polygraph-session-end --> (cherry picked from commit edc9212)
1 parent 23d2a55 commit 74311e7

2 files changed

Lines changed: 80 additions & 4 deletions

File tree

packages/nx/src/utils/package-json.spec.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,71 @@ describe('installPackageToTmp', () => {
9898

9999
expect(execSyncSpy).toHaveBeenCalledTimes(1);
100100
expect(execSyncSpy.mock.calls[0][0]).toBe(
101-
'pnpm add -Dw nx@latest --ignore-scripts'
101+
'pnpm add -Dw nx@latest --config.auto-install-peers=false --ignore-scripts'
102102
);
103103

104104
cleanup();
105105
});
106+
107+
it('should omit peer dependencies so peers resolve from the workspace, not the temp dir', () => {
108+
const tempDir = mkdtempSync(join(tmpdir(), 'nx-install-test-'));
109+
const cleanup = jest.fn(() =>
110+
rmSync(tempDir, { recursive: true, force: true })
111+
);
112+
jest.spyOn(pacakgeManager, 'createTempNpmDirectory').mockReturnValue({
113+
dir: tempDir,
114+
cleanup,
115+
});
116+
jest
117+
.spyOn(pacakgeManager, 'getPackageManagerVersion')
118+
.mockReturnValue('10.0.0');
119+
jest.spyOn(pacakgeManager, 'getPackageManagerCommand').mockReturnValue({
120+
addDev: 'npm install -D',
121+
ignoreScriptsFlag: '--ignore-scripts',
122+
} as any);
123+
const execSyncSpy = jest
124+
.spyOn(childProcess, 'execSync')
125+
.mockReturnValue('' as any);
126+
127+
// npm: peers are omitted via `--omit=peer`
128+
installPackageToTmp('@nx/cypress', '1.0.0', 'npm');
129+
expect(execSyncSpy.mock.calls[0][0]).toBe(
130+
'npm install -D @nx/cypress@1.0.0 --omit=peer --ignore-scripts'
131+
);
132+
133+
// bun: also accepts `--omit=peer`
134+
execSyncSpy.mockClear();
135+
jest.spyOn(pacakgeManager, 'getPackageManagerCommand').mockReturnValue({
136+
addDev: 'bun add -D',
137+
ignoreScriptsFlag: undefined,
138+
} as any);
139+
installPackageToTmp('@nx/cypress', '1.0.0', 'bun');
140+
expect(execSyncSpy.mock.calls[0][0]).toBe(
141+
'bun add -D @nx/cypress@1.0.0 --omit=peer'
142+
);
143+
144+
// pnpm: peers are omitted by disabling auto-install
145+
execSyncSpy.mockClear();
146+
jest.spyOn(pacakgeManager, 'getPackageManagerCommand').mockReturnValue({
147+
addDev: 'pnpm add -Dw',
148+
ignoreScriptsFlag: '--ignore-scripts',
149+
} as any);
150+
installPackageToTmp('@nx/cypress', '1.0.0', 'pnpm');
151+
expect(execSyncSpy.mock.calls[0][0]).toBe(
152+
'pnpm add -Dw @nx/cypress@1.0.0 --config.auto-install-peers=false --ignore-scripts'
153+
);
154+
155+
// yarn: Berry does not auto-install peers, so no flag is added
156+
execSyncSpy.mockClear();
157+
jest.spyOn(pacakgeManager, 'getPackageManagerCommand').mockReturnValue({
158+
addDev: 'yarn add -D',
159+
ignoreScriptsFlag: undefined,
160+
} as any);
161+
installPackageToTmp('@nx/cypress', '1.0.0', 'yarn');
162+
expect(execSyncSpy.mock.calls[0][0]).toBe('yarn add -D @nx/cypress@1.0.0');
163+
164+
cleanup();
165+
});
106166
});
107167

108168
describe('readTargetsFromPackageJson', () => {

packages/nx/src/utils/package-json.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,25 @@ function preparePackageInstallation(
384384
// it into the temp dir, so the `-w` here resolves to the temp dir.
385385
const pmCommands = getPackageManagerCommand(packageManager);
386386
const preInstallCommand = pmCommands.preInstall;
387-
const installCommand = `${pmCommands.addDev} ${pkg}@${requiredVersion} ${
388-
pmCommands.ignoreScriptsFlag ?? ''
389-
}`;
387+
388+
// Omit peer dependencies from the temp install. `ensurePackage` puts the
389+
// workspace's `node_modules` on `NODE_PATH`, so a loaded package resolves its
390+
// peers from the workspace instead of pulling its own (possibly incompatible)
391+
// copies into the temp dir.
392+
const omitPeerDependenciesFlag =
393+
packageManager === 'npm' || packageManager === 'bun'
394+
? '--omit=peer'
395+
: packageManager === 'pnpm'
396+
? '--config.auto-install-peers=false'
397+
: '';
398+
const installCommand = [
399+
pmCommands.addDev,
400+
`${pkg}@${requiredVersion}`,
401+
omitPeerDependenciesFlag,
402+
pmCommands.ignoreScriptsFlag,
403+
]
404+
.filter(Boolean)
405+
.join(' ');
390406

391407
const execOptions = {
392408
cwd: tempDir,

0 commit comments

Comments
 (0)