-
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
Option to treat all node modules as external #619
Comments
I made a plugin to use the import NodeResolve from '@esbuild-plugins/node-resolve'
import { build } from 'esbuild'
build({
plugins: [
NodeResolve({
extensions: ['.ts', '.js'],
onResolved: (resolved) => {
if (resolved.includes('node_modules')) {
return {
external: true,
}
}
return resolved
},
}),
],
}) You can find other plugins here |
I'm intending for plugins to be used to solve custom use cases like this one instead of having this behavior built in. I'm going to close this since the original poster gave a 👍 on the previous post about using a plugin. FWIW you don't even need to resolve the path to do this assuming all non-node_modules paths are relative or absolute paths, which is the case for node's module resolution algorithm. Something like this might be sufficient: let makeAllPackagesExternalPlugin = {
name: 'make-all-packages-external',
setup(build) {
let filter = /^[^.\/]|^\.[^.\/]|^\.\.[^\/]/ // Must not start with "/" or "./" or "../"
build.onResolve({ filter }, args => ({ path: args.path, external: true }))
},
} |
@evanw This is probably a sane delineation for what should be in esbuild. I added a thumbs-up on the plugins comment because plugins may solve the problem for others and the 👍 would draw attention to it for future readers, but for myself we're trying to stick to the command-line for now (until we have the cycles to dedicate to adding esbuild to our golang server). In the interim we're using --external-imports := $(shell jq '.dependencies|keys[]' package.json) If a glob were available for esbuild we'd probably use it, but as you can see we've worked around it and there are plugins so unless it's trivial there are probably better problems to dedicate time to. |
@evanw Seems that currently we cannot use plugins in synchronous API calls, so the solution above is limited. |
FWIW, this issue caused me not to use esbuild. When I ran into this in a node context and saw that "The plugin API is new and still experimental" and doesn't seem to be something I can run from the command line, I gave up on using esbuild for compiling my TS Node project – esbuild went from "plug & play" to "takes some work to setup". Just sharing in case it's helpful feedback – I'm not frustrated and it may make sense for this to not be in core (though I do have a hard time understanding why |
Hi guys, I am trying to get a better grasp at building, but basically when I am bundling some TS code for Node (eg React components that should render server side), I'd expect this to be the default. I am still not sure what to do with the answers here. |
This isn't some edge case, it's the only sane way to build a node.js project by default. If esbuild supports the 'node' platform, which marks internal node libs as 'external', it goes to reason that node_modules should be a part of that configuration. Having to pick competing plugins just to keep node_modules separate from my bundle is a clear shortcoming of esbuild, and is causing me to consider using something else. |
After much pain I can't go vanilla esbuild, it's just not mature enough for bundling nodejs applications. moving to vite. |
of course not, It's much sane to bundle node_module, which could reduce cold start time and save lots of node_modules space for user |
How is it saving node_modules space, if you need to have a copy of node_modules present in order to build it in the first place? What about when you're deploying to a server environment that's got a different OS, architecture to your own? What about when you don't want to blow up your deployment package to 80mb when it could just be < 2mb, so it's easily to open and inspect directly if needed? Esbuild doesn't even do vendor bundle splitting yet, so at least I can separate all the junk from my actual code. I have been developing in nodejs for over 8 years, and not once has the desire to slam my entire node_modules dir into our app bundle ever made sense. Esbuild should account for such a common use case, and it doesn't. |
esbuild can do vendor splitting actually |
The question might be: why do people build for Node.js:
|
This is actually easy to do without any plugins. const pkg = require("./package.json")
require("esbuild").build({
entryPoints: ["./src/index.ts"],
// ...other configs
external: [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.peerDependencies || {})]
}) |
@Dudeonyx I think the point is more whether it should be a default option or not, indeed it's not that hard to do in user-land, but imo it shows that some use cases of Esbuild might be missed by the maintainers (voluntarily or not, I respect their design choices). |
I'm going to enable you to just do
I see what you're saying. But unfortunately there's no way to configure esbuild to override an external module to make it not external, at least not without a plugin. So it's not immediately clear whether this is a good default or not. This also wouldn't work for other package managers such as Yarn, which store packages in a different directory. I think just making it easy to mark everything under a directory as external is a good approach. |
Documentation has been updated as well: |
In monorepos, you end up with node_modules folders at different levels, in different packages. I would like to externalize everything in all of these node_modules folders, but I've only found this hacky way of doing that:
Is there a more catch-all solution to externalize all node_modules folders no matter what their path? |
@beorn It's not clear to me from the code if this would work, but have you tried |
No, unfortunately esbuild only allows one wilecard in a pattern. |
Should this be reopened, or should there be a new issue for it? It doesn't seem feasible to manually specify each parent level of node_modules. |
This reverts commit c678f84. trigger pulled too soon: - evanw/esbuild#619 was gone - but then come evanw/esbuild#1958
A way to do this has just been released: https://github.com/evanw/esbuild/releases/tag/v0.16.5. You can now pass |
@evanw I saw that, thanks. When I've cycles I'm going to look at this in the context of a monorepo where our tsconfig rewrites the imports as top-level. Will report if there's anything interesting. |
For those that use a typescript base dir and paths like |
In case anyone is facing this issue with I added |
@evanw would it be possible to get a list of the packages that were marked as external this way, together with their versions? |
You can easily do that with an esbuild plugin. Something like this perhaps (untested): const plugin = {
name: ...,
setup(build) {
build.onResolve({ filter: /.*/ }, args => {
if (!/^(#|\/|\.\/|\.\.\/)/.test(args.path)) {
reportPackagePath(args.path)
return { external: true }
}
})
}
} |
Looks like I have the following:
But I took a look at the output file from my build and there is an |
I'd prefer an option that works like |
hey evan, I'm doing some work on the nx esbuild plugin where the user's options are passed directly to the build API, but we'd still like to collect the names of packages marked as external
ideally we'd like to rely on the default resolution logic, is there a way to still collect the names of external packages without inserting our own plugin? if not, the only alternative is to re-implement the default logic, which could be tricky since we're supporting all types of user config (eg. external array with wildcards, or packages=external) |
I think this should do it... although this assumes building with node builtins automatically marked as external build.onResolve({ filter: /.*/, namespace: 'file' }, async args => {
const { path, external, errors } = await build.resolve(args.path, {
kind: args.kind,
resolveDir: args.resolveDir,
namespace: 'default'
})
if (errors.length > 0) {
return { errors }
}
if (external && !/^(node:|#|\/|\.\/|\.\.\/)/.test(path)) {
const parts = path.split('/')
let pkg = parts[0]
if (pkg[0] === '@') {
pkg = parts.slice(0, 2).join('/')
}
if (!require('module').builtinModules.includes(pkg)) {
externals.add(pkg)
}
}
return { path, external }
}) |
Unless I'm missing something, when bundling for Node.js it appears as though one has to use
--external
for every external module imported, even if those are packages that the compiled bundle could otherwiserequire
/import
at run-time.Without using
externals
our bundled files end up being in the hundreds of megabytes vs 400k when everything is properly externalized.So in
Makefile
I'm doing something like this:This is leading to problems when we add/remove packages and their references i.e. additional overhead and developer cycles remembering/identifying/fixing this step.
I've toyed with using e.g.
jq
to read thepackages.json
or an ESBuild plugin, etc., but it feels like the responsibility for this is probably with the bundler proper.I think what is desirable for this proposed option would be if ESBuild treated every package in
node_modules/
as an external (or perhaps alternatively, everything inpackage.json
).This is not a blocker, and there's probably something more general here that would work (e.g. an "
--external-path
" that matches a glob against the file-system path), but in any case I hope it's something easy/fun.The text was updated successfully, but these errors were encountered: