evanw/esbuild

Useless repeated imports when using external modules

zaygraveyard opened this issue Β· 42 comments

Setup

npx esbuild --version
# 0.7.17

echo 'export const x = 1' > a.js
echo 'import {x} from "./a.js"; console.log(x);' > b.js
echo 'import {x} from "./a.js"; console.log(x);' > c.js
echo 'import "./b.js"; import "./c.js";' > d.js

Actual behavior

$ npx esbuild --bundle --format=esm --external:"`pwd`/a.js" d.js
// b.js
import {x} from "./a.js";
console.log(x);

// c.js
import {x as x2} from "./a.js";
console.log(x2);

And

$ npx esbuild --bundle --minify --format=esm --external:"`pwd`/a.js" d.js
import{x as o}from"./a.js";console.log(o);import{x as m}from"./a.js";console.log(m);

NB: x is imported twice, once as x and once as x2

Expected behavior

$ npx esbuild --bundle --format=esm --external:"`pwd`/a.js" d.js
// b.js
import {x} from "./a.js";
console.log(x);

// c.js
console.log(x);

And

$ npx esbuild --bundle --minify --format=esm --external:"`pwd`/a.js" d.js
import{x as o}from"./a.js";console.log(o);console.log(o);

NB: x is imported only once

evanw commented

