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.32%↓
dist/prod -1.45%↓
smoke-tests/v2-app-hello-world-template/dist -31%↓
🤖 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
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>
fcead41 to
b5304e9
Compare
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.
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.