Skip to content

Commit

Permalink
Uses NODE_OPTIONS when spawning a process
Browse files Browse the repository at this point in the history
  • Loading branch information
Maël Nison committed Nov 3, 2018
1 parent 9783400 commit 2f98cf6
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 7 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@
"yarn": "./bin/yarn.js",
"yarnpkg": "./bin/yarn.js"
},
"scripts": {
"scripts": {
"env": "env",
"build": "gulp build",
"build-bundle": "node ./scripts/build-webpack.js",
"build-chocolatey": "powershell ./scripts/build-chocolatey.ps1",
Expand Down
53 changes: 53 additions & 0 deletions packages/pkg-tests/pkg-tests-specs/sources/pnp.js
Original file line number Diff line number Diff line change
Expand Up @@ -1340,5 +1340,58 @@ module.exports = makeTemporaryEnv => {
},
),
);

test(
`it should not break spawning new Node processes ('node' command)`,
makeTemporaryEnv(
{
dependencies: {[`no-deps`]: `1.0.0`},
},
{plugNPlay: true},
async ({path, run, source}) => {
await run(`install`);

await writeFile(`${path}/script.js`, `console.log(JSON.stringify(require('no-deps')))`);

await expect(
source(
`JSON.parse(require('child_process').execFileSync(process.execPath, [${JSON.stringify(
`${path}/script.js`,
)}]).toString())`,
),
).resolves.toMatchObject({
name: `no-deps`,
version: `1.0.0`,
});
},
),
);

test(
`it should not break spawning new Node processes ('run' command)`,
makeTemporaryEnv(
{
dependencies: {[`no-deps`]: `1.0.0`},
scripts: {[`script`]: `node main.js`},
},
{plugNPlay: true},
async ({path, run, source}) => {
await run(`install`);

await writeFile(`${path}/sub.js`, `console.log(JSON.stringify(require('no-deps')))`);
await writeFile(
`${path}/main.js`,
`console.log(require('child_process').execFileSync(process.execPath, [${JSON.stringify(
`${path}/sub.js`,
)}]).toString())`,
);

expect(JSON.parse((await run(`run`, `script`)).stdout)).toMatchObject({
name: `no-deps`,
version: `1.0.0`,
});
},
),
);
});
};
7 changes: 4 additions & 3 deletions src/cli/commands/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean {
export async function run(config: Config, reporter: Reporter, flags: Object, args: Array<string>): Promise<void> {
const pnpPath = `${config.lockfileFolder}/${PNP_FILENAME}`;

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

try {
await child.spawn(NODE_BIN_PATH, args, {
stdio: 'inherit',
cwd: flags.into || config.cwd,
env: {... process.env, NODE_OPTIONS: nodeOptions},
});
} catch (err) {
throw err;
Expand Down
11 changes: 8 additions & 3 deletions src/util/execute-lifecycle-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export async function getWrappersFolder(config: Config): Promise<string> {

await makePortableProxyScript(process.execPath, wrappersFolder, {
proxyBasename: 'node',
prependArguments: [...(await getPnpParameters(config))],
});

await makePortableProxyScript(process.execPath, wrappersFolder, {
Expand Down Expand Up @@ -212,8 +211,9 @@ export async function makeEnv(
}
}

if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
const pnpApi = dynamicRequire(`${config.lockfileFolder}/${constants.PNP_FILENAME}`);
const pnpFile = `${config.lockfileFolder}/${constants.PNP_FILENAME}`;
if (await fs.exists(pnpFile)) {
const pnpApi = dynamicRequire(pnpFile);

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

pathParts.unshift(`${dependencyInformation.packageLocation}/.bin`);
}

// Note that NODE_OPTIONS doesn't support any style of quoting its arguments at the moment
// For this reason, it won't work if the user has a space inside its $PATH
env.NODE_OPTIONS = env.NODE_OPTIONS || '';
env.NODE_OPTIONS += ` --require ${pnpFile}`;
}

pathParts.unshift(await getWrappersFolder(config));
Expand Down

0 comments on commit 2f98cf6

Please sign in to comment.