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

[kherock]

What's the problem this PR addresses?
Resolves #3782.

How did you fix it?
I carefully noted differences in how Node behaves with or without --experimental-loader with respect to loading a CJS entrypoint. The goal of this PR is to preserve the behavior that Node has when no --experimental-loader flag is passed to the entrypoint script. The behavior introduced in #3667 is missing the following:

• extensionless files should fail to load when the package has { "type": "module" } in package.json
  • more precisely, any extension outside of the set [.js, .cjs, .mjs] (and [.json, .wasm] under certain circumstances) should fail to load with { "type": "module" }
• conversely (and only when the file is an entrypoint), when package.json does not have { "type": "module" }, any non-.mjs file should load as CJS.

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[kherock]

I do want to point out that this doesn't resolve some bad behavior that won't be possible to fix without an implementation of this comment:

nodejs/node#33226 (comment)

If/when this lands, we can restore completely correct behavior by gating off my default case with if (isMain) { ... }. As it stands, an ESM file could do something like this when run with yarn node:

/* project has { "type": "module" } */
import m from './some-cjs-bin-script'; // should throw with ERR_UNKNOWN_FILE_EXTENSION ""

console.log(m); // [Module: null prototype] ...

[merceyz]

You can use parentURL to check if it's the main module its resolving for, Node itself uses it for that here https://github.com/nodejs/node/blob/36e2ffe6dc379043be96ebfd889ccd1a3e38aa84/lib/internal/modules/esm/resolve.js#L994

[kherock]

It doesn't look like parentURL is available to me in the context object like that code suggests:

$ yarn node entrypoint.mjs
[Arguments] {
  '0': 'file:///Users/herockk/Workspaces/yarn-esm-loader-entrypoint/entrypoint.mjs',
  '1': { format: undefined },
  '2': [AsyncFunction: defaultLoad]
}
[Arguments] {
  '0': 'file:///Users/herockk/Workspaces/yarn-esm-loader-entrypoint/commonjs',
  '1': { format: undefined },
  '2': [AsyncFunction: defaultLoad]
}
[Arguments] {
  '0': 'file:///Users/herockk/Workspaces/yarn-esm-loader-entrypoint/test.js',
  '1': { format: undefined },
  '2': [AsyncFunction: defaultLoad]
}

However, this approach looks viable to me: https://twitter.com/Rich_Harris/status/1355289863130673153

In my unix environments, the following seems to work

function getFileFormat(filepath: string): string | null {
  const isMain = process.argv[1] === filepath;
  // ...
}

[merceyz]

It's only there for the load hook but yeah, checking process.argv[1] would work as well

[kherock]

It's only there for the load hook but yeah, checking process.argv[1] would work as well

Interesting, those logs were actually produced with console.log(arguments) as the first line in the load hook. I'm running Node v16.13.1 It's apparently there for the resolve hook. Regardless, the other approach seems to be working, it's been pushed.

[merceyz]

LGTM, thanks for fixing this 👍

Tests are failing

[kherock]

Tests are failing because the import causes an unhandled promise rejection, which is associated with exit code 0 in Node <15

[andreialecu]

It appears this might not be the complete fix for this problem. I've been suddenly getting this error with Yarn canary during the installation and packing of a git dependency. Ref: https://github.com/andreialecu/nestjs-sentry/blob/aa/refactor/package.json#L22

➤ YN0000: ➤ YN0000: TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension "" for /tmp/xfs-dae6297b/node_modules/typescript/bin/tsc
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at new NodeError (node:internal/errors:371:5)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at defaultGetFormat (node:internal/modules/esm/get_format:71:15)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at getFormat$1 (file:///home/circleci/project/.pnp.loader.mjs:156:10)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at Loader.getFormat (node:internal/modules/esm/loader:105:42)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at Loader.getModuleJob (node:internal/modules/esm/loader:243:31)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at async Loader.import (node:internal/modules/esm/loader:177:17)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at async Object.loadESM (node:internal/process/esm_loader:68:5)
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR     at async handleMainPromise (node:internal/modules/run_main:63:12) {
➤ YN0000: ➤ YN0000: │ │ /tmp/xfs-dae6297b STDERR   code: 'ERR_UNKNOWN_FILE_EXTENSION'

This is a regression, because reverting to yarn 3.1.1 fixes it.

I also tried yarn set version from sources which I think should include this PR, but the error above still occurs.

Note that the package.json actually specifies the npm package manager for dealing with the repo. And this is not supposed to go through pnp at all I think.

More context at: https://discord.com/channels/226791405589233664/654372321225605128/917853587642671154

[kherock]

Make sure that .pnp.loader.mjs is up-to-date in the environment that is running the install. You're probably seeing this despite the use of npm because the runtime loading the tsc script has --experimental-loader in its NODE_OPTIONS env variable. Regardless, I tried

yarn add @ntegral/nestjs-sentry@andreialecu/nestjs-sentry

with 3.2.0-rc.7 and it exited successfully.

[andreialecu]

Here's a repro:

corepack enable npm
cd `mktemp -d`
yarn init -2
yarn set version from sources
yarn config set pnpEnableEsmLoader true
yarn
NODE_OPTIONS="--loader $PWD/.pnp.loader.mjs" yarn add test@andreialecu/nestjs-sentry#aa/refactor

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension "" for /private/var/folders/9p/k1yqxx0d7rn1nlztg_wm7sbw0000gn/T/xfs-1ff1050a/node_modules/typescript/bin/tsc
    at new NodeError (node:internal/errors:371:5)

Note that in my actual project I'm not adding NODE_OPTIONS="--loader $PWD/.pnp.loader.mjs" manually. Yarn does that itself for some reason which is probably related to heuristics where it finds that one of my dependencies needs it.

[andreialecu]

Above issue still exists on latest from sources build from today (December 22).

Opened #3900