testdouble / quibble

Makes it easy to replace require'd dependencies.
94 stars 25 forks source link

`require` of package with CJS `exports.require` and ESM `main` fails with `ERR_REQUIRE_ESM` after any quibble #110

Open lukekarrys opened 6 months ago

lukekarrys commented 6 months ago

A dependency with type: "module" and conditional exports where the default is ESM and the require is CJS a CJS exports["."].require and an ESM main will fail with an ERR_REQUIRE_ESM error after any call to quibble().

This reproduces for me with the latest version of quibble (0.9.2) and the latest version of Node 18 (18.20.1) an 20 (20.12.1).

Reproduction

Save the following bash script as repro.sh to an empty directory and run it with bash repro.sh:

#!/usr/bin/env bash

rm -rf index.js
rm -rf package.json
rm -rf package-lock.json
rm -rf node_modules/
rm -rf dep-with-conditional-exports/

[[ "$1" == "reset" ]] && exit 0

# Create the dep with conditional exports
mkdir dep-with-conditional-exports
echo "module.exports = 'cjs'" > ./dep-with-conditional-exports/index.cjs
echo "export default 'esm'" > ./dep-with-conditional-exports/index.js
cat << EOF > ./dep-with-conditional-exports/package.json
{
  "name": "dep-with-conditional-exports",
  "type": "module",
  "main": "index.js",
  "exports": {
    "require": "./index.cjs",
    "default": "./index.js"
  }
}
EOF

# Create root package json and install it
cat << EOF > ./package.json
{
  "name": "require-conditional-exports-after-quibble",
  "version": "1.0.0",
  "dependencies": {
    "dep-with-conditional-exports": "file:./dep-with-conditional-exports",
    "quibble": "^0.9.2"
  }
}
EOF
npm install --no-progress --silent

# Require conditional exports dep
echo "Can be required:"
node -e 'console.log(require("dep-with-conditional-exports"))'
echo ""

# Import conditional exports dep
echo "Can be imported:"
node -e 'import("dep-with-conditional-exports").then(r => console.log(r.default))'
echo ""

# Require it after an empty quibble
cat << EOF > ./index.js
require('quibble')()
console.log(require('dep-with-conditional-exports'))
EOF
echo "Can be required after quibble:"
node index.js
lukekarrys commented 6 months ago

Doing a bit more digging, I found that this is due to the resolve package not supporting package.json#exports. So when resolve.sync is called it looks up package.json#main which is the ESM version in my reproduction: https://github.com/testdouble/quibble/blob/1afc5fb31f63f19e32dd26d72818bd8d5bb14a5b/lib/quibble.js#L223

This is arguably a misconfigured package.json. I modeled the fixture after what axios is doing in their package.json since that was the culprit in our tests. So I guess an issue/PR is in order to them as well. (Edit: https://github.com/axios/axios/pull/6348) If the main pointed to the CJS version this wouldn't be an issue for quibble.

So I don't think this is a bug in quibble since package.json#main should always be a CJS entry point. Another option would be to use another library like resolve.exports in addition to or in place of resolve since that supports exports the way Node does.