Declare side-effecting modules so bundlers have better hints for what can be tree-shaken#21365
Open
NullVoxPopuli wants to merge 6 commits intomainfrom
Open
Declare side-effecting modules so bundlers have better hints for what can be tree-shaken#21365NullVoxPopuli wants to merge 6 commits intomainfrom
NullVoxPopuli wants to merge 6 commits intomainfrom
Conversation
Contributor
📊 Size reportTarball size — dist/dev -1.2%↓
dist/prod -1.31%↓
smoke-tests/v2-app-hello-world-template/dist -37.5%↓
🤖 This report was automatically generated by wyvox/pkg-size |
2 tasks
This was referenced May 3, 2026
NullVoxPopuli
commented
May 4, 2026
Contributor
Author
There was a problem hiding this comment.
a manual lint, of sorts
fcead41 to
b5304e9
Compare
Two purely-additive bundler hints that let consumer bundlers (vite/rolldown, webpack, esbuild) tree-shake aggressively through ember-source's internal barrel re-exports without requiring any source-file restructuring. 1. **`"sideEffects": false` on `ember-source/package.json`** - declares that no module in this package has top-level side effects that need to be preserved if the module's exports are unused. The bundler can then DCE re-exports through `index.ts` barrels that currently anchor the rest of the graph in place. This is safe in practice because rollup's chunking groups symbols with their side effects: any chunk containing the classic `Component` class also contains the `setInternalComponentManager(CURLY_COMPONENT_MANAGER, Component)` call, the `setHelperManager` registrations live in the chunk that holds `helper.ts`, etc. Importing a symbol from a chunk pulls the chunk's side effects along; apps that don't reach those symbols don't need their side effects either. 2. **`treeshake.moduleSideEffects` callback in `rollup.config.mjs`** - the package-level `sideEffects: false` declarations on `@glimmer/debug`, `@glimmer/debug-util`, and `@glimmer/local-debug-flags` get lost when rolldown emits shared chunks (debug code from these packages can leak into chunks that the renderer-only path then pulls in). The callback re-asserts module purity at the chunk level so leaked debug code drops out of the renderer-only path. Measured against `smoke-tests/v2-app-hello-world-template`: | | raw | gzip | | - | - | - | | before | 251.05 KB | 79.75 KB | | after | 172.99 KB | 55.31 KB | Classic `v2-app-template` and v1 `app-template` smoke tests still build and pass. `pnpm test:node` 20/20. `pnpm vite build --mode development` (full dev test suite app) builds clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`sideEffects: false` declares a contract that's invisible to reviewers: any module-level mutation (`set*Manager(...)`, `Foo.reopenClass(...)`, `_backburner = new Backburner(...)`) silently rots the contract. Bundlers will drop these side effects when the symbol they ride along with isn't imported. Adds a `pnpm test:tree-shake` script that bundles each pure-today entry point with rollup as if a consumer imported it for side effects only, and asserts nothing survives. Regression in any known-pure entry fails CI with an actionable message. Today the list is 23 entries — `@glimmer/util`, `@glimmer/destroyable`, `@ember/owner`, `@ember/version`, etc. The 43 currently-impure entries (@ember/-internals/glimmer, @ember/runloop, @glimmer/runtime, …) are omitted because they have known top-level side effects that sideEffects: false promises bundlers can drop in practice anyway. Patch: agadoo's bundled acorn pins ecmaVersion 11 (ES2020), which chokes on private class fields and other ES2022+ syntax that appears in dist. The patch bumps it to `'latest'`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Of the three entries previously in PURE_INTERNAL_PACKAGES, only `@glimmer/debug` actually contributes to the hello-world bundle size: | array contents | hello-world raw | gzip | | - | - | - | | `[]` (no callback) | 181.12 KB | 57.82 KB | | `['@glimmer/debug-util']` | 181.12 KB | 57.82 KB | | `['@glimmer/local-debug-flags']` | 181.12 KB | 57.82 KB | | `['@glimmer/debug']` | 172.99 KB | 55.35 KB | | all three | 172.99 KB | 55.31 KB | `@glimmer/debug-util` and `@glimmer/local-debug-flags` are already inferred pure by rolldown's static analysis (their content either has no top-level statements that look like side effects, or gets fully stripped by the LOCAL_DEBUG macro in prod builds). Re-asserting them adds nothing. Inlined the array since the callback now checks a single path prefix. Added a note explaining the measurement so the next person adding to the list does so with data, not pattern-matching. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e6f0453 to
31fccff
Compare
Contributor
Author
|
The lint for this was extracted to: #21378 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
The key thing is that the side-effects signal is only relevant if nothing is imported from a file.
Demo:
bundler hints that let consumer bundlers (vite/rolldown, webpack, esbuild) tree-shake aggressively through ember-source's internal barrel re-exports without requiring any source-file restructuring.
for the ember-source dist/tarball, the result is that chunks shake out a bit differently.
on webpack docs:
on svelte's docs
https://github.com/sveltejs/kit/pull/10691/changes#diff-4b03f147d0e7e4750663fcdb0ef3bf8ba11ad0bc47204456bf5d7aa5fd9a7e00R129
They do recommend explicitly declaring which modules have side-effects (which is something I want to do, I just want it generated)
this person says sideEffects is "standard" vitejs/vite#14321 (comment)
from rollup:
I believe this means that it happens that if something is used from a file, it can have side-effects within that file -- which is good.
Because, say you never use helpers:
we want:
to be removed, because you never used them.
let's say that file also registers helper managers.
the output removes the import entirely.
but because you don't use helpers, there is no behavior loss.
If you did
then the file is used, and any side-effects within would remain, because deeper analysis occurs.
however, if we declared
@ember/helperas having side-effects in pacakge.json, we'd likely not get the correct stripping behavior that we want.so, it is "less scary" to have all side-effects actually live in private files so that this "did you import it?" check and deeper analysis trigger isn't directly controlled by users