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

Dedupe dependencies #1038

Closed
ebeloded opened this issue Mar 23, 2021 · 8 comments
Closed

Dedupe dependencies #1038

ebeloded opened this issue Mar 23, 2021 · 8 comments

Comments

@ebeloded
Copy link

While analyzing the bundle, I found that two of my dependencies -rxjs and rxfire, both depend on tslib. This results in tslib being included twice.

image

Is there any way to dedupe tslib so that only one version is included in the final bundle?

@remorses
Copy link
Contributor

remorses commented Mar 23, 2021

This is more a problem with your package manager, if your package manager has duplicate dependencies esbuild will duplicate that code too

If you use yarn you can use the resolutions field to force the resolution of a package to only one versoin
https://classic.yarnpkg.com/en/docs/selective-version-resolutions/

@merceyz
Copy link

merceyz commented Mar 23, 2021

Yarn has a builtin dedupe command so you can run yarn dedupe tslib - https://yarnpkg.com/cli/dedupe

Though it would be interesting if esbuild could detect when files are indentical and only store its content once but still preserve the dependency tree / module instance

@evanw
Copy link
Owner

evanw commented Mar 23, 2021

What esbuild is doing here appears to be correct. The file node_modules/rxjs/node_modules/tslib/tslib.js is a different path from the file node_modules/rxfire/node_modules/tslib/tslib.js and so must be duplicated in the bundle for correctness.

Unlike some other programming languages, a duplicate copy of code is not equivalent to the original in JavaScript. Function objects in JavaScript have their own referential identity and each copy of the library would have its own copy of its internal state. It would be incorrect for esbuild to make this transformation itself, so I'm going to close this issue as "working as intended". See also #928.

Note that using --preserve-symlinks can cause this because it tells esbuild to treat two separate symlinks to the same file as two separate files. Make sure you don't have that enabled if you don't need it (it's not on by default). Otherwise the way to fix this is to fix your package manager.

You can also work around this with an on-resolve plugin: https://esbuild.github.io/plugins/#resolve-callbacks. The plugin could intercept these separate paths and map them to the same path. Then they would be considered to be the same module and be merged.

@evanw evanw closed this as completed Mar 23, 2021
@lukeed
Copy link
Contributor

lukeed commented Mar 24, 2021

I just ran into this too w/o anything fancy going on.

Here's a reproduction, but the gist of it is that 5 source files include:

import colors from 'kleur';

...which, with esbuild, produces an output file with 5 instances of the same statement. I'm targeting esm output & using bundle: true. By default, this should dedupe as esbuild already handles much more complex dependency-resolution logic by default.

My reproduction includes the output code that Rollup was producing.

  • esbuild: 0.9.6
  • pnpm 5.13.6 for installs (uses symlinks, but all references point to the same file)

Edit: Also included a lib-rollup/esbuild.mjs file for comparison. The other lib-rollup files are Rollup only.

@ebeloded
Copy link
Author

@evanw thank you for the explanation
@remorses resolutions was exactly what I needed

@evanw
Copy link
Owner

evanw commented Mar 24, 2021

@lukeed That sounds like an unrelated issue, and a duplicate of #475. I'm working on fixing this one. The problem in that case is not that a file is present in the bundle multiple times like this issue, since two import statements for the same path do not result in that path being imported multiple times.

@lukeed
Copy link
Contributor

lukeed commented Mar 24, 2021

@evanw Ah sorry, thanks! Should I freeze the reproduction branch in its current state or is it of no help to you?

@evanw
Copy link
Owner

evanw commented Mar 24, 2021

I don't need it, no. It's covered by #475.

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

5 participants