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

re-exported namespaces cannot be tree-shaked #1420

Open
Conaclos opened this issue Jul 4, 2021 · 15 comments
Open

re-exported namespaces cannot be tree-shaked #1420

Conaclos opened this issue Jul 4, 2021 · 15 comments

Comments

@Conaclos
Copy link

Conaclos commented Jul 4, 2021

Hi!

I am thinking about switching to esbuild. However I noticed a bump in the size of my projects when I switch to esbuild. A little investigation leads me to the following conclusion: esbuild does not tree-shake star-exports. And unfortunately I am heavily use star-export for namespacing.

Example

Consider a project with three files: constants.js, namespaced-constants.js and entry.js.
constants.js exports X and Y, namespaced-constants.js exports these two constants under the namespace constants, and entry.js exports only X using the namspace constants.

// constants.js
export const X = "X"
export const Y = "Y"
// namespaced-constants.js
export * as constants from "./constants.js"
// entry.js
import { constants } from "./namespaced-constants.js"
export const X = constants.X

esbuild generates the following code:

// npx esbuild entry.js --format=esm --bundle
var __defProp = Object.defineProperty;
var __markAsModule = (target) => __defProp(target, "__esModule", { value: true });
var __export = (target, all) => {
  __markAsModule(target);
  for (var name in all)
    __defProp(target, name, { get: all[name], enumerable: true });
};

// constants.js
var constants_exports = {};
__export(constants_exports, {
  X: () => X,
  Y: () => Y
});
var X = "X";
var Y = "Y";

// entry.js
var X2 = constants_exports.X;
export {
  X2 as X
};

Y is not removed of the bundle while the constant is not used.

In contrast, rollup and webpack support tree-shaking for star-export. For example, rollup generates the following code:

// npx rollup entry.js
const X$1 = "X";

const X = X$1;

export { X };

esbuild is already a fantastic tool. It shines by its simplicity. It could be great to get better tree-shaking.

@evanw
Copy link
Owner

evanw commented Jul 5, 2021

This is a known limitation with esbuild. Tree shaking is supported with import * as x but not when x is then re-exported and re-imported. Imports are only tracked through one level, not through multiple levels. This is a fairly advanced optimization; it was only added in Webpack 5 for example, and is not present in Webpack 4 (see webpack/webpack#9607).

@dzearing
Copy link

dzearing commented Nov 4, 2021

One minor note; perhaps title should read "re-exported namespaces cannot be tree-shaked". If you export * from the source rather than the namespace itself as a named export, tree-shaking does work.

// namespaced-constants.js
export * from "./constants.js"
// index.js
export { X } from "./namespaced-constants.js"

So it's not necessarily that star-exports can't be tree shaken but rather that namespaces exported as a named export won't get tree shaken when a subset of their content is consumed.

I did validate that running the example listed in the bug through webpack with experimental module output did produce a tree-shaken result:

var o="X";export{o as X};

@Conaclos Conaclos changed the title star-exports cannot be tree-shaked re-exported namespaces cannot be tree-shaked Nov 6, 2021
@jonyw4
Copy link

jonyw4 commented Feb 9, 2022

Any thoughts on how to optimize the tree shaking?

@jeron-diovis
Copy link

jeron-diovis commented Jan 26, 2023

One minor note; perhaps title should read "re-exported namespaces cannot be tree-shaked". If you export * from the source rather than the namespace itself as a named export, tree-shaking does work.

// namespaced-constants.js
export * from "./constants.js"
// index.js
export { X } from "./namespaced-constants.js"

So it's not necessarily that star-exports can't be tree shaken but rather that namespaces exported as a named export won't get tree shaken when a subset of their content is consumed.

I did validate that running the example listed in the bug through webpack with experimental module output did produce a tree-shaken result:

var o="X";export{o as X};

@dzearing How does it work for you?

I'm playing with Vite now (which uses esbuild internally, afaik), and with setup like this:

// lib/lib_a.js
export function helper_a() {}

// lib/lib_b.js
export function helper_b() {}

// lib/index.js
export * from './lib_a'
export * from './lib_b'

// my_entry.js
import { helper_a } from './lib'

I have following results:

  • if helper_a is not used in module, it gets eliminated, and neither lib_a nor anything else is downloaded
  • if helper_a IS used – then lib/index.js, lib/lib_a.js, lib/lib_b.js are all downloaded

idk, maybe I need to enable some special build option, same as with webpack?

@simonwep
Copy link

Unfortunately we rely on even renaming exports (for example the default export) - and we use it extensively in combination with vue and components (where you re-export the default of a component to get a named component). This causes our bundle to grow indefinitely and breaks tree-shaking almost entirely (since most modules are components that are re-exported this way).

Is there anything planned in this regard? Or any update to this?

@gustavopch
Copy link

I'd love to have ESBuild properly tree-shake namespace imports because I think namespaces are just a superior way to deal with module dependencies, mainly because they avoid name conflicts.

For example, suppose I have two functions called format in different modules – one in currency.js and another in date.js.

  • If I use named imports, I need to alias them, possibly to formatCurrency and formatDate. Not great because I may eventually alias to a different name like formatMoney and formatDay and then I have the same function being called by different names across the codebase.
  • Another approach is to just declare them as formatCurrency and formatDate in their modules to avoid the need to alias when importing, but then I have to always keep in mind all possible name conflicts in my codebase when I'm naming anything. Not great also.

On the other hand, with namespaces, I can just have import * as Lib from './lib' and call Lib.Currency.format(input) and Lib.Date.format(input). That way, name conflicts are basically gone. One less thing to worry about.

@NfNitLoop
Copy link

(I mentioned it in my own ticket (linked above) but suppose I should add it here as well for visibility.)

Package exports are a recommended pattern for managing dependencies in deps.ts files in Deno, so Deno folks are going to be particularly prone to running into this issue.

@lazarljubenovic
Copy link

lazarljubenovic commented Jun 19, 2023

This is a known limitation with esbuild. Tree shaking is supported with import * as x but not when x is then re-exported and re-imported. Imports are only tracked through one level, not through multiple levels. This is a fairly advanced optimization; it was only added in Webpack 5 for example, and is not present in Webpack 4 (see webpack/webpack#9607).

If deep tracking is currently out of scope, maybe a good starting point would be an opt-in flag which would treat namespaced imports not as objects, but as aliases (assignments of sorts) right from the start (from within, i.e. from the namespaced export; not from the usage), essentially completely bypassing the object semantics:

TRY
// entry.js
import * as Functions from './functions'
console.log(Functions.add(1, 2))

// functions.js
export function add (a, b) { return a + b }
export function sub (a, b) { return a - b }
Proposed output
  (() => {
    // functions.js
-  function add(a, b) {
+  function Functions$add(a, b) {
      return a + b;
    }

    // entry.js
-   console.log(add(1, 2));
+   console.log(Functions$add(1, 2));
  })();

With this transformation in place, it should be much easier to "propagate" the tree-shaking mechanism through modules and namespaced re-exports:

TRY
// entry.js
import { Functions } from './api'
console.log(Functions.add(1, 2))

// api.js
export * as Functions from './functions'

// functions.js
export function add (a, b) { return a + b }
export function sub (a, b) { return a - b }
Proposed output
  (() => {
-   var __defProp = Object.defineProperty;
-   var __export = (target, all) => {
-     for (var name in all)
-       __defProp(target, name, { get: all[name], enumerable: true });
-   };

    // functions.js
-   var functions_exports = {};
-   __export(functions_exports, {
-     add: () => add,
-     sub: () => sub
-   });
-   function add(a, b) {
+   function Functions$add(a, b) {
      return a + b;
    }
-   function sub(a, b) {
-     return a - b;
-   }

    // entry.js
-   console.log(functions_exports.add(1, 2));
+   console.log(Functions$add(1, 2));
  })();

The issue is, of course, that you're losing the compliant semantics of an object (spec calls them exotic objects) when used directly. This would require the object to exist, and would have to throw an error when the flag is turned on.

TRY
// entry.js
import * as Functions from './functions'
console.log(Functions)

// functions.js
export function add (a, b) { return a + b }
export function sub (a, b) { return a - b }
Output
(() => {
  var __defProp = Object.defineProperty;
  var __export = (target, all) => {
    for (var name in all)
      __defProp(target, name, { get: all[name], enumerable: true });
  };

  // functions.ts
  var functions_exports = {};
  __export(functions_exports, {
    add: () => add,
    sub: () => sub
  });
  function add(a, b) {
    return a + b;
  }
  function sub(a, b) {
    return a - b;
  }

  // entry.ts
  console.log(functions_exports);
})();

But this kind of usage is rare, and would be properly documented in the gotchas of opting in. esbuild would raise an error if it encounters a namespace used without accessing a concrete symbol outside the import and export statements and suggest either changing the code or turning the optimization off.

After this API is stabilized, it's "just" (ha) a matter of following the namespaced (re-re-)export through the code and checking if it's ever used directly as an object. This seems to be how Rollup does it. Comment the last line from the entry module to see the difference. Also notice how, irrespective of its presence, Functions.add gets compiled into just add.

@Conaclos
Copy link
Author

I like your idea!

What is preventing to do both? i.e. combining your approach with the current one.

Taking the following oinput:

// entry.js
import { Functions } from './api'
console.log(Functions.add(1, 2))
console.log(Functions) // use namespace as object

// api.js
export * as Functions from './functions'

// functions.js
export function add (a, b) { return a + b }
export function sub (a, b) { return a - b }

The emitted code could then be:

  (() => {
    var __defProp = Object.defineProperty;
    var __export = (target, all) => {
      for (var name in all)
        __defProp(target, name, { get: all[name], enumerable: true });
  };

    // functions.js
    var functions_exports = {};
    __export(functions_exports, {
-    add: () => Functions$add,
-    sub: () => Functions$sub,
+    add: () => Functions$add,
+    sub: () => Functions$sub,
    });
-   function add(a, b) {
+   function Functions$add(a, b) {
       return a + b;
    }
-   function sub(a, b) {
+   function Functions$sub((a, b) {
      return a - b;
   }

    // entry.js
-   console.log(functions_exports.add(1, 2));
+   console.log(Functions$add(1, 2));
    console.log(functions_exports);
  })();

@ibrahimqasim
Copy link

Are there any updates on if this is a planned optimization?
If it is a speed issue it might be fine to have this enabled with a separate flag in order to not affect general bundling performance.
Like many in this thread, I'm working with code that has large amounts of reexported namespaces, and the only build tool available is esbuild, so this feature would be extremely useful for properly bundling those modules.

@mohamedmansour
Copy link

Are there any updates on if this is a planned optimization?

If it is a speed issue it might be fine to have this enabled with a separate flag in order to not affect general bundling performance.

Like many in this thread, I'm working with code that has large amounts of reexported namespaces, and the only build tool available is esbuild, so this feature would be extremely useful for properly bundling those modules.

Just make sure you are importing the files you need and not index.js because once you import index.js and one of the exports are sideffects, then it will import it still.

@ibrahimqasim
Copy link

Are there any updates on if this is a planned optimization?
If it is a speed issue it might be fine to have this enabled with a separate flag in order to not affect general bundling performance.
Like many in this thread, I'm working with code that has large amounts of reexported namespaces, and the only build tool available is esbuild, so this feature would be extremely useful for properly bundling those modules.

Just make sure you are importing the files you need and not index.js because once you import index.js and one of the exports are sideffects, then it will import it still.

What do you mean by this?
If index.js contains an aliased exported module, then the that module will be included in its entirety already, even if only part of it is used.

@ibrahimqasim
Copy link

I would also suggest that this limitation be documented here until it can be addressed.

@UNDERCOVERj
Copy link

any updates header

@danielo515
Copy link

This is something I will also look implemented. Subscribing to this

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