fix(commonjs): resolve export "exports" not found

[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:

• resolves [@rollup/plugin-commonjs] does not provide an export named 'exports' #1306
• may resolve some of the issues listed in the thread of commonjs - Missing export (Error: ... is not exported by ...) #986

Description

Previously, this plugin relied on "synthetic named exports" to get a reference for the module.exports export of a module in a way that would update if module.exports is reassigned.
This apparently caused issues, so this PR changes that to instead use a getter and setter that updates exports if module.exports is reassigned.

[shellscape]

Feel free to merge 🎉

[lukastaegert]

Currently thinking about this, maybe I know a solution that is even more CommonJS spec compliant, let me check. Will convert it to draft for a moment.

[lukastaegert]

Turns out my other idea did not really work out. As I already got the go for this, I will merge this one now.

[wizardist]

@lukastaegert would you consider merging a patch to replace get/set with Object.defineProperty()? It would help building for some ES3 runtimes, incl. older Rhino builds. Or is it an absolute no go?

[lukastaegert]

The code in question is theoretically included for every single CommonJS module. Using Object.defineProperty can blow the code up quite a bit and potentially harm tree-shaking for everyone. Can't you run a code transform plugin to rewrite the accessors?

[wizardist]

Yes, that’s our coping strategy for the time being. Thanks!