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

ESM entry module's exports signature is not preserved when required() by other modules #706

Closed
yyx990803 opened this issue Jan 25, 2021 · 5 comments · Fixed by #1078
Closed
Labels

Comments

@yyx990803
Copy link
Contributor

yyx990803 commented Jan 25, 2021

Context: I am using esbuild to do dependency-pre-bundling in Vite. This is a multiple-entry build with splitting: true. The entries are resolved package entry files - e.g.

- node_modules/preact/index.module.js
- node_modules/react/index.js

I'm also using a plugin with onResolve that redirects any imports to preact or react to these already resolved file locations to avoid duplicated copies of them in the resulting bundle.

Preact is a pure ESM module, so its corresponding output entry chunk is expected to preserve the same export signature as the original entry module. However, this is broken if any transitive module in the bundle contains a cjs require() call to it. E.g. if somewhere down the tree a dependency does this:

const preact = require('preact')

This causes esbuild to wrap the ESM preact module with a CommonJS wrapper and expose only a default export. This makes sense for the internal interop, but currently this also affects the final output entry chunk for preact: now the output chunk for preact also only has a default export! This creates a mismatch between the output of the preact chunk and the original entry chunk.

What I imagine esbuild needs to do is to

  • move both the preact module and its wrapped CommonJS code to a shared chunk
  • re-export the same original exports from the preact entry chunk
  • import and use the wrapped CommonJS version in the sub dependency that uses require()

Currently I have to work around this by detecting exports mismatch between input/output entry chunks and perform additional runtime interop. Would be great to have this fixed in esbuild itself.

@yyx990803
Copy link
Contributor Author

I created a reproduction repo: https://github.com/yyx990803/esbuild-exports-signature-repro

@yyx990803 yyx990803 changed the title ESM entry modules are converted to wrapped CJS with only default export when required() ESM entry module's exports signature is not preserved when required() by other modules Jan 25, 2021
@evanw
Copy link
Owner

evanw commented Jan 26, 2021

Thanks for the report. I'll keep this in mind during the upcoming code splitting rewrite.

@diervo
Copy link

diervo commented Feb 5, 2021

Just to add to Evan's decription: when bundling ESM (even when format:'esm') the ModuleRecord shape is not preserved. Or said in another words, named exports get transformed into a non-equivalent default export binding.

An even simpler example is esbuild: when importing it as an ESM module in NodeJS
will not be able to find the named exports correctly (as per the typescript definition).

Example:

//  node test.mjs
import { transform } from 'esbuild';
console.log(transform);

Error:

import { transform } from 'esbuild';
         ^^^^^^^^^
SyntaxError: Named export 'transform' not found. The requested module 'esbuild' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

I think esbuild is being bundle with esbuild, so it exhibits the same export mangling problem.

@evanw
Copy link
Owner

evanw commented Mar 8, 2021

Preact is a pure ESM module

Actually, why do you say this? It looks like CommonJS to me: https://unpkg.com/preact@10.5.12/dist/preact.min.js. It assigns to module.exports. I'm asking because I'm currently considering removing the ability to bundle code that calls require on ESM code.

@diervo
Copy link

diervo commented Mar 11, 2021

@evanw I think that would be awesome as it will remove the ambiguity (the same applies for the module.exports intrinsics occurrence which is the issue that I raised in this thread that was closed already) and it will force library authors to do the right exports behaviour.

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

Successfully merging a pull request may close this issue.

3 participants