-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 Bundled Output still uses dynamic requires. Unable to make bundled, module safe output #1944
Comments
A dynamic import crypto from "crypto"; This is effectively what the script in #1921 generates with esbuild This functionality ought to be relatively easy to integrate into esbuild itself as it already did all of the upfront analysis. |
oh i see, so would your suggestion be moving all dynamic requires to static imports? Any possible issues with modules crossing over with each other? say 2 different packages have the same dynamic require but different versions? Maybe you were suggesting static imports for node modules only? Either way is good with me, though I am just gonna stick with |
It is more incorrect to transform a require to dynamic import than bare import because then you have to mark all related functions async, which we called function color. The script in #1921 is a good workaround. And don't worry if you're generating import statement in cjs context, esbuild can still transform them to require call. I have made a plugin using the same trick, you may have a look: https://github.com/hyrious/esbuild-plugin-commonjs. Through witch 1. it is not post processing your code, 2. no versions problem since the import statement is injected to the module's content. |
I definitely see where your coming from but almost all of this code is being generated to begin with so I think its fair to change how it is assembled. I can see how messing with how someone's code works during transpile could get touchy though. However I could also see a case where moving to a bare import would be not what a developers wants. For example a module that does some heavy amount of work it its module scope. For example compiling templates or something upfront. Now you have to pay for that compile time even if you never have a code path to that file. In Lambda I could see this coming up more as you would need to pay to run compilation on things that may never even happen. If you go to Either way it would be nice if esbuild understood esm output OOTB. Whichever they land on, be it bare imports, or async, I trust they will work through the cases. Heck maybe we need to be able to configure which type of transform we would like to use, async or bare. I will take a look at the plugin though, thanks for putting that together! This could be a good way to get by until esbuild can work this in. 🙏 |
If you're trying to generate an esm bundle with cjs sources, createRequire() could be used in a banner Consider an entrypoint, import { ulid } from 'ulid'
console.log(ulid()) This will throw Adding a banner to the build: {
entryPoints: ['./ulid.js'],
bundle: true,
platform: 'node',
format: 'esm',
outfile: './ulid.bundle.mjs',
banner: {
js: [
`import { createRequire as topLevelCreateRequire } from 'module'`,
`const require = topLevelCreateRequire(import.meta.url)`
].join('\n')
}
} This will still cause actual dynamic requires (for example, require(process.env.THING_TO_REQUIRE)) to fail, but I mean, in both cases it's a runtime error. I wonder if esbuild could do this automatically if platform is node and the thing being passed to __require is a builtin. |
Thanks @dougmoscrop this is a pretty great solution. It would be great if esbuild had a more official option to output a ESM compatible bundle and all it did was inject this into the top. So I am gonna leave this open in hopes this makes it was into a proper config. For now though this is very helpful. Thanks! |
The solution of @dougmoscrop works only on |
for me this little config resolved all my problems with require in packages, and the use of __dirname and __filename in packages. tested and a middle size project and deployed
|
@guushamann Thanks for sharing. The banner that worked for me (I believe your version is missing import { fileURLToPath } from 'node:url';
import { dirname } from 'node:path';
import { createRequire as topLevelCreateRequire } from 'node:module';
const require = topLevelCreateRequire(import.meta.url);
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename); I was hoping that this banner will finally let me bundle esbuild itself, but it only got me to runtime error “The esbuild JavaScript API cannot be bundled”, and I guess that’s that. :( |
import { fileURLToPath } from 'url';
import { createRequire as topLevelCreateRequire } from 'module';
import { dirname as ddirname } from 'path';
const require = topLevelCreateRequire(import.meta.url);
const __filename = fileURLToPath(import.meta.url);
const __dirname = ddirname(__filename); alias to dirname to avoid var name conflict |
Using a very simple npm package as an example, like
jwt-simple
given the following inputrunning this through esbuild with bundle and format ESM set we will get something like this
Currently it is not possible to use esbuild to bundle a project that imports npm packages that still use cjs. I would understand if the npm package itself had a dyanmic require in it. That should throw an error, but esbuild should not be generating dynamic requires in its bundles if want to target esm. Maybe this needs to be another option? Some way to output module safe bundles code? Though I would think if a user specifies a format of ESM, they would want to use
import
overrequire
.I believe this is connected to #1921
The text was updated successfully, but these errors were encountered: