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

Useless repeated imports when using external modules #475

Open
zaygraveyard opened this issue Oct 20, 2020 · 39 comments
Open

Useless repeated imports when using external modules #475

zaygraveyard opened this issue Oct 20, 2020 · 39 comments

Comments

@zaygraveyard
Copy link

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

@zaygraveyard zaygraveyard changed the title [BUG] Usless repeated imports when using external modules Useless repeated imports when using external modules Oct 20, 2020
@evanw
Copy link
Owner

evanw commented Oct 22, 2020

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.

@lxsmnsyc
Copy link

Related: #382

@xinaesthete
Copy link

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.

@AlvinThorn008
Copy link

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

@xinaesthete
Copy link

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

@AlvinThorn008
Copy link

AlvinThorn008 commented Dec 22, 2020

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

@lucas-jones
Copy link

Has there been any update on this?

@huvber
Copy link

huvber commented Jun 17, 2021

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.

@andrewmunro
Copy link

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 ☹

@osddeitf
Copy link

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.

@xinaesthete
Copy link

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.

@neutraali
Copy link

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.

@specious
Copy link

specious commented Nov 8, 2021

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?

@izcream
Copy link

izcream commented Nov 14, 2021

Have any update about this issues? I have same problem(duplicate import) too😅

@lmeyerov
Copy link

lmeyerov commented Dec 4, 2021

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

@lmeyerov
Copy link

lmeyerov commented Dec 4, 2021

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

@Hideman85
Copy link

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

@iampava
Copy link

iampava commented Feb 10, 2022

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

@jaschaio
Copy link

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

@lxsmnsyc
Copy link

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.

@jaschaio
Copy link

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

evanw commented Jun 15, 2022

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

agcty commented Jun 15, 2022

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

@Valexr
Copy link

Valexr commented Aug 23, 2022

Any progress ?

@FlatMapIO
Copy link

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'), ...

@jaschaio
Copy link

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

@FlatMapIO
Copy link

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

@allomov
Copy link

allomov commented Jan 16, 2023

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

evanw commented Jan 17, 2023

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.

@allomov
Copy link

allomov commented Jan 18, 2023

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

@nsainaney
Copy link

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

evanw commented Mar 22, 2023

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

@nsainaney
Copy link

I see - thank you for the prompt response.

@rashidul-xs
Copy link

is there any update?

@allomov
Copy link

allomov commented Oct 5, 2023

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

@u12206050
Copy link

u12206050 commented Apr 12, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests