feat(commonjs): Add ignore-dynamic-requires option

[lukastaegert]

Rollup Plugin Name: commonjs

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

As I am doing more Node bundling, I sometimes want to completely externalise a dependency (because it is doing evil things) but do so in a way that the exact execution order is preserved. So what I want is to keep an untranspiled require statement. This works well for static requires but is just not possible for dynamic require statements EXCEPT if you use a loophole: If you specify some dynamicRequireTargets but do not use them, the dynamic require function injected will do an actual require as fallback.

Now this is not nice for two reasons:

• I do not want the overhead of dynamicRequireTargets, and it is also weird that I have to specify an actually existing file I do not use to trigger the use of the enhanced require function
• IF I want to use dynamicRequireTargets, then I have no feedback if I really did it correctly. Why? Because when testing locally, the existing node_modules will gladly "patch over" any dependencies I forgot. But when I deploy my bundled code without the adjacent node_modules, it will be broken unexpectedly.

The goal of this PR is to make this a conscious decision:

• if ignoreDynamicRequires is set to true, dynamic requires will either be left untouched, or if dynamicRequireTargets is used, will get the current require fallback
• if ignoreDynamicRequires is set to false or left unspecified, then dynamic requires will always throw an error if they cannot be resolved via dynamicRequireTargets.

You already got this error before without the dynamicRequireTargets option, the breaking change is that you now also get it when dynamicRequireTargets is specified. I feel this is much more consistent and hopefully worth the break. We could make it non-breaking by changing the default, though. On the other hand, maybe we can couple this with #658?

The naming choice was made to extend the ignore option that does the same for static require calls.

Possible future extensions that I would see as out-of-scope for now but we could easily add if there is demand:

• allow string values to "replace" the require function with another global specified?
• allow a special value ignoreDynamicRequires: "stub" that replaces unresolved dynamic require calls with an empty object? This might be the more useful version, that way, tree-shaking could later easily clean up these calls without side-effects. But again, only if there is actual demand.

[lukastaegert]

With regard to the coverage issues, maybe we want to switch to the official codecov instead of codecov-lite package? The latter does not seem to be too well maintained.

[danielgindi]

Looks good!

[tjenkinson]

Looks good to me! The new behaviour is clearer and worth the breaking change IMO, especially given if it does break for someone it’s a simple fix