[@rollup/plugin-commonjs] Fails to bundle json-bigint

[perrin4869]

• Rollup Plugin Name: @rollup/plugin-commonjs
• Rollup Plugin Version: Both ^21 and 22.0.0-13
• Rollup Version: 2.70.0
• Operating System (or Browser): Slackware64-current
• Node Version: 16
• Link to reproduction (⚠️ read below): https://github.com/perrin4869/rollup-json-bigint

Expected Behavior

Run npm ci && npm run build && node index.out.js.
It should print

❯ node index.out.js
[Object: null prototype] { foo: 'bar' }
{"foo":"omg"}

Actual Behavior

❯ node index.out.js
[Object: null prototype] { foo: 'bar' }
/home/perrin4869/nodejs/rollup-json-bigint/index.out.js:3135
                    isBigNumber = value != null && (value instanceof BigNumber || BigNumber.isBigNumber(value));
                                                                                            ^

TypeError: BigNumber.isBigNumber is not a function
    at str (/home/perrin4869/nodejs/rollup-json-bigint/index.out.js:3135:86)
    at JSON.stringify (/home/perrin4869/nodejs/rollup-json-bigint/index.out.js:3298:21)
    at Object.<anonymous> (/home/perrin4869/nodejs/rollup-json-bigint/index.out.js:3763:13)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47

Additional Information

[lukastaegert]

There are some things that can be done from within the commonjs plugin to handle this situation better by default, but the main problem I see is with the bignumber.js package. This package provides both a CommonJS and an ES module entry point. However, the CommonJS entry is not "interchangeable" with the ES module entry because it just exports what is the default export of the ES module entry while the ES module entry has additional named exports.
Now what happens is that json-bigint happens to be CommonJS, and since the bignumber package uses the module property, Rollup will resolve the require expression in json-bigint with the ESM version of bignumber. This actually kind of works except that the additional properties of BigNumber are not copied over in the generated interop.

Still, the proper solution would be for the bignumber package to implement the exports property in its package.json file correctly instead of relying on the module property alone.

To work around the issue, you can specify requireReturnsDefault: true as an option for the commonjs plugin to tell how the correct interop works.

[perrin4869]

@lukastaegert thanks!! I sent a PR with the changes you suggested, hopefully it gets merged. In the meantime I'm just adding a patch-package patch and call it a day.
Feel free to close this if you want

[shellscape]

May also be related: #1047

[stale]

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