-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Comments
This is a known limitation with esbuild. Tree shaking is supported with |
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 var o="X";export{o as X}; |
Any thoughts on how to optimize the tree shaking? |
@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:
idk, maybe I need to enable some special build option, same as with webpack? |
Unfortunately we rely on even renaming exports (for example the Is there anything planned in this regard? Or any update to this? |
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
On the other hand, with namespaces, I can just have |
(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 |
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, |
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);
})(); |
See [evanw/esbuild#1420](evanw/esbuild#1420). Signed-off-by: William So <polyipseity@gmail.com>
Are there any updates on if this is a planned optimization? |
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? |
I would also suggest that this limitation be documented here until it can be addressed. |
any updates header |
This is something I will also look implemented. Subscribing to this |
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
andentry.js
.constants.js
exportsX
andY
,namespaced-constants.js
exports these two constants under the namespaceconstants
, andentry.js
exports onlyX
using the namspaceconstants
.esbuild generates the following code:
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:
esbuild is already a fantastic tool. It shines by its simplicity. It could be great to get better tree-shaking.
The text was updated successfully, but these errors were encountered: