esbuild icon indicating copy to clipboard operation
esbuild copied to clipboard

Rewrite `require()` calls of Node built-ins to import statements when emitting ESM for Node

Open eduardoboucas opened this issue 2 years ago • 15 comments

When outputting ESM, any require calls will be replaced with the __require shim, since require will not be available. However, since external paths will not be bundled, they will generate a runtime error.

This is particularly problematic when targeting Node, since any Node built-in modules are automatically marked as external and therefore requiring them will fail. This is described in https://github.com/evanw/esbuild/issues/1921.

That case is addressed in this PR. When targeting Node, any require calls for Node built-ins will be replaced by import statements, since we know those are available.

Example

Take the example below:

const { extname } = require("path");

console.log(extname("one.ts"));

And the following command to bundle it with esbuild:

esbuild --format=esm --bundle --platform=node --outfile=out.mjs

Before 🔴

Without this PR, esbuild produces the following bundle:

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');
});

// ../../tests/esbuild-issue/index.js
var { extname } = __require("path");
console.log(extname("example.js"));

Running the bundle generates a runtime error:

$ node out.mjs
file:///Users/eduardoboucas/Sites/evanw/esbuild/test-out/index.mjs:6
  throw new Error('Dynamic require of "' + x + '" is not supported');
        ^

Error: Dynamic require of "path" is not supported
    at file:///Users/eduardoboucas/Sites/evanw/esbuild/test-out/index.mjs:6:9
    at file:///Users/eduardoboucas/Sites/evanw/esbuild/test-out/index.mjs:10:19
    at ModuleJob.run (node:internal/modules/esm/module_job:185:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)
    at async loadESM (node:internal/process/esm_loader:88:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

After ✅

With this PR, esbuild produces the following bundle:

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');
});

// ../../tests/esbuild-issue/index.js
var { extname } = import_path;
console.log(extname("example.js"));
import import_path from "path";

And running the bundle produces the expected output:

$ node out.mjs
.js

Notes

This issue impacted Netlify customers, as reported in https://github.com/netlify/zip-it-and-ship-it/issues/1036. We would love to contribute a fix back to esbuild, and we'll be happy to accommodate any feedback from @evanw and the esbuild community into our PR.

If the maintainers decide against merging this functionality, we'll probably add it to our esbuild fork (which you can read about here). For this reason, I've tried to reduce the surface area of the changes to the absolute minimum, which reflects in a couple of implementation details:

  1. Passing the list of Node built-in modules from the bundler to the parser feels a bit awkward. This is due to the fact that importing the resolver from the parser would lead to an import cycle. To avoid moving the list of Node built-ins to its own package and include from the resolver and the parser, I'm passing the list into the parser.

  2. The __require runtime shim is not being dead-code-eliminated when it's not used, as shown in the example above. We can address this in different ways, depending on what the maintainers decide to do with this PR.

eduardoboucas avatar Feb 28 '22 12:02 eduardoboucas

This seems to work quite well, however when trying to use esbuild targeting esm with prisma in it, it still bugged out on: https://github.com/prisma/prisma/blob/cd0ec0a3d84bbc1ea0d81e7cf9c519f18e405bc0/packages/engine-core/src/library/LibraryEngine.ts#L502 .

I guess it will continue to be a hassle to have code-splitting in a node application using esbuild as long as we have to rely on esm while tons of libraries not being ready for esm.

On a side note.

The plugin I was using to fix __filename and __dirname:

import type { Loader, Plugin } from 'esbuild';
import fs from 'fs-extra';

export const esmPlugin = (): Plugin => ({
    name: 'esmplugin',
    setup(build) {
        build.onLoad({ filter: /.\.(js|ts|jsx|tsx)$/, namespace: 'file' }, async (args) => {
            const globalsRegex = /__(?=(filename|dirname))/g;
            let fileContent = new TextDecoder().decode(await fs.readFile(args.path));
            let variables = fileContent.match(globalsRegex)
                ? `
                import { fileURLToPath as urlESMPluginFileURLToPath } from "url";
                import { dirname as pathESMPluginDirname} from "path";
                var __filename =urlESMPluginFileURLToPath(import.meta.url);
                var __dirname = pathESMPluginDirname(urlESMPluginFileURLToPath(import.meta.url));
            `
                : '';
           

            const contents = variables + '\n' + fileContent;
            const loader = args.path.split('.').pop() as Loader;

            return {
                contents,
                loader,
            };
        });
    },
});

now fails with this esbuild build, where it didn't do that before:

import import_path from "path";
SyntaxError: Identifier 'import_path' has already been declared

