Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with CommonJS modules that reexport transpiled ES modules #1116

Closed
sodatea opened this issue Feb 22, 2022 · 4 comments
Closed

Comments

@sodatea
Copy link
Contributor

sodatea commented Feb 22, 2022

Expected Behavior

import foo from './reexport.cjs should get the default export from transpiled-esm.cjs.

Actual Behavior

The CJS plugin doesn't realize that the reexport.cjs should also be treated as an ES module.

Additional Information

See also vitejs/vite#2139 (comment)
Previously reported at #872

It seems quite like Improvement 7 in #481 but that issue only addresses the issue with external dependencies, not bundled dependencies.

I'm willing to write a third-party plugin or create a PR for the commonjs plugin to fix this issue.
But I couldn't figure out a solution by myself yet.
Could you provide some guidance?

@lukastaegert
Copy link
Member

That is definitely something one should have a look at. But note that ideally, any PR should be rebased on #1038 as that one is a major rewrite of the plugin. I think it will not fix this issue, though, as currently one assumption is that __esModule is always defined in the same file. The question is if one would just handle this specific case as a heuristic, or want to fix any possible case where the __esModule is merged in via some external code.

@sodatea
Copy link
Contributor Author

sodatea commented Feb 22, 2022

The question is if one would just handle this specific case as a heuristic, or want to fix any possible case where the __esModule is merged in via some external code

I think we can start from the simple case, i.e. plain reexports such as the example code in the reproduction repo.

Implementing this would give us the tool to set a module's isEsModule, hasDefaultExport flags based on information from other modules.
Then we can gradually support other cases in the plugin.

@fwouts
Copy link
Contributor

fwouts commented Apr 8, 2022

FYI @lukastaegert I've tried and failed to implement a patch for #1038 that would solve this as I couldn't find an elegant way to look into the required module (possibly recursively) to see whether it does set __esModule.

I thought I'd at least contribute a (failing) test case for later, which reproduces the current situation exhibited for example by ./link in the next package: fwouts@347c148

Hope this helps get it fixed ultimately! :)

@lukastaegert
Copy link
Member

Was resolved via #1165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants