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

should use esm/cjs's native way to import externals (no wrappers needed) #1927

Open
hronro opened this issue Jan 11, 2022 · 4 comments
Open

Comments

@hronro
Copy link

hronro commented Jan 11, 2022

Code Example

Say the we have an input file foobar.js:

import foo from 'foo';
const bar = require('bar');
module.exports = foo + bar;

What I expect when running esbuild foobar.js --bundle --format=esm --external:foo --external:bar --platform=browser is:

import foo from "foo";
import bar from "bar";

var __getOwnPropNames = Object.getOwnPropertyNames;
var __commonJS = (cb, mod) => function __require() {
  return mod || (0, cb[__getOwnPropNames(cb)[0]])((mod = { exports: {} }).exports, mod), mod.exports;
};

// foobar.js
var require_foobar = __commonJS({
  "foobar.js"(exports, module) {
    module.exports = foo + bar;
  }
});
export default require_foobar(); 

However the actual is:

var __getOwnPropNames = Object.getOwnPropertyNames;
var __require = /* @__PURE__ */ ((x) => typeof require !== "undefined" ? require : typeof Proxy !== "undefined" ? new Proxy(x, {
  get: (a, b) => (typeof require !== "undefined" ? require : a)[b]
}) : x)(function(x) {
  if (typeof require !== "undefined")
    return require.apply(this, arguments);
  throw new Error('Dynamic require of "' + x + '" is not supported');
});
var __commonJS = (cb, mod) => function __require2() {
  return mod || (0, cb[__getOwnPropNames(cb)[0]])((mod = { exports: {} }).exports, mod), mod.exports;
};

// foobar.js
import foo from "foo";
var require_foobar = __commonJS({
  "foobar.js"(exports, module) {
    var bar = __require("bar");
    module.exports = foo + bar;
  }
});
export default require_foobar();

Reason

Generally we have two ways to consume the output:

  1. Consume the output directly (e.g. running the esm output in <script type="module"> in browser, or running it in Node.js with native esm support).
  2. Consume the output via a bundler (e.g. esbuild, webpack, rollup...).

Consume the output directly

When we consume the output directly, the externals should ready available as well. In browsers, we should already specified what the module foo and the module bar are using import-maps. Since foo and bar must be esm, we don't need a wrapper when improting them. In Node.js, it means foo and bar are already installed or them are just nodejs built-in modules. Thought foo and bar could be esm or cjs, but we dont' need an import wrapper because Node.js will automatically does it for us.

However in current implementation, esbuild will produce something like var bar = __require("bar");, which will cause a rumtime error.

Consume the output via a bundler

When consume the output via a bundler, the bundler will actually add those wrappers. So no wrappers needed for esbuild.

@hyrious
Copy link

hyrious commented Jan 13, 2022

This is actually some sort of @rollup/plugin-commonjs does. But that's not trivial, changing the import method will result in choosing different files according to the exports field in package.json.

While I'd like to see esbuild can do this tiny transform (just get rid of __requires).

In simple cases, someone has written a script to do that transform: #1921 (comment).

@hronro
Copy link
Author

hronro commented Jan 14, 2022

@hyrious

hanging the import method will result in choosing different files according to the exports field in package.json.

That only happens when running the output in a Node.js environment, not for browsers. (most bundlers implenment the Node.js resovle algorithm, but not all of them)

@mickdelaney
Copy link

whats the consensus on this ?
will there be a built in way to deal with this in the future ?

@paralin
Copy link

paralin commented Oct 19, 2023

See my comment here: #1921 (comment)

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