Reduce bundle size before release candidates go live - #5197
Merged
Conversation
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Resultscreate10kduration
usedJSHeapSize
filter-listduration
usedJSHeapSize
hydrate1kduration
usedJSHeapSize
many-updatesduration
usedJSHeapSize
replace1kduration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
text-updateduration
usedJSHeapSize
tododuration
usedJSHeapSize
update10th1kduration
usedJSHeapSize
|
getDomSibling, updateParentDomPointers and useId's root walk all compared against null where the values can only be a vnode/DOM node or null, so a plain truthiness check is equivalent. preact 4549 -> 4548 B br hooks 1331 -> 1324 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only converted comparisons whose operands are already known to be the same type (typeof results, for-in keys, string literals, numbers, object identity). Value comparisons that can see coercible operands (value === true/false/null/'', defaultProps undefined checks, prop diffing) stay strict. Core was left alone: the same change measured +1 B br there. compat 3490 -> 3482 B br hooks 1324 -> 1322 B br jsx-runtime 740 -> 735 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
options._render already clears _pendingArgs on every render, and nothing reads it between options.diffed and the next options._render, so clearing it a second time in diffed is dead work. hooks 1322 -> 1316 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…guards - `'setAttribute' in value == !!nodeType` is implied by the branch that follows it: `localName` only exists on elements and `nodeType == 3` only on text nodes, so the `in` check can never change the outcome. - reuse the `i = 'value'` local for `removeAttribute` - fold the two `isClassComponent &&` guards into one block preact 4548 -> 4533 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The do/while advanced oldDom in the body and re-tested in the condition; doing the advance inside the condition is equivalent and compresses far better than the do/while form. preact 4533 -> 4519 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both call sites just call removeNode for every entry (removeNode already no-ops on null), so the manual reverse index loops can be replaced by `excessDomChildren.some(removeNode)`. Order is irrelevant: each entry is a distinct node and removeNode only detaches it from its parent. preact 4519 -> 4502 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- the resume scan's `depth > 0` loop condition is dead: depth only reaches zero through the `break` that follows the decrement - flatten the catch-side close-marker branch into a single condition and use `!--commentMarkersToFind` instead of `--commentMarkersToFind == 0` preact 4502 -> 4494 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- _catchError: getDerivedStateFromError/componentDidCatch can only be a function or undefined, so truthiness suffices - children: `childVNode.ref || NULL` instead of the ternary - setStyle: normalise a nullish value once instead of per branch - portal container swap: Array#some instead of forEach preact 4494 -> 4491 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`subs = null` on the Provider's componentWillUnmount was added in #4396 as a hot path: back then `subs` was an array and each unmounting consumer paid an O(n) `splice(indexOf(c), 1)`, so nulling the list short-circuited an O(n^2) teardown. `subs` is a Set now and `delete` is O(1), so the short-circuit no longer buys anything and the null guard in the consumer wrapper can go too. The Set is still released with the Provider instance: every subscriber is a descendant, so they all unmount and remove themselves first. preact 4491 -> 4483 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`firstChildDom = firstChildDom || newDom` is equivalent to the guarded assignment, and `_nextState` is only ever an object or null so the explicit null comparison in setState is redundant. preact 4483 -> 4481 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The replaceNode path (non-hydrating render into a container that already has DOM) is the only caller, so trading the index loop for a copied NamedNodeMap is cheap and compresses better. The callback captures `oldProps`, which V8 then context-allocates in diffElementNodes — a synthetic micro-benchmark of that shape runs ~50% slower. It does not show up in practice: create10k, replace1k, hydrate1k, update10th1k, filter-list and many-updates are all statistically unchanged against the same build without this. preact 4481 -> 4472 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
encodeEntities is only reachable through jsxAttr/jsxEscape, which exist for the precompiled JSX transform (pracht) — nothing in preact, preact/compat or preact-render-to-string calls them, so the hand-rolled charCodeAt scan is paying for a path that only precompiled templates hit. Keeping the `test()` early-out is what matters: it keeps entity-free strings (the common case) on the fast path — dropping it makes them ~2.5x slower. Strings that do need escaping get ~1.7x slower, which is the trade being made here. `&` is replaced first so the inserted entities aren't re-escaped. jsx-runtime 735 -> 656 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The callback captures `oldDom` so V8 context-allocates it in constructNewChildrenArray, which is the hottest function in the diff — but it does not show up in the repo benchmarks: replace1k, create10k, hydrate1k, update10th1k, filter-list and many-updates are all statistically unchanged against the same build without this (tachometer, 20+ samples, chrome headless). preact 4472 -> 4465 B br Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JoviDeCroock
force-pushed
the
JoviDeCroock/reduce-bundle-size
branch
from
August 6, 2026 16:00
130129b to
532e114
Compare
JoviDeCroock
force-pushed
the
JoviDeCroock/reduce-bundle-size
branch
from
August 7, 2026 03:16
532e114 to
8a6e574
Compare
|
Size Change: -78 B (-0.49%) Total Size: 15.9 kB 📦 View Changed
ℹ️ View Unchanged
|
rschristian
reviewed
Aug 7, 2026
The `.some(arrow)` rewrites in `diffElementNodes` and `constructNewChildrenArray` both capture a local of their enclosing function (`oldProps` and `oldDom`). V8 has to context-allocate any variable an inner closure touches, so both functions started heap- allocating a Context on every call and reading those locals through it — including the hot `for (i in oldProps)` loop that runs for every element we diff. Measured against main with the repo benchmarks (tachometer, chrome 151): replace1k was 20-22% slower and hydrate1k 7.5-10.7% slower with 5-15% more heap; both are statistically unchanged again after this. Costs 5 B brotli back on core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The chained-replace version is functionally equivalent (fuzz-verified over 211k inputs) but 2-6x slower whenever the string actually contains an entity, and the -84 B brotli it bought never reaches bundled apps: esbuild and rollup both tree-shake encodeEntities out entirely for a jsx/Fragment-only import. The bytes only landed for unbundled consumers (Deno, esm.sh, no-build) — i.e. exactly the precompile users who run this in their SSR hot path — so they paid the slowdown to collect them. Keeps the loose-equality golf from 5e96b7a.
marvinhagemeister
approved these changes
Aug 7, 2026
|
coverage: 99.297% (-0.002%) from 99.299% — JoviDeCroock/reduce-bundle-size into main |
Merged
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
4 participants
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.
Ran an autoresearch loop with Claude Opus 5 and came to the following result.
Currently looking through perf regressions