fix(pnp) esm - support loaders importing named exports from commonjs

yarnpkg · Nov 11, 2023 · 9d2ff32 · 9d2ff32
1 parent 75ff474
commit 9d2ff32
Show file tree

Hide file tree

Showing 9 changed files with 229 additions and 84 deletions.
diff --git a/.pnp.cjs b/.pnp.cjs
diff --git a/.pnp.loader.mjs b/.pnp.loader.mjs
diff --git a/.yarn/versions/3fde6039.yml b/.yarn/versions/3fde6039.yml
@@ -0,0 +1,27 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/plugin-pnp": patch
+  "@yarnpkg/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"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
@@ -777,6 +777,34 @@ describe(`Plug'n'Play - ESM`, () => {
     ),
   );
 
+  // Tests /packages/yarnpkg-pnp/sources/esm-loader/fspatch.ts
+  it(
+    `should support loaders importing named exports from commonjs files`,
+    makeTemporaryEnv(
+      {
+        dependencies: {
+          'no-deps-exports': `1.0.0`,
+        },
+        type: `module`,
+      },
+      async ({path, run, source}) => {
+        await xfs.writeFilePromise(ppath.join(path, `loader.mjs`), `
+          import {foo} from 'no-deps-exports';
+          console.log(foo);
+        `);
+        await xfs.writeFilePromise(ppath.join(path, `index.js`), ``);
+
+        await expect(run(`install`)).resolves.toMatchObject({code: 0});
+
+        await expect(run(`node`, `--loader`, `./loader.mjs`, `./index.js`)).resolves.toMatchObject({
+          code: 0,
+          stdout: `42\n`,
+          stderr: ``,
+        });
+      },
+    ),
+  );
+
   describe(`private import mappings`, () => {
     test(
       `it should support private import mappings`,

diff --git a/packages/yarnpkg-core/sources/worker-zip/index.js b/packages/yarnpkg-core/sources/worker-zip/index.js
diff --git a/packages/yarnpkg-pnp/sources/esm-loader/built-loader.js b/packages/yarnpkg-pnp/sources/esm-loader/built-loader.js
diff --git a/packages/yarnpkg-pnp/sources/esm-loader/fspatch.ts b/packages/yarnpkg-pnp/sources/esm-loader/fspatch.ts
@@ -4,61 +4,91 @@ import {HAS_LAZY_LOADED_TRANSLATORS} from './loaderFlags';
 
 //#region ESM to CJS support
 if (!HAS_LAZY_LOADED_TRANSLATORS) {
-  /*
-  In order to import CJS files from ESM Node does some translating
-  internally[1]. This translator calls an unpatched `readFileSync`[2]
-  which itself calls an internal `tryStatSync`[3] which calls
-  `binding.fstat`[4]. A PR[5] has been made to use the monkey-patchable
-  `fs.readFileSync` but assuming that wont be merged this region of code
-  patches that final `binding.fstat` call.
-
-  1: https://github.com/nodejs/node/blob/d872aaf1cf20d5b6f56a699e2e3a64300e034269/lib/internal/modules/esm/translators.js#L177-L277
-  2: https://github.com/nodejs/node/blob/d872aaf1cf20d5b6f56a699e2e3a64300e034269/lib/internal/modules/esm/translators.js#L240
-  3: https://github.com/nodejs/node/blob/1317252dfe8824fd9cfee125d2aaa94004db2f3b/lib/fs.js#L452
-  4: https://github.com/nodejs/node/blob/1317252dfe8824fd9cfee125d2aaa94004db2f3b/lib/fs.js#L403
-  5: https://github.com/nodejs/node/pull/39513
-*/
-
   const binding = (process as any).binding(`fs`) as {
     fstat: (fd: number, useBigint: false, req: any, ctx: object) => Float64Array;
+    /**
+     * Added in https://github.com/nodejs/node/pull/48658 / v20.5.0
+     * Renamed in https://github.com/nodejs/node/pull/49593 / v20.8.0
+     */
+    readFileSync?: (path: string, flag: number) => string;
+    /**
+     * Added in https://github.com/nodejs/node/pull/49593
+     */
+    readFileUtf8?: (path: string, flag: number) => string;
   };
-  const originalfstat = binding.fstat;
-
-  // Those values must be synced with packages/yarnpkg-fslib/sources/ZipOpenFS.ts
-  const ZIP_MASK  = 0xff000000;
-  const ZIP_MAGIC = 0x2a000000;
 
-  binding.fstat = function(...args) {
-    const [fd, useBigint, req] = args;
-    if ((fd & ZIP_MASK) === ZIP_MAGIC && useBigint === false && req === undefined) {
+  const originalReadFile = binding.readFileUtf8 || binding.readFileSync;
+  if (originalReadFile) {
+    binding.readFileSync = binding.readFileUtf8 = function (...args) {
       try {
-        const stats = fs.fstatSync(fd);
-        // The reverse of this internal util
-        // https://github.com/nodejs/node/blob/8886b63cf66c29d453fdc1ece2e489dace97ae9d/lib/internal/fs/utils.js#L542-L551
-        return new Float64Array([
-          stats.dev,
-          stats.mode,
-          stats.nlink,
-          stats.uid,
-          stats.gid,
-          stats.rdev,
-          stats.blksize,
-          stats.ino,
-          stats.size,
-          stats.blocks,
-        // atime sec
-        // atime ns
-        // mtime sec
-        // mtime ns
-        // ctime sec
-        // ctime ns
-        // birthtime sec
-        // birthtime ns
-        ]);
-      } catch {}
-    }
+        return fs.readFileSync(args[0], {
+          encoding: `utf8`,
+          // @ts-expect-error - The docs says it needs to be a string but
+          // links to https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#file-system-flags
+          // which says it can be a number which matches the implementation.
+          flag: args[1],
+        });
+      } catch { }
 
-    return originalfstat.apply(this, args);
-  };
+      return originalReadFile.apply(this, args);
+    };
+  } else {
+    /*
+      In order to import CJS files from ESM Node does some translating
+      internally[1]. This translator calls an unpatched `readFileSync`[2]
+      which itself calls an internal `tryStatSync`[3] which calls
+      `binding.fstat`[4]. A PR[5] has been made to use the monkey-patchable
+      `fs.readFileSync` but assuming that wont be merged this region of code
+      patches that final `binding.fstat` call.
+
+      1: https://github.com/nodejs/node/blob/d872aaf1cf20d5b6f56a699e2e3a64300e034269/lib/internal/modules/esm/translators.js#L177-L277
+      2: https://github.com/nodejs/node/blob/d872aaf1cf20d5b6f56a699e2e3a64300e034269/lib/internal/modules/esm/translators.js#L240
+      3: https://github.com/nodejs/node/blob/1317252dfe8824fd9cfee125d2aaa94004db2f3b/lib/fs.js#L452
+      4: https://github.com/nodejs/node/blob/1317252dfe8824fd9cfee125d2aaa94004db2f3b/lib/fs.js#L403
+      5: https://github.com/nodejs/node/pull/39513
+    */
+
+    const binding = (process as any).binding(`fs`) as {
+      fstat: (fd: number, useBigint: false, req: any, ctx: object) => Float64Array;
+    };
+    const originalfstat = binding.fstat;
+
+    // Those values must be synced with packages/yarnpkg-fslib/sources/ZipOpenFS.ts
+    const ZIP_MASK = 0xff000000;
+    const ZIP_MAGIC = 0x2a000000;
+
+    binding.fstat = function (...args) {
+      const [fd, useBigint, req] = args;
+      if ((fd & ZIP_MASK) === ZIP_MAGIC && useBigint === false && req === undefined) {
+        try {
+          const stats = fs.fstatSync(fd);
+          // The reverse of this internal util
+          // https://github.com/nodejs/node/blob/8886b63cf66c29d453fdc1ece2e489dace97ae9d/lib/internal/fs/utils.js#L542-L551
+          return new Float64Array([
+            stats.dev,
+            stats.mode,
+            stats.nlink,
+            stats.uid,
+            stats.gid,
+            stats.rdev,
+            stats.blksize,
+            stats.ino,
+            stats.size,
+            stats.blocks,
+            // atime sec
+            // atime ns
+            // mtime sec
+            // mtime ns
+            // ctime sec
+            // ctime ns
+            // birthtime sec
+            // birthtime ns
+          ]);
+        } catch { }
+      }
+
+      return originalfstat.apply(this, args);
+    };
+  }
 }
 //#endregion
diff --git a/packages/yarnpkg-pnp/sources/esm-loader/loaderFlags.ts b/packages/yarnpkg-pnp/sources/esm-loader/loaderFlags.ts
@@ -3,6 +3,7 @@ const [major, minor] = process.versions.node.split(`.`).map(value => parseInt(va
 // The message switched to using an array in https://github.com/nodejs/node/pull/45348
 export const WATCH_MODE_MESSAGE_USES_ARRAYS = major > 19 || (major === 19 && minor >= 2) || (major === 18 && minor >= 13);
 
-// https://github.com/nodejs/node/pull/45659 changed the internal translators to be lazy loaded
+// https://github.com/nodejs/node/pull/45659 changed the internal translators to be lazy loaded so they use our patch.
+// https://github.com/nodejs/node/pull/48842 changed it so that our patch is loaded after the internal translators.
 // TODO: Update the version range if https://github.com/nodejs/node/pull/46425 lands.
-export const HAS_LAZY_LOADED_TRANSLATORS = major > 19 || (major === 19 && minor >= 3);
+export const HAS_LAZY_LOADED_TRANSLATORS = (major === 20 && minor < 6) || (major === 19 && minor >= 3);
diff --git a/packages/yarnpkg-pnp/sources/hook.js b/packages/yarnpkg-pnp/sources/hook.js