fix: vnode clone compatible with frozen intrinsics#5078
Draft
kumavis wants to merge 1 commit into
Draft
Conversation
kumavis
changed the title
📊 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
|
kumavis
marked this pull request as ready for review
April 26, 2026 03:30
|
coverage: 98.544% (+0.02%) from 98.528% — kumavis:fix-hardenedjs-compat into preactjs:main |
Contributor
Author
|
requesting a review from @JoviDeCroock (bc of recent change to vnode constructor logic (strict equality)) |
Member
|
While I think the fix makes sense, there's a site that has been forgotten by the LLM. |
Contributor
Author
good catch, thanks for the review.
can you help me understand this? sounds like 3rd parties are also cloning nodes, but I don't get the "through option hooks" part. the good news is this change would not break those ecosystem libraries, and users running in HardenedJS environments get a working core to help the ecosystem we could export |
kumavis
force-pushed
the
fix-hardenedjs-compat
branch
3 times, most recently
from
May 1, 2026 22:11
4b73972 to
00658dc
Compare
kumavis
marked this pull request as draft
May 1, 2026 22:18
Member
|
The through options hook part is mostly the Do note that this is a PR for the main branch which is currently v11 (beta) and we also have the v10.x branch which is the mainline atm. |
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.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude <noreply@anthropic.com>
kumavis
force-pushed
the
fix-hardenedjs-compat
branch
from
June 12, 2026 11:00
00658dc to
a628f41
Compare
1 task
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
3 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.
Disclaimer: I (a human) identified this issue years ago and have always just worked around it via a local patch. I used an LLM (opus 4.7) to generate the fix. I reviewed the change and wrote most of the PR description.
This PR fixes a compatibility issue when Preact is run in Hardened JavaScript environments such as:
The compatibility issue comes from a javascript quirk known as "the override mistake" related to frozen intrinsics. For this bug, the minimal environment change is just
Object.freeze(Object.prototype).In this environment, assigning the
constructorproperty via assignment syntax (=) orObject.assigncauses an error.Specifically,
Object.assign({}, vnode)inrenderComponent()(andcloneNodeinsrc/diff/index.js) tries to copy a vnode'sconstructor: undefinedown property via[[Set]]. Under environments with a frozenObject.prototypetheObject.prototype.constructoris non-writable, so the assignment fails.This PR introduces a
cloneVNodeutility that ensuresVNodesare cloned safely. It also reorders theconstructorkey for consistent object shape betweenVNodesand their clones.Note: I erred on the side of excessive documentation in comments, let me know if you would like this reduced.