Not sure how keep the behavior from working...

jrmyio avatar Mar 01 '22 13:03 jrmyio

@ghaedi1993 can you merge it?

PH4NTOMiki avatar Mar 05 '22 17:03 PH4NTOMiki

hi @evanw , plase merge is pr if it will helpful the issue

sinoon avatar Mar 09 '22 11:03 sinoon

Excellent work! Hope to see this merged.

ctjlewis avatar Mar 11 '22 22:03 ctjlewis

running into this exact issue so would appreciate the merge :)

cytommi avatar Mar 12 '22 00:03 cytommi

Great work! Any chance we can see this merged?

jensmeindertsma avatar Mar 12 '22 10:03 jensmeindertsma

Can we have some light on what is needed for this to be merged?

davegomez avatar Mar 15 '22 14:03 davegomez

@evanw please

PH4NTOMiki avatar Mar 15 '22 14:03 PH4NTOMiki

Maybe something like below, when target node-esm?

import {createRequire} from 'module'

var __require =  requireWrapper(createRequire(import.meta.url))

Fix all Dynamic require of "X" is not supported

loynoir avatar Mar 18 '22 22:03 loynoir

It is possible to work around this for the time being. cc @eduardoboucas

The following approach lets us get working ESM bundles out. It requires --bundle because all other imports besides Node builtins must be inlined, so only dynamic Node builtins remain.

In the emitted bundle, esbuild adds a polyfill to check if there's a require defined, and throws if not: Dynamic require of [package] is not supported. We will ensure there is a fallback require defined for the runtime to load Node builtins that remain in the bundle.

This requires esnext target and top-level await available in the runtime environment, as that is the only way to dynamically import the modules needed to polyfill require at the top of the context.

Approach

See the BuildOptions below:

const ESM_REQUIRE_SHIM = `
await (async () => {
  const { dirname } = await import("path");
  const { fileURLToPath } = await import("url");

  /**
   * Shim entry-point related paths.
   */
  if (typeof globalThis.__filename === "undefined") {
    globalThis.__filename = fileURLToPath(import.meta.url);
  }
  if (typeof globalThis.__dirname === "undefined") {
    globalThis.__dirname = dirname(globalThis.__filename);
  }
  /**
   * Shim require if needed.
   */
  if (typeof globalThis.require === "undefined") {
    const { default: module } = await import("module");
    globalThis.require = module.createRequire(import.meta.url);
  }
})();
`;

/** Whether or not you're bundling. */
const bundle = true;

/** Tell esbuild to add the shim to emitted JS. */
const shimBanner = {
  "js": ESM_REQUIRE_SHIM
};

/**
 * ESNext + ESM, bundle: true, and require() shim in banner.
 */
const buildOptions: BuildOptions = {
  ...common,
  format: "esm",
  target: "esnext",
  platform: "node",
  banner: bundle ? shimBanner : undefined,
  bundle,
};

esbuild(buildOptions);

For a minified version of the shim, you can use the following (in general, you should not add minified code to the top of your bundle contexts because a stranger on GitHub tells you to, but feel free to verify it):

const ESM_REQUIRE_SHIM = `
await(async()=>{let{dirname:e}=await import("path"),{fileURLToPath:i}=await import("url");if(typeof globalThis.__filename>"u"&&(globalThis.__filename=i(import.meta.url)),typeof globalThis.__dirname>"u"&&(globalThis.__dirname=e(globalThis.__filename)),typeof globalThis.require>"u"){let{default:a}=await import("module");globalThis.require=a.createRequire(import.meta.url)}})();
`;

The end result of these build options are a single ESM bundle with all non-builtin modules inlined, and a global require() shimmed for any CJS imports that are left in the bundle (e.g. builtins like events).

Footnotes

If your program depends on esbuild, you will need to add it to your externs, i.e. { external: ["esnext"] }.

ctjlewis avatar Mar 19 '22 16:03 ctjlewis

@ctjlewis This looks like an interesting approach that might even fix the issues I had with prisma. However, how does esbuild know the modules required with 'require' should be part of the bundle? Doesn't esbuild follow import statements to understand what is part of the bundle?

jrmyio avatar Mar 20 '22 08:03 jrmyio

@ctjlewis This looks like an interesting approach that might even fix the issues I had with prisma. However, how does esbuild know the modules required with 'require' should be part of the bundle? Doesn't esbuild follow import statements to understand what is part of the bundle?

You need bundle: true, all non-builtin imports and requires will be inlined. The only thing that will remain are (should be) builtins and the shim provide a require to load those with at runtime.

