Fixes the vscode builtins (#848)

* Fixes the vscode builtins

* Fixes tests for Windows

.yarn/versions/733f3da7.yml

@@ -0,0 +1,22 @@
+releases:
+  "@yarnpkg/cli": prerelease
+  "@yarnpkg/plugin-pnp": prerelease
+  "@yarnpkg/pnp": prerelease
+
+declined:
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-node-modules"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/pnpify"

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

@@ -1374,4 +1374,40 @@ describe(`Plug'n'Play`, () => {
     ),
     15000,
   );
+
+  test(
+    `it shouldn't break the vscode builtin resolution`,
+    makeTemporaryEnv({}, async ({path, run, source}) => {
+      // VSCode has its own layer on top of `require` to provide extra builtins
+      // to the plugins. We don't want to accidentally break this:
+      //
+      // https://github.com/microsoft/vscode/blob/dcecb9eea6158f561ee703cbcace49b84048e6e3/src/vs/workbench/api/node/extHostExtensionService.ts#L23
+
+      await run(`install`);
+
+      const tmp = await xfs.mktempPromise();
+      await xfs.writeFilePromise(ppath.join(tmp, `index.js`), `
+        const realLoad = module.constructor._load;
+
+        module.constructor._load = function (name, ...args) {
+          if (name === 'foo') {
+            return 'this works';
+          } else {
+            return realLoad.call(this, name, ...args);
+          }
+        };
+
+        require(process.argv[2]).setup();
+
+        if (require('foo') !== 'this works') {
+          throw new Error('Assertion failed');
+        }
+      `);
+
+      cp.execFileSync(`node`, [
+        npath.fromPortablePath(`${tmp}/index.js`),
+        npath.fromPortablePath(`${path}/.pnp.js`),
+      ], {encoding: `utf-8`});
+    }),
+  );
 });

packages/yarnpkg-pnp/sources/loader/applyPatch.ts

@@ -88,10 +88,17 @@ export function applyPatch(pnpapi: PnpApi, opts: ApplyPatchOptions) {
       ? opts.manager.getApiEntry(parentApiPath, true).instance
       : null;
 
+    // Requests that aren't covered by the PnP runtime goes through the
+    // parent `_load` implementation. This is required for VSCode, for example,
+    // which override `_load` to provide additional builtins to its extensions.
+
+    if (parentApi === null)
+      return originalModuleLoad(request, parent, isMain);
+
     // The 'pnpapi' name is reserved to return the PnP api currently in use
     // by the program
 
-    if (parentApi !== null && request === `pnpapi`)
+    if (request === `pnpapi`)
       return parentApi;
 
     // Request `Module._resolveFilename` (ie. `resolveRequest`) to tell us