feat(commonjs): prevent rewrite require.resolve

[DorianBlues]

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:
#406

Description

Prevent commonjs plugin rewrite Nodejs original require methods.

[danielgindi]

Could you please outline real-world use cases of require.resolve/require.cache/require.main where you roll it up, and how/why the output should look?

Because there are many cases where it’s not clear what that require should be afterwards, especially if it’s compiled for ESM.

[guybedford]

This seems like a sensible approach to me. Allowing remapping of require.resolve expressions could be another approach.

Ideally such a function could be rewritten to import.meta.resolve - but the work for that is still in progress and it will be a long while before browsers ship something like this.

[shellscape]

@danielgindi @lukastaegert should we merge this one? thanks for taking a look @guybedford

[danielgindi]

@danielgindi @lukastaegert should we merge this one? thanks for taking a look @guybedford

I’m not sure yet. I don’t think we have enough clarity on what happens when, or what the user expects when using these specific require members.

[shellscape]

@lukastaegert please take a look when life allows. I think this one is important and don't want to let it get too stale.

[lukastaegert]

I think it is a reasonable solution for now. The code itself looks sensible. Otherwise, generated code will only work in a CommonJS module in Node, but that is more than can be said for the current state where the code works nowhere. As @guybedford said, in the future there might be a native ESM equivalent, but that does not really exist yet.
So I would ship it like it is for now as it does not break anything but makes some use-cases work and is not very complicated code-wise and thus easily replaced by a better solution in the future if we find one.

[lukastaegert]

Actually with this solution in place, one might easily inject a polyfill for require.resolve in the generated bundle for non CJS formats, so yes, I think it is a reasonable step forward.