evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
38.16k stars 1.15k forks source link

Option to treat all node modules as external #619

Closed brianmhunt closed 3 years ago

brianmhunt commented 3 years ago

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 otherwise require/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:

--external-files    := fs zlib uuid commander source-map-support sanitize-filename date-fns pako
--externals         := $(--external-files:%=--external:%)
ESBUILD     :=  ./node_modules/.bin/esbuild --platform=node --bundle $(--externals)

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 the packages.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 in package.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.

remorses commented 3 years ago

I made a plugin to use the resolve package to resolve packages and add more customization options, to make node modules external you can do this

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

evanw commented 3 years ago

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 }))
  },
}
brianmhunt commented 3 years ago

@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 jq for something like this in our Makefile:

--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.

meteorlxy commented 3 years ago

@evanw Seems that currently we cannot use plugins in synchronous API calls, so the solution above is limited.

OnurGvnc commented 3 years ago

https://github.com/egoist/tsup/blob/master/src/esbuild/external.ts

rattrayalex commented 3 years ago

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 node_modules aren't handled out of the box). I'm sure I could use esbuild if I really wanted to.

eric-burel commented 2 years ago

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.

jpike88 commented 2 years ago

I'm intending for plugins to be used to solve custom use cases like this one instead of having this behavior built in.

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.

jpike88 commented 2 years ago

After much pain I can't go vanilla esbuild, it's just not mature enough for bundling nodejs applications. moving to vite.

hardfist commented 2 years ago

This isn't some edge case, it's the only sane way to build a node.js project by default.

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

jpike88 commented 2 years ago

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.

hardfist commented 2 years ago

esbuild can do vendor splitting actually

eric-burel commented 2 years ago

The question might be: why do people build for Node.js:

Dudeonyx commented 2 years ago

This is actually easy to do without any plugins. This is how i did it for rollup and it works just fine for esbuild

const pkg = require("./package.json")

require("esbuild").build({
    entryPoints: ["./src/index.ts"],
    // ...other configs
    external: [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.peerDependencies || {})]
})
eric-burel commented 2 years ago

@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).

evanw commented 2 years ago

I'm going to enable you to just do --external:./node_modules/* in the next release. That should make this easier.

I'd expect this to be the default

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.

evanw commented 2 years ago

Documentation has been updated as well:

beorn commented 2 years ago

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:

--external:./node_modules/* --external:../node_modules/* --external:../../node_modules/* and so on...

Is there a more catch-all solution to externalize all node_modules folders no matter what their path?

brianmhunt commented 2 years ago

@beorn It's not clear to me from the code if this would work, but have you tried --external:*/node_modules/*? This generally works for most pattern matchers, but I haven't had a chance to try it yet, but we're also using a monorepo and I'd like to know the answer too.

beorn commented 2 years ago

@beorn It's not clear to me from the code if this would work, but have you tried --external:*/node_modules/*? This generally works for most pattern matchers, but I haven't had a chance to try it yet, but we're also using a monorepo and I'd like to know the answer too.

No, unfortunately esbuild only allows one wilecard in a pattern.

beorn commented 2 years ago

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.

evanw commented 1 year ago

A way to do this has just been released: https://github.com/evanw/esbuild/releases/tag/v0.16.5. You can now pass --packages=external to accomplish this.

brianmhunt commented 1 year ago

@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.

andsens commented 1 year ago

For those that use a typescript base dir and paths like src/config.ts to include source files (rather than ./config.ts): The --packages=external will cause esbuild to recognize those paths as node modules as well.
One can work around this by using aliases: Replace src/ with their absolute version (e.g. --alias:src=/home/user/workspace/project/src) and do the same for any other top-level dirs.
Assuming you have some tooling around the esbuild invocation it should be pretty trivial to do :-)

marcustut commented 1 year ago

In case anyone is facing this issue with esbuild, I share the exact same scenario as @brianmhunt and the trick is to not bundle @prisma/client during the build.

I added external: ["@prisma/client"] to the build script.

espoal commented 1 year ago

@evanw would it be possible to get a list of the packages that were marked as external this way, together with their versions? I'm trying to automate CI/CD and I want to automatically update the package.json of the dist folder.

evanw commented 1 year ago

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 }
      }
    })
  }
}
pthieu commented 1 year ago

Looks like packages: 'external' doesn't respect tsconfig aliases. I was under the impression from this release note that this would ignore only node_modules.

I have the following:

index.ts
    -> imports {x} from foo.ts
        -> imports from node_modules

But I took a look at the output file from my build and there is an import x from '~/...' after adding packages: 'external' in my esbuild.mjs

aleclarson commented 8 months ago

I'd prefer an option that works like --packages=external but still bundles any devDependencies. Is there an issue open for such a feature?

mattfysh commented 6 months ago

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

If no callback returns a path, esbuild will run its default path resolution logic

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)

mattfysh commented 6 months ago

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