ctjlewis avatar Mar 21 '22 01:03 ctjlewis

Anything we can do to help move this along? This is an issue that's also affecting a number of libraries we maintain, and as more and more of the ecosystem is moving to ESM this appears to be picking up steam.

scalvert avatar Apr 18 '22 16:04 scalvert

This work for me:

const buildOptions: BuildOptions = {
  ...common,
  format: "esm",
  target: "esnext",
  platform: "node",
  banner: 'import { createRequire } from 'module';const require = createRequire(import.meta.url);',
  bundle,
};

esbuild(buildOptions);`

fospitia avatar Jun 10 '22 14:06 fospitia

This work for me:

const buildOptions: BuildOptions = {
  ...common,
  format: "esm",
  target: "esnext",
  platform: "node",
  banner: 'import { createRequire } from 'module';const require = createRequire(import.meta.url);',
  bundle,
};

esbuild(buildOptions);`

[0] file:///Users/josuevalencia/dog/the-dog/ui/etc/esbuild/esbuild.mjs:62 [0] banner: 'import { createRequire } from 'module';const require = createRequire(import.meta.url);', [0] ^^^^^^ [0] [0] SyntaxError: Unexpected identifier [0] at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:139:18) [0] at ESMLoader.moduleProvider (node:internal/modules/esm/loader:236:14) [0] at async link (node:internal/modules/esm/module_job:67:21) [0] yarn bundle:dev:esbuild exited with code 1

josuevalrob avatar Aug 22 '22 13:08 josuevalrob

This work for me:

const buildOptions: BuildOptions = {
  ...common,
  format: "esm",
  target: "esnext",
  platform: "node",
  banner: 'import { createRequire } from 'module';const require = createRequire(import.meta.url);',
  bundle,
};

esbuild(buildOptions);`

Thank you this worked for me 🙏 Note that there is a typo with the quotes in the code snippet. It should be:

const buildOptions: BuildOptions = {
  ...common,
  format: "esm",
  target: "esnext",
  platform: "node",
  banner: "import { createRequire } from 'module';const require = createRequire(import.meta.url);",
  bundle,
};
esbuild(buildOptions);`

Edweis avatar Nov 17 '22 02:11 Edweis

Maybe I'm missing something, but is there a reason this hasn't been merged?? Seems like a pretty substantial bug, has been around for months and months, already has the PR ready, and has community support. Does anyone have insight onto why this bug still exists?

arimgibson avatar Nov 18 '22 00:11 arimgibson

I talked with @revmischa and he says he's just having a goof and that we all want to see this fixed. Glad that's resolved.

ctjlewis avatar Nov 19 '22 22:11 ctjlewis

banner takes an object:

const buildOptions: BuildOptions = {
  ...common,
  format: "esm",
  target: "esnext",
  platform: "node",
  banner: {
    js: "import { createRequire } from 'module'; const require = createRequire(import.meta.url);",
  },
  bundle,
};
esbuild(buildOptions);

GeoffreyBooth avatar Nov 22 '22 19:11 GeoffreyBooth

Closing, as it got stale and the community seems to have found different workarounds.

eduardoboucas avatar Feb 22 '23 09:02 eduardoboucas

@eduardoboucas

Closing, as it got stale and the community seems to have found different workarounds.

I understand that there is a way around this, but this issue still affects the community. It is hard to find this hack workaround as the error is quite vague and simply searching for a solution does not lead to this issue. This leads to people wasting time. The initially proposed solution seemed a bit more elegant. Would it be possible to reconsider a proper implementation for this issue?

If you do decide that no it's not worth it, would it be possible to document that the banner hack approach is mandatory when bundling as esm?

thanks

trobert2 avatar Apr 11 '23 15:04 trobert2

The purpose of banner is not to polyfill missing initialization code. This bug should still be fixed.

GeoffreyBooth avatar Apr 11 '23 15:04 GeoffreyBooth

I’m reopening this because I still plan on doing this.

evanw avatar Apr 11 '23 16:04 evanw

Ah whoops sorry, didn’t see that this was a PR instead of an issue. I’ll close again. In any case, I still plan on doing this.

evanw avatar Apr 11 '23 16:04 evanw

If your program depends on esbuild, you will need to add it to your externs, i.e. { external: ["esnext"] }.

@ctjlewis by depends on esbuild, do you mean you bundle with esbuild, or you use esbuild as a module in your app?

Jamie-BitFlight avatar Oct 19 '23 12:10 Jamie-BitFlight

@evanw did you get around to this?

shellscape avatar Nov 12 '23 15:11 shellscape