fix(pnp): throw ERR_REQUIRE_ESM when requiring an ES Module

.github/workflows/e2e-cra-workflow.yml

@@ -30,6 +30,7 @@ jobs:
         yarn add -D eslint-config-react-app eslint
 
         yarn build
+        yarn test
 
     - name: 'Running the TypeScript integration test'
       run: |
@@ -43,5 +44,6 @@ jobs:
         yarn add -D @types/testing-library__jest-dom
 
         yarn build
+        yarn test
       if: |
         always()

.yarn/versions/f89dc89d.yml

@@ -0,0 +1,28 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/plugin-pnp": patch
+  "@yarnpkg/pnp": patch
+
+declined:
+  - "@yarnpkg/esbuild-plugin-pnp"
+  - "@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"

CHANGELOG.md

@@ -43,6 +43,7 @@ Various improvements have been made in the core to improve performance. Addition
 - The PnP filesystem now handles `read` and `readSync` using options.
 - The PnP filesystem now handles UNC paths using forward slashes.
 - The PnP filesystem now sets the proper `path` property on streams created by `createReadStream()` and obtained from zip archives.
+- The PnP runtime now throws an `ERR_REQUIRE_ESM` error when attempting to require an ES Module, matching the default Node.js behaviour.
 - Updates the PnP compatibility layer for TypeScript 4.6 Beta (it's possible we'll need to publish another patch update once the 4.6 enters stable).
 
 ### Bugfixes

packages/acceptance-tests/pkg-tests-fixtures/packages/no-deps-esm-1.0.0/index.js

@@ -0,0 +1 @@
+import 'fs'

packages/acceptance-tests/pkg-tests-fixtures/packages/no-deps-esm-1.0.0/package.json

@@ -0,0 +1,5 @@
+{
+    "name": "no-deps-esm",
+    "version": "1.0.0",
+    "type": "module"
+}

packages/acceptance-tests/pkg-tests-fixtures/packages/no-deps-mjs-1.0.0/index.mjs

@@ -0,0 +1 @@
+import 'fs'

packages/acceptance-tests/pkg-tests-fixtures/packages/no-deps-mjs-1.0.0/package.json

@@ -0,0 +1,4 @@
+{
+    "name": "no-deps-mjs",
+    "version": "1.0.0"
+}

packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts

@@ -499,4 +499,64 @@ describe(`Plug'n'Play - ESM`, () => {
       },
     ),
   );
+
+  test(
+    `it should throw ERR_REQUIRE_ESM when requiring a file with type=module`,
+    makeTemporaryEnv(
+      {
+        dependencies: {
+          'no-deps-esm': `1.0.0`,
+        },
+      },
+      {
+        pnpEnableEsmLoader: true,
+      },
+      async ({path, run, source}) => {
+        await expect(run(`install`)).resolves.toMatchObject({code: 0});
+
+        await xfs.writeFilePromise(ppath.join(path, `index.js` as Filename), `
+          try {
+            require('no-deps-esm')
+          } catch (err) {
+            console.log(err.code)
+          }
+        `);
+
+        await expect(run(`node`, `index.js`)).resolves.toMatchObject({
+          code: 0,
+          stdout: `ERR_REQUIRE_ESM\n`,
+        });
+      },
+    ),
+  );
+
+  test(
+    `it should throw ERR_REQUIRE_ESM when requiring a .mjs file`,
+    makeTemporaryEnv(
+      {
+        dependencies: {
+          'no-deps-mjs': `1.0.0`,
+        },
+      },
+      {
+        pnpEnableEsmLoader: true,
+      },
+      async ({path, run, source}) => {
+        await expect(run(`install`)).resolves.toMatchObject({code: 0});
+
+        await xfs.writeFilePromise(ppath.join(path, `index.js` as Filename), `
+          try {
+            require('no-deps-mjs/index.mjs')
+          } catch (err) {
+            console.log(err.code)
+          }
+        `);
+
+        await expect(run(`node`, `index.js`)).resolves.toMatchObject({
+          code: 0,
+          stdout: `ERR_REQUIRE_ESM\n`,
+        });
+      },
+    ),
+  );
 });

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

@@ -2087,7 +2087,7 @@ describe(`Plug'n'Play`, () => {
             return originalStatSync(__filename);
           }
 
-          console.log(require('${path}/does/not/exist.js'))
+          console.log(require('${path}/does/not/exist.cjs'))
         `);
 
         await expect(run(`node`, `./index.js`)).resolves.toMatchObject({

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

@@ -396,6 +396,21 @@ export function applyPatch(pnpapi: PnpApi, opts: ApplyPatchOptions) {
     return false;
   };
 
+  // https://github.com/nodejs/node/blob/3743406b0a44e13de491c8590386a964dbe327bb/lib/internal/modules/cjs/loader.js#L1110-L1154
+  const originalExtensionJSFunction = Module._extensions[`.js`] as (module: Module, filename: string) => void;
+  Module._extensions[`.js`] = function (module: Module, filename: string) {
+    if (filename.endsWith(`.js`)) {
+      const pkg = nodeUtils.readPackageScope(filename);
+      if (pkg && pkg.data?.type === `module`) {
+        const err = nodeUtils.ERR_REQUIRE_ESM(filename, module.parent?.filename);
+        Error.captureStackTrace(err);
+        throw err;
+      }
+    }
+
+    originalExtensionJSFunction.call(this, module, filename);
+  };
+
   // When using the ESM loader Node.js prints the following warning
   //
   // (node:14632) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time

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

@@ -1,6 +1,7 @@
 import {NativePath, npath} from '@yarnpkg/fslib';
 import fs                  from 'fs';
 import {Module}            from 'module';
+import path                from 'path';
 
 // @ts-expect-error
 const builtinModules = new Set(Module.builtinModules || Object.keys(process.binding(`natives`)));
@@ -36,3 +37,21 @@ export function readPackage(requestPath: NativePath) {
 
   return JSON.parse(fs.readFileSync(jsonPath, `utf8`));
 }
+
+// https://github.com/nodejs/node/blob/972d9218559877f7fff4bb6086afacac8933f8d1/lib/internal/errors.js#L1450-L1478
+// Our error isn't as detailed since we don't have access to acorn to check
+// if the file contains ESM syntax
+export function ERR_REQUIRE_ESM(filename: string, parentPath: string | null = null) {
+  const basename =
+    parentPath && path.basename(filename) === path.basename(parentPath)
+      ? filename
+      : path.basename(filename);
+
+  const msg =
+    `require() of ES Module ${filename}${parentPath ? ` from ${parentPath}` : ``} not supported.
+Instead change the require of ${basename} in ${parentPath} to a dynamic import() which is available in all CommonJS modules.`;
+
+  const err = new Error(msg) as Error & { code: string };
+  err.code = `ERR_REQUIRE_ESM`;
+  return err;
+}