evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
38.13k stars 1.15k forks source link

Unnecessary resolutions in sideEffects: false package #3255

Open privatenumber opened 1 year ago

privatenumber commented 1 year ago

When re-exporting nothing from a package that has sideEffects disabled, I would expect esbuild to discard the export completely. However, it seems to try to resolve all paths within the package unnecessarily.

I think this has a non-trivial performance impact on WASM.

Reproduction

export {} from "viem"

https://stackblitz.com/edit/stackblitz-starters-b1bwlb?file=index.mjs

evanw commented 1 year ago

Tree shaking is supposed to be an optimization that doesn't affect the behavior of the build. In particular, the build should fail if some part of the module graph contains a syntax error. So esbuild deliberately crawls the entire module graph before tree shaking begins.

privatenumber commented 1 year ago

I appreciate your perspective on this! I believe it could be a missed chance to get more out of sideEffect: false and further optimize build performance.

Wouldn't someone deliberately writing an empty import/export from a side-effect free package suffice as a signal to disregard the statement?

I do understand that type imports are treated as annotations, but if having type is sufficient to drop the statement and not follow the import, it seems there's enough of a signal here as well. What do you think?