Add the constructor in when cloning VNodes - #5195
Merged
Merged
Conversation
Object.assign({}, vnode) in renderComponent(), cloneNode in
src/diff/index.js, and detachedClone in compat/src/suspense.js
tries to copy a vnode's `constructor: undefined` own property via
[[Set]]. Under environments where `Object.prototype.constructor`
is a non-writable data property (e.g. raw
`Object.freeze(Object.prototype)` or hardenedjs configured with
`overrideTaming: 'min'`), the assignment trips the override
mistake and fails with:
TypeError: Cannot assign to read only property 'constructor'
Initial render works because createVNode builds vnodes with an
object literal ([[CreateDataProperty]], not Set), but any
setState/forceUpdate-triggered re-render goes through
Object.assign({}, oldVNode) and trips the override mistake.
Note: hardening configs that tame the override mistake by
replacing `Object.prototype.constructor` with an accessor pair
(SES default, Node's `--frozen-intrinsics`) do NOT exhibit this
bug — but the fix is still required for the untamed configs
above. The added Node subprocess test guards against future
changes to that taming.
Fixes #5109
Co-authored-by: kumavis <kumavis@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Inline `assign({ constructor: UNDEFINED }, vnode)` at the three clone
sites instead of routing them through a shared `cloneVNode` helper.
compat can't reach a core-internal helper across the `preact` external
boundary, so the helper ends up duplicated into the compat bundle:
4555 B / 3518 B brotli (core/compat) with the helper vs 4550 B / 3510 B
inline, against 4547 B / 3490 B on main. Net cost of the fix is +3 B
core, +20 B compat.
Drop the `constructor`-first reordering of createVNode's object literal.
`%HaveSameMap` shows a cloned vnode never shares a hidden class with
createVNode's literal, with or without the reorder — V8 gives literals a
different in-object slot count than transition-built objects — so it
buys no shape identity. Clone-vs-clone stays monomorphic either way, and
a microbench (node 22, best of 7, 2M clones) puts the seeded clone at
374 ms against 428 ms for `assign({}, v)`, ~13% faster with or without
the reorder. Leaving the key order alone also keeps the createElement
key-order test untouched.
Replace the node tests with browser tests that exercise the actual
failure. `frozen-intrinsics.test.js` cannot reproduce the bug — node
tames `Object.prototype.constructor` into an accessor pair under
`--frozen-intrinsics`, as its own comment notes — and it reaches for
esbuild, which is not a declared devDependency;
`object-prototype-freeze.test.js` covered the clone helper rather than a
render. The new tests instead redefine `Object.prototype.constructor` as
non-writable (the narrowest reproduction of the override mistake, and
reversible, so the shared browser realm isn't poisoned) and drive
setState, a Fragment-returning component (cloneNode) and Suspense
(detachedClone). All three fail on main and pass here.
Checked end to end in Chromium under real SES `lockdown()` and under
bare `Object.freeze(Object.prototype)`: hydrate, class + hooks +
context, memo/forwardRef, portals, keyed reordering, controlled inputs,
error boundaries and lazy/Suspense all run clean, so this clone was the
only hardening hazard on those paths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📊 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
|
|
Size Change: +17 B (+0.11%) Total Size: 15.9 kB 📦 View Changed
ℹ️ View Unchanged
|
JoviDeCroock
marked this pull request as ready for review
August 6, 2026 13:49
marvinhagemeister
approved these changes
Aug 6, 2026
This was referenced Aug 6, 2026
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
2 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.
Fixes #5109
Supersedes #5078
This fixes the SES setup and has the nice benefit of positively affecting perf for low byte-size