Skip to content

Commit fcb9cfc

Browse files
author
Maël Nison
committed
Uses NODE_OPTIONS when spawning a process
1 parent 9783400 commit fcb9cfc

File tree

3 files changed

+64
-12
lines changed

3 files changed

+64
-12
lines changed

packages/pkg-tests/pkg-tests-specs/sources/pnp.js

+53
Original file line numberDiff line numberDiff line change
@@ -1340,5 +1340,58 @@ module.exports = makeTemporaryEnv => {
13401340
},
13411341
),
13421342
);
1343+
1344+
test(
1345+
`it should not break spawning new Node processes ('node' command)`,
1346+
makeTemporaryEnv(
1347+
{
1348+
dependencies: {[`no-deps`]: `1.0.0`},
1349+
},
1350+
{plugNPlay: true},
1351+
async ({path, run, source}) => {
1352+
await run(`install`);
1353+
1354+
await writeFile(`${path}/script.js`, `console.log(JSON.stringify(require('no-deps')))`);
1355+
1356+
await expect(
1357+
source(
1358+
`JSON.parse(require('child_process').execFileSync(process.execPath, [${JSON.stringify(
1359+
`${path}/script.js`,
1360+
)}]).toString())`,
1361+
),
1362+
).resolves.toMatchObject({
1363+
name: `no-deps`,
1364+
version: `1.0.0`,
1365+
});
1366+
},
1367+
),
1368+
);
1369+
1370+
test(
1371+
`it should not break spawning new Node processes ('run' command)`,
1372+
makeTemporaryEnv(
1373+
{
1374+
dependencies: {[`no-deps`]: `1.0.0`},
1375+
scripts: {[`script`]: `node main.js`},
1376+
},
1377+
{plugNPlay: true},
1378+
async ({path, run, source}) => {
1379+
await run(`install`);
1380+
1381+
await writeFile(`${path}/sub.js`, `console.log(JSON.stringify(require('no-deps')))`);
1382+
await writeFile(
1383+
`${path}/main.js`,
1384+
`console.log(require('child_process').execFileSync(process.execPath, [${JSON.stringify(
1385+
`${path}/sub.js`,
1386+
)}]).toString())`,
1387+
);
1388+
1389+
expect(JSON.parse((await run(`run`, `script`)).stdout)).toMatchObject({
1390+
name: `no-deps`,
1391+
version: `1.0.0`,
1392+
});
1393+
},
1394+
),
1395+
);
13431396
});
13441397
};

src/cli/commands/node.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,16 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean {
2121
export async function run(config: Config, reporter: Reporter, flags: Object, args: Array<string>): Promise<void> {
2222
const pnpPath = `${config.lockfileFolder}/${PNP_FILENAME}`;
2323

24+
let nodeOptions = process.env.NODE_OPTIONS || '';
2425
if (await fs.exists(pnpPath)) {
25-
args = ['-r', pnpPath, ...args];
26+
nodeOptions += ` --require ${pnpPath}`;
2627
}
2728

2829
try {
2930
await child.spawn(NODE_BIN_PATH, args, {
3031
stdio: 'inherit',
3132
cwd: flags.into || config.cwd,
33+
env: {...process.env, NODE_OPTIONS: nodeOptions},
3234
});
3335
} catch (err) {
3436
throw err;

src/util/execute-lifecycle-script.js

+8-11
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,6 @@ export const IGNORE_MANIFEST_KEYS: Set<string> = new Set(['readme', 'notice', 'l
2626
// See https://github.com/yarnpkg/yarn/issues/2286.
2727
const IGNORE_CONFIG_KEYS = ['lastUpdateCheck'];
2828

29-
async function getPnpParameters(config: Config): Promise<Array<string>> {
30-
if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
31-
return ['-r', `${config.lockfileFolder}/${constants.PNP_FILENAME}`];
32-
} else {
33-
return [];
34-
}
35-
}
36-
3729
let wrappersFolder = null;
3830

3931
export async function getWrappersFolder(config: Config): Promise<string> {
@@ -45,7 +37,6 @@ export async function getWrappersFolder(config: Config): Promise<string> {
4537

4638
await makePortableProxyScript(process.execPath, wrappersFolder, {
4739
proxyBasename: 'node',
48-
prependArguments: [...(await getPnpParameters(config))],
4940
});
5041

5142
await makePortableProxyScript(process.execPath, wrappersFolder, {
@@ -212,8 +203,9 @@ export async function makeEnv(
212203
}
213204
}
214205

215-
if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
216-
const pnpApi = dynamicRequire(`${config.lockfileFolder}/${constants.PNP_FILENAME}`);
206+
const pnpFile = `${config.lockfileFolder}/${constants.PNP_FILENAME}`;
207+
if (await fs.exists(pnpFile)) {
208+
const pnpApi = dynamicRequire(pnpFile);
217209

218210
const packageLocator = pnpApi.findPackageLocator(`${config.cwd}/`);
219211
const packageInformation = pnpApi.getPackageInformation(packageLocator);
@@ -227,6 +219,11 @@ export async function makeEnv(
227219

228220
pathParts.unshift(`${dependencyInformation.packageLocation}/.bin`);
229221
}
222+
223+
// Note that NODE_OPTIONS doesn't support any style of quoting its arguments at the moment
224+
// For this reason, it won't work if the user has a space inside its $PATH
225+
env.NODE_OPTIONS = env.NODE_OPTIONS || '';
226+
env.NODE_OPTIONS += ` --require ${pnpFile}`;
230227
}
231228

232229
pathParts.unshift(await getWrappersFolder(config));

0 commit comments

Comments
 (0)