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

Option to treat all node modules as external #619

Closed
brianmhunt opened this issue Dec 23, 2020 · 31 comments
Closed

Option to treat all node modules as external #619

brianmhunt opened this issue Dec 23, 2020 · 31 comments

Comments

@brianmhunt
Copy link

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
Copy link
Contributor

remorses commented Dec 28, 2020

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
Copy link
Owner

evanw commented Dec 29, 2020

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 evanw closed this as completed Dec 29, 2020
@brianmhunt
Copy link
Author

@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
Copy link

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

@OnurGvnc
Copy link

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

@rattrayalex
Copy link

rattrayalex commented Oct 18, 2021

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
Copy link

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
Copy link

jpike88 commented Jan 11, 2022

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
Copy link

jpike88 commented Jan 12, 2022

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

@hardfist
Copy link
Contributor

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
Copy link

jpike88 commented Jan 12, 2022

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
Copy link
Contributor

esbuild can do vendor splitting actually

@eric-burel
Copy link

eric-burel commented Jan 12, 2022

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

  • a standalone script => you may want to bundle everything, a bit like Vercel's NCC https://github.com/vercel/ncc
  • a reusable package => on the contrary, you may want to treat everything as external. For instance, I've tried using Esbuild to basically transpile from TS to JS. I don't need anything francy in this scenario, only transpiling and bundling local files together

@Dudeonyx
Copy link

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
Copy link

@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 added a commit that referenced this issue Jan 22, 2022
@evanw
Copy link
Owner

evanw commented Jan 22, 2022

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
Copy link
Owner

evanw commented Jan 22, 2022

Documentation has been updated as well:

@beorn
Copy link

beorn commented Jan 29, 2022

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
Copy link
Author

@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
Copy link

beorn commented Jan 31, 2022

@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
Copy link

beorn commented Feb 20, 2022

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.

AntonioMeireles added a commit to AntonioMeireles/homebridge-vieramatic that referenced this issue Jun 11, 2022
This reverts commit c678f84.

trigger pulled too soon:
- evanw/esbuild#619 was gone
- but then come evanw/esbuild#1958
@evanw
Copy link
Owner

evanw commented Dec 13, 2022

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
Copy link
Author

@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
Copy link

andsens commented Feb 1, 2023

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
Copy link

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
Copy link

espoal commented Apr 11, 2023

@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
Copy link
Owner

evanw commented Apr 11, 2023

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
Copy link

pthieu commented Jun 24, 2023

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
Copy link

aleclarson commented Feb 26, 2024

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
Copy link

mattfysh commented Apr 28, 2024

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
Copy link

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

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