Thanks for reporting this. I have wanted to fix this for a while (and also the similar case for require). I agree this isn't great but it's also not a correctness issue, so FYI I'm going to prioritize some other things over this. For example, I'm currently working on shipping the plugin API (#111) and fixing some correctness issues (#399 and #465) and work like that will take priority. It may take a while to get to this but I do want to fix it.

Related: #382

I noticed I have an absolutely huge (near 50mb) .map file for a pretty trivial three.js project. Indeed, the bundle itself is probably rather larger than necessary, but not enough to worry about... the source map is mind-boggling.

The project is here, esbuild configuration in watch.js.

I have to say that in most ways I find this library a joy to work with - very efficient and generally I find that the configuration options make sense etc. Not sure if I just missed something here.

Is there a workaround for this? I really don't want to use another bundler.

@AlvinThorn008 I partially work around this with a plugin specifically geared at adding a shim around my imports to three. I won't suggest that it is a brilliantly scalable solution, but I'll probably look to expanding my configuration to do similar things for other specific libraries. It wouldn't really work on things imported from three/examples/whatever, for example (although a more elaborated script could be concocted), and I'm sure various modules may have different ways in which they may need to be adapted, which may not be easy to formally test, etc.

It's made a massive difference to my proejct(s), though.

  • I add a <script> tag to load it the old-fashioned way, resulting (in the case of three) in a global THREE object.
  • My build script runs in node where it calls require('three') and produces a string representing a module that exports each member by the same name.
  • A plugin declares a new namespace and loads my shim as appropriate.

The interface for writing plugins is liable to change, but it is still considerably less painful than using WebPack. Or rollup. Or any other bundler I've tried thus far. Sorry to the kind and diligent souls who maintain said projects.

// assuming that THREE is a global object,
// makes any imported reference to three proxy to that instead.
const threeShim = Object.keys(require("three")).map(k => `export const ${k} = window.THREE.${k}`).join('\n');

//https://esbuild.github.io/plugins/#using-plugins
const externaliseThreePlugin = {
    name: 'three',
    setup(build) {
        build.onResolve({ filter: /^three$/ }, args => ({
            path: args.path,
            namespace: 'three-ns'
        }));
        build.onLoad({ filter: /.*/, namespace: 'three-ns' }, (args) => ({
            contents: threeShim,
            loader: 'js'
        }));
    }
}

I'd previously written that for another project, but you can see it in context in my current version of watch.js linked above.

@xinaesthete
Thanks for this. I'll see how I can integrate this into my project.

Has there been any update on this?

Is this in the roadmap somehow? we would like to adopt esbuild but since we have different react packages we can't use it because it generate multiple instances of react.

Same issue here. Causes huge problems for projects with sensitive dependencies... wasm, three.js, react etc. Have to go back to other bundlers for now ☹

Is this in the roadmap somehow? we would like to adopt esbuild but since we have different react packages we can't use it because it generate multiple instances of react.

My way of avoiding the "multiple instances of react" is to use React CDN, then use inject to add global React.
Anyway, it's always a waste of bundle size when there's multiple duplicated imports.

vite seems to be working fairly well with sane configuration, and running fast using esbuild under the hood. I didn't get on so well with Snowpack for some reason.

Using a CDN & inject also seems like not a bad idea for a specific library like React. Indeed I haven't looked into it, but that sounds like something cleaner than my three shim.

Trying to inject React globally just results in:

Uncaught SyntaxError: Identifier 'React' has already been declared

Yep, kinda stuck on how to proceed here. We're largely using cjs and I dunno how feasible it would be to transform the entire codebase to satisfy esm. We also have other global imports like jQuery, Promise, Immutable and the like, and it seems like if every chunk (when using code splitting) gets fully injected with the import it seems like a huge increase in bundle sizes.

I've got an app that depends on the solid library. Building with esbuild produces a bundle with code sections like this:

  // src/app/Main.jsx
  init_define_config();
  init_web();
  init_web();
  init_web();
  init_solid();
  init_dist();

  // src/app/parts/Header.jsx
  init_define_config();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();

  // src/app/parts/InfoBox.jsx
  init_define_config();
  init_web();
  init_web();
  init_web();

  // src/app/parts/ProfileMenu.jsx
  init_define_config();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();
  init_web();

I'm having some difficulty understanding exactly what is going on. Perhaps the JSX transform is producing duplicate imports and esbuild then doesn't consolidate them?

Have any update about this issues? I have same problem(duplicate import) tooπŸ˜…

Indeed, I think this is preventing us from packaging a react component library with esbuild and then being liberal in who can import, e.g., a webpack consumer is giving warnings of more than one copy of React due to failing pointer equality checks on React: https://reactjs.org/warnings/invalid-hook-call-warning.html . Hooks seem to require us to make React external with pointer equality, which makes this bug an issue.

Am working on a minimal test case, but basically:

Library (esbuild):

"module": "dist/index.pure.esm.js",
"scripts": {
"build:esm:js": "esbuild --bundle src/index.js       --sourcemap --target=es6 --loader:.js=jsx --format=esm  --external:react --external:react-dom --outfile=dist/index.pure.esm.js",
},
 "peerDependencies": {
    "react": ">=16.8.6",
    "react-dom": ">=16.8.6"
  },
   "devDependencies": {
    "react": ">=17.0.2",
    "react-dom": ">=17.0.2",

Consumer (CRA):

  "dependencies": {
    "@mylib": "file:/mylib",
    "react": "^17.0.2",
    "react-dom": "^17.0.2",

We want to support multiple consumers -- it's an OSS lib -- with at least cjs + esm as officially support. But right now can't get any combo of esbuild component => wp4 CRA consumer. Right now we are trying random --format etc options to see if we can push anything through.

Edit: Why I think it may be this -- console.log(window.reactDomPatchReact == window.componentPatchReact, window.componentPatchReact == window.appPatchReact ) => false, false: React is loaded, but different references

Ah maybe false alarm in our case: facebook/react#13991 (comment)

I would love an update on this too, I do have several libraries duplicated :(

Any updates on this? πŸ‘€ I have the same issue when building a react component library:

// bundle.js
// src/Button/Button.tsx
import React from "react";
var Button = () => /* @__PURE__ */ React.createElement("p", null, "This is the Button");

// src/Avatar/Avatar.tsx
import React2 from "react";
var Avatar = () => /* @__PURE__ */ React2.createElement("p", null, "This is the avatar");
export {
  Avatar,
  Button
};

@evanw I saw you've mentioned that this issue has something to do with rewriting the linker that hasn't landed yet.

Is there an issue I can subscribe to in the meantime? :-)

agcty commented

@evanw any updates on this or something that we can help with?

Unless I am missing something or there is a workaround this basically makes it impossible to use esbuild for any react project that makes use of an internal component or common hooks library.

I posted a workaround in remix-run/remix#2987 (comment)

I posted a workaround in remix-run/remix#2987 (comment)

I can't say if that's a workaround since it only works on a single environment.

I posted a workaround in remix-run/remix#2987 (comment)

I can't say if that's a workaround since it only works on a single environment.

This was based on an earlier comment in this issue, you should be able to adapt it to other environments within your esbuild config

evanw commented

Unless I am missing something or there is a workaround this basically makes it impossible to use esbuild for any react project that makes use of an internal component or common hooks library.

Two import statements for the same module does not cause any observable behavior changes (unless your JS runtime is seriously broken). So this is an issue for aesthetics and code size, but it should not impact correctness. If you do have an example case where it impacts correctness, please post it here.

agcty commented

@evanw we were able to pinpoint the issue now, seems like a react shim clashes with some imports. you're right, it doesn't affect correctness, sorry about all the confusion!

Any progress ?

I found in remix app that the generated build/index.js file contains duplicate symbols, but require() module is different package:

// a.tsx
var import_react = require('react'), ...


//  b.tsx
var import_react = require('@remix-run/react'), ...

// c.tsx
var import_react2 = require('react'), ...

This was fixed for me in remix@1.7.0 remix-run/remix#2987

This was fixed for me in remix@1.7.0 remix-run/remix#2987

I observed this problem because I upgraded Remix from 1.6.x to 1.7 the day before yesterday

I am also interested in resolving this issue because it makes my resulting javascript file to weight ~100MB.

Update: I was able to resolve the issue, I've put details here rails/jsbundling-rails#143

evanw commented

I am also interested in resolving this issue because it makes my resulting javascript file to weight ~100MB.

This issue is talking about what happens when you bundle multiple files with import "external" where external has been marked as external (i.e. excluded from the bundle). The problem is that currently esbuild will keep each of these import statements in the bundle instead of merging them (compare the "actual" and "expected" behaviors above). This issue isn't high priority because it typically only results in a few extra duplicate statements, which typically only adds a small additional size to the bundle but otherwise has no effect (since in JavaScript, multiple import statements for the same file only imports that file once).

Are you saying that a significant portion of your 100mb bundle is only external import statements? Even 50mb would be a crazy number of import statements. For example, the text import "react" is about 15 characters long. If there was one import "react" per file, you'd need to bundle over 3 million separate files containing only import "react" to reach 50mb.

@allomov It sounds like you're somehow getting a 100mb bundle that consists almost completely of duplicated external import statements (since that's what this issue you commented on is about). This is very surprising. Can you describe more how this is happening in your code base? It would be helpful if you could provide a code sample that reproduces this issue.

@evanw thank you for the answer. My problem was only partly related to esbuild. There were several reasons for having such a large resulting bundle:

  1. we also use MUI and some other UI libs, which were primarily imported in each file, and it appeared the smallest output file we had was 2 MB in size
  2. we used index.js in each folder for module export. These files were empty but were also compiled and added size to the "composed" output file.

To give you more context, we've migrated a Ruby on Rails application with tons of legacy code, and we could easily switch from webpack to esbuild. I used this article to get an idea of how to do it in a new way https://gorails.com/episodes/esbuild-jsbundling-rails. To get the production build, sprockets gathered files from the output and glued them in one applicaiton.js file. In our case, there were ~50 such files, and each contained isolated external imports, which caused the final bundle to have such a large file. To cope with this issue, I needed to re-organize imports: I added one main.js file that imported all other files and asked esbuild to work only with this one file. The fix works quickly, and the final size of the uncompressed bundle is 1.5 MB.

I'm having this issue importing and bundling @vaadin/vaadin-core web components into my app. Web components can only be registered once. I have created a sample project here to reproduce this issue: https://github.com/nsainaney/esvaadin

When running the app locally, you get the following error in the browser console:

Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "vaadin-lumo-styles" has already been used with this registry
    at http://127.0.0.1:8080/lib/index.js:11413:18
    at http://127.0.0.1:8080/lib/index.js:83160:3

The reason for the above error is web-components should only be registered once and the generated bundle imports the following imports multiple times:

  // node_modules/@vaadin/vaadin-lumo-styles/version.js
  var Lumo = class extends HTMLElement {
    static get version() {
      return "24.0.1";
    }
  };
  customElements.define("vaadin-lumo-styles", Lumo);

...

// node_modules/@vaadin/vaadin-core/node_modules/@vaadin/avatar/node_modules/@vaadin/vaadin-lumo-styles/version.js
  var Lumo2 = class extends HTMLElement {
    static get version() {
      return "24.0.1";
    }
  };
  customElements.define("vaadin-lumo-styles", Lumo2);
evanw commented

@nsainaney I believe that's irrelevant to this issue. This issue is about external modules (i.e. those that aren't in the bundle).

In addition, it looks like esbuild is working correctly in your case. If there are two separate files on the file system and you import both of them, then esbuild will include both files in the bundle. To have that not happen you'd have to either only have one file on the file system (e.g. reconfigure your package manager to not install the same package multiple times) or rewrite the two import paths to point to a single file (e.g. with an esbuild plugin). It would be invalid for esbuild to do this automatically because JavaScript requires each separate module identifier to become a separate module.

I see - thank you for the prompt response.

is there any update?

@rashidul-xs I was able to optimize the file size by inspecting the dependencies and removing duplications. I guess that could be only my case.

I see this a lot even with internal node packages, here path is imported 3 times:

...
import path from "path";
import path2 from "path";
import { createHash } from "crypto";
import { hostname } from "os";
import { readFileSync } from "fs";
import path3 from "path";
import { createRequire } from "module";
import { createHash as createHash2 } from "crypto";
import fs from "fs/promises";
import { tmpdir } from "os";
import { join } from "path";
function isIn2(value, array) {
  return array.includes(value);
  ...

I created a plugin to address this issue. You can find it here: esbuild-plugin-rdi. This ESBuild plugin removes duplicate require statements from the minified build.

The tests are currently failing as my GitHub actions quota was drained due to a script stuck at postinstall for several hours. However, you can manually verify that test are passing and the plugin works as expected.

is there any update?

I created a plugin to address this issue. You can find it here: esbuild-plugin-rdi. This ESBuild plugin removes duplicate require statements from the minified build.