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 Format contains external require calls #1232

Closed
DeMoorJasper opened this issue May 1, 2021 · 6 comments
Closed

ESM Format contains external require calls #1232

DeMoorJasper opened this issue May 1, 2021 · 6 comments

Comments

@DeMoorJasper
Copy link

When running esbuild with "format": "esm" it produces an invalid esm bundle by not transforming require calls into imports.

This only happens if there are external modules like with "platform": "node"

Minimal repro:

await esbuild.build({
  bundle: true,
  platform: "node",
  format: "esm",
});
import * as realpath from "fs.realpath@1.0.0"

console.log(realpath);
@evanw
Copy link
Owner

evanw commented May 1, 2021

This transformation is not done because an import statement is not equivalent to a require call (they run at different times, and require calls may not run at all while import statements always run). Doing this transformation would change the meaning of the code. You can work around this by aliasing require yourself since node doesn't do this automatically:

 await esbuild.build({
   bundle: true,
   platform: "node",
   format: "esm",
+  banner: {
+    js: `
+      import {createRequire} from 'module'
+      const require = createRequire(import.meta.url)
+    `,
+  },
 });

Edit: The problem is that there is no equivalent to require in native ESM syntax. There is import but it returns a promise, and you can't just use await import because you may not be in an async context.

@evanw
Copy link
Owner

evanw commented May 1, 2021

Closing as a duplicate of #946.

@evanw evanw closed this as completed May 1, 2021
@guybedford
Copy link
Contributor

This is an important use case in terms of having well-defined support for externalization membranes so would recommend reconsidering the decision here.

When building libraries that want to externalize dependencies of dependencies, they might not have control over how the import was made in the first place - whether it was an import or require.

We have the simple rule that can be applied here and that is that CJS modules are always just a default export so that that externalization can make complete sense.

Yes that's not comprehensive but more advanced corrections can be made over time or options can be provided to have fine-grained control further like RollupJS and Webpack offer as well. Eg via inspection functions on the namespace import in this case.

@evanw
Copy link
Owner

evanw commented May 2, 2021

CJS modules are always just a default export so that that externalization can make complete sense

The problem is that the evaluation order is wrong, not that the external modules are CommonJS. For example:

exports.lazyImport = () => require('x')

It's not equivalent to compile that to this code:

import x from 'x'
export let lazyImport = () => x

It's also not equivalent to compile that to this code:

export let lazyImport = () => import('x').then(x => x.default)

There's no ESM equivalent of require(). You already know this; I'm just saying that I'm not sure "externalization can make complete sense" because they aren't equivalent. The most accurate way to represent the original code is to keep the CommonJS require, although that's node-specific:

import {createRequire} from 'module'
const require = createRequire(import.meta.url)
export let lazyImport = () => require('x')

would recommend reconsidering the decision here

There was no decision made here other than to close this as a duplicate of #946 (which is still open). I agree that this is something to figure out, but this is already tracked by #946 and I'd like to try to keep the conversation in one place instead of having multiple issues.

@guybedford
Copy link
Contributor

@evanw externalization always causes reordering, eg for import './local.js'; import 'external', 'external' effectively being hoisted. I think it's an accepted part of the semantics of externalization at this point and similarly for CommonJS lazy imports no longer being lazy. I didn't see #946 was an identical issue, that seems fine to track further. Those ilisted workarounds all give me headaches too...

@maartenbreddels
Copy link

and require calls may not run at all while import statements always run

For this, can esbuild not translate this to import function calls

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

4 participants