fix(plugin-pnp): unset NODE_OPTIONS when preparing the environment after switching linkers

.github/workflows/e2e-nm-berry-workflow.yml

@@ -4,13 +4,6 @@ on:
   push:
     branches:
     - master
-    paths:
-    - .github/actions/prepare/action.yml
-    - .github/workflows/e2e-nm-berry-workflow.yml
-    - scripts/e2e-setup-ci.sh
-    - packages/yarnpkg-nm/sources/hoist.ts
-    - packages/yarnpkg-nm/sources/buildNodeModulesTree.ts
-    - packages/plugin-nm/sources/NodeModulesLinker.ts
   pull_request:
     paths:
     - .github/actions/prepare/action.yml

.yarn/versions/89a69c6d.yml

@@ -0,0 +1,23 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/plugin-pnp": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"

packages/acceptance-tests/pkg-tests-specs/sources/features/installArtifactCleanup.test.ts

@@ -1,4 +1,5 @@
-import {Filename, PortablePath, xfs} from '@yarnpkg/fslib';
+import {Filename, PortablePath, npath, ppath, xfs} from '@yarnpkg/fslib';
+import {pathToFileURL}                             from 'url';
 
 const lsStore = async (path: PortablePath) => {
   return await xfs.readdirPromise(`${path}/${Filename.nodeModules}/.store` as PortablePath);
@@ -66,6 +67,29 @@ describe(`Install Artifact Cleanup`, () => {
 
             await expect(xfs.existsPromise(`${path}/.yarn/unplugged` as PortablePath)).resolves.toStrictEqual(false);
           }));
+
+          it(`should remove the PnP flags from NODE_OPTIONS in build scripts after switching to the ${linker} linker`, makeTemporaryEnv({
+            dependencies: {
+              [`no-deps-scripted`]: `1.0.0`,
+            },
+          }, {
+            pnpEnableEsmLoader: true,
+          }, async ({path, run, source}) => {
+            await run(`install`);
+
+            const hook = `--require ${JSON.stringify(npath.fromPortablePath(ppath.join(path, Filename.pnpCjs)))}`;
+            const esmLoader = `--experimental-loader ${JSON.stringify(pathToFileURL(npath.fromPortablePath(ppath.join(path, Filename.pnpEsmLoader))).href)}`;
+
+            const env = {
+              NODE_OPTIONS: `${hook} ${esmLoader}`,
+            };
+
+            await run(`config`, `set`, `nodeLinker`, linker, {env});
+
+            await expect(run(`install`, `--inline-builds`, {env})).resolves.toMatchObject({
+              stdout: expect.stringContaining(`STDOUT preinstall out`),
+            });
+          }));
         });
       }
     });

packages/plugin-pnp/sources/index.ts

@@ -25,24 +25,32 @@ export const quotePathIfNeeded = (path: string) => {
 };
 
 async function setupScriptEnvironment(project: Project, env: {[key: string]: string}, makePathWrapper: (name: string, argv0: string, args: Array<string>) => Promise<void>) {
+  // We still support .pnp.js files to improve multi-project compatibility.
+  // TODO: Drop the question mark in the RegExp after .pnp.js files stop being used.
+  // TODO: Support `-r` as an alias for `--require` (in all packages)
+  const pnpRegularExpression = /\s*--require\s+\S*\.pnp\.c?js\s*/g;
+  const esmLoaderExpression = /\s*--experimental-loader\s+\S*\.pnp\.loader\.mjs\s*/;
+
+  const nodeOptions = (env.NODE_OPTIONS ?? ``)
+    .replace(pnpRegularExpression, ` `)
+    .replace(esmLoaderExpression, ` `)
+    .trim();
+
+  // We remove the PnP hook from NODE_OPTIONS because the process can have
+  // NODE_OPTIONS set while changing linkers, which affects build scripts.
+  if (project.configuration.get(`nodeLinker`) !== `pnp`) {
+    env.NODE_OPTIONS = nodeOptions;
+    return;
+  }
+
   const pnpPath = getPnpPath(project);
   let pnpRequire = `--require ${quotePathIfNeeded(npath.fromPortablePath(pnpPath.cjs))}`;
 
   if (xfs.existsSync(pnpPath.esmLoader))
     pnpRequire = `${pnpRequire} --experimental-loader ${pathToFileURL(npath.fromPortablePath(pnpPath.esmLoader)).href}`;
 
   if (xfs.existsSync(pnpPath.cjs)) {
-    let nodeOptions = env.NODE_OPTIONS || ``;
-
-    // We still support .pnp.js files to improve multi-project compatibility.
-    // TODO: Drop the question mark in the RegExp after .pnp.js files stop being used.
-    const pnpRegularExpression = /\s*--require\s+\S*\.pnp\.c?js\s*/g;
-    const esmLoaderExpression = /\s*--experimental-loader\s+\S*\.pnp\.loader\.mjs\s*/;
-    nodeOptions = nodeOptions.replace(pnpRegularExpression, ` `).replace(esmLoaderExpression, ` `).trim();
-
-    nodeOptions = nodeOptions ? `${pnpRequire} ${nodeOptions}` : pnpRequire;
-
-    env.NODE_OPTIONS = nodeOptions;
+    env.NODE_OPTIONS = nodeOptions ? `${pnpRequire} ${nodeOptions}` : pnpRequire;
   }
 }
 

scripts/setup-ts-execution.js

@@ -8,7 +8,9 @@ const zlib = require(`zlib`);
 
 // Needed by the worker spawned by esbuild
 if (process.versions.pnp)
-  process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS || ``} -r ${JSON.stringify(require.resolve(`pnpapi`))}`;
+  // Unquoted because Yarn doesn't support it quoted yet
+  // TODO: make Yarn support quoted PnP requires in NODE_OPTIONS
+  process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS || ``} --require ${require.resolve(`pnpapi`)}`;
 
 const resolveVirtual = process.versions.pnp
   ? require(`pnpapi`).resolveVirtual