fix(pnp): load the entrypoint as CJS if it isn't ESM (#3832)

* fix(pnp): allow ESM loader to load arbitrary file extensions as CJS

* fix(pnp): disallow extensionless imports in ESM loader

* chore: lazy compute `isMain`

* test(pnp): ensure a consistent non-zero exit code when extensionless import fails

Co-authored-by: merceyz <merceyz@users.noreply.github.com>

.yarn/versions/32961798.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"

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

@@ -194,7 +194,25 @@ describe(`Plug'n'Play - ESM`, () => {
   );
 
   test(
-    `it should not allow unknown extensions`,
+    `it should load commonjs with an unknown extension`,
+    makeTemporaryEnv(
+      {
+      },
+      async ({path, run, source}) => {
+        await xfs.writeFilePromise(ppath.join(path, `index.ts` as Filename), `console.log(typeof require === 'undefined')`);
+
+        await expect(run(`install`)).resolves.toMatchObject({code: 0});
+
+        await expect(run(`node`, `./index.ts`)).resolves.toMatchObject({
+          code: 0,
+          stdout: `false\n`,
+        });
+      },
+    ),
+  );
+
+  test(
+    `it should not allow unknown extensions with {type: "module"}`,
     makeTemporaryEnv(
       {
         type: `module`,
@@ -214,7 +232,7 @@ describe(`Plug'n'Play - ESM`, () => {
 
   // Tests https://github.com/nodejs/node/issues/33226
   test(
-    `it should not load extensionless commonjs files as ESM`,
+    `it should load extensionless commonjs files as an entrypoint`,
     makeTemporaryEnv(
       { },
       {
@@ -233,6 +251,49 @@ describe(`Plug'n'Play - ESM`, () => {
     ),
   );
 
+  test(
+    `it should not allow extensionless commonjs imports`,
+    makeTemporaryEnv(
+      { },
+      {
+        pnpEnableEsmLoader: true,
+      },
+      async ({path, run, source}) => {
+        await xfs.writeFilePromise(ppath.join(path, `index.mjs` as Filename), `import bin from './cjs-bin';\nconsole.log(bin)`);
+        await xfs.writeFilePromise(ppath.join(path, `cjs-bin` as Filename), `module.exports = {foo: 'bar'}`);
+
+        await expect(run(`install`)).resolves.toMatchObject({code: 0});
+
+        await expect(run(`node`, `./index.mjs`)).rejects.toMatchObject({
+          code: 1,
+          stderr: expect.stringContaining(`Unknown file extension`),
+        });
+      },
+    ),
+  );
+
+  test(
+    `it should not allow extensionless files with {"type": "module"}`,
+    makeTemporaryEnv(
+      {
+        type: `module`,
+      },
+      {
+        pnpEnableEsmLoader: true,
+      },
+      async ({path, run, source}) => {
+        await xfs.writeFilePromise(ppath.join(path, `index` as Filename), ``);
+
+        await expect(run(`install`)).resolves.toMatchObject({code: 0});
+
+        await expect(run(`node`, `./index`)).rejects.toMatchObject({
+          code: 1,
+          stderr: expect.stringContaining(`Unknown file extension`),
+        });
+      },
+    ),
+  );
+
   test(
     `it should support ESM binaries`,
     makeTemporaryEnv(

packages/yarnpkg-pnp/sources/esm-loader/loaderUtils.ts

@@ -48,16 +48,27 @@ export function getFileFormat(filepath: string): string | null {
         `Unknown file extension ".json" for ${filepath}`,
       );
     }
-    // Matching files without extensions deviates from Node's default
-    // behaviour but is a fix for https://github.com/nodejs/node/issues/33226
-    case ``:
     case `.js`: {
       const pkg = nodeUtils.readPackageScope(filepath);
-      if (pkg) {
-        return pkg.data.type ?? `commonjs`;
-      }
+      // assume CJS for files outside of a package boundary
+      if (!pkg)
+        return `commonjs`;
+      return pkg.data.type ?? `commonjs`;
+    }
+    // Matching files beyond those handled above deviates from Node's default
+    // --experimental-loader behavior but is required to work around
+    // https://github.com/nodejs/node/issues/33226
+    default: {
+      const isMain = process.argv[1] === filepath;
+      if (!isMain)
+        return null;
+      const pkg = nodeUtils.readPackageScope(filepath);
+      if (!pkg)
+        return `commonjs`;
+      // prevent extensions beyond .mjs or .js from loading as ESM
+      if (pkg.data.type === `module`)
+        return null;
+      return pkg.data.type ?? `commonjs`;
     }
   }
-
-  return null;
 }