From fcb9cfc2744bc758a51b0246b9b6aa67d544a069 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Nison?= <mael@fb.com>
Date: Sat, 3 Nov 2018 13:43:17 -0700
Subject: [PATCH] Uses NODE_OPTIONS when spawning a process

---
 .../pkg-tests/pkg-tests-specs/sources/pnp.js  | 53 +++++++++++++++++++
 src/cli/commands/node.js                      |  4 +-
 src/util/execute-lifecycle-script.js          | 19 +++----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/packages/pkg-tests/pkg-tests-specs/sources/pnp.js b/packages/pkg-tests/pkg-tests-specs/sources/pnp.js
index ec95577320..9b469ed334 100644
--- a/packages/pkg-tests/pkg-tests-specs/sources/pnp.js
+++ b/packages/pkg-tests/pkg-tests-specs/sources/pnp.js
@@ -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`,
+          });
+        },
+      ),
+    );
   });
 };
diff --git a/src/cli/commands/node.js b/src/cli/commands/node.js
index a8c752c1a1..1fb70fd1ee 100644
--- a/src/cli/commands/node.js
+++ b/src/cli/commands/node.js
@@ -21,14 +21,16 @@ 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}`;
 
+  let nodeOptions = process.env.NODE_OPTIONS || '';
   if (await fs.exists(pnpPath)) {
-    args = ['-r', pnpPath, ...args];
+    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;
diff --git a/src/util/execute-lifecycle-script.js b/src/util/execute-lifecycle-script.js
index 82c01acae3..7401cc10e8 100644
--- a/src/util/execute-lifecycle-script.js
+++ b/src/util/execute-lifecycle-script.js
@@ -26,14 +26,6 @@ export const IGNORE_MANIFEST_KEYS: Set<string> = new Set(['readme', 'notice', 'l
 // See https://github.com/yarnpkg/yarn/issues/2286.
 const IGNORE_CONFIG_KEYS = ['lastUpdateCheck'];
 
-async function getPnpParameters(config: Config): Promise<Array<string>> {
-  if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
-    return ['-r', `${config.lockfileFolder}/${constants.PNP_FILENAME}`];
-  } else {
-    return [];
-  }
-}
-
 let wrappersFolder = null;
 
 export async function getWrappersFolder(config: Config): Promise<string> {
@@ -45,7 +37,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, {
@@ -212,8 +203,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);
@@ -227,6 +219,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));