fix(store): non-configurable property descriptor and stale indices after array truncation#2797
Open
tsushanth wants to merge 1 commit into
Open
fix(store): non-configurable property descriptor and stale indices after array truncation#2797tsushanth wants to merge 1 commit into
tsushanth wants to merge 1 commit into
Conversation
…ter array truncation Two proxy invariant bugs in the store proxy. getOwnPropertyDescriptor (issue solidjs#2770): when the underlying source object has a non-configurable own property (e.g. created with Object.defineProperty configurable:false), the trap forwarded that descriptor verbatim. JavaScript proxy invariants require that if a property is reported as non-configurable, it must exist as non-configurable on the proxy target. The proxy target is an internal node object, not the original source, so this violated the invariant and caused Object.keys (which calls the trap internally) to throw. The fix checks whether the property exists as non-configurable on the proxy target before forwarding; if not, it forces configurable:true in the returned descriptor. The same guard is applied to the optimistic-override fast path. Array truncation via length write (issue solidjs#2768): writing store.length = N on an array store updated the length signal but left indices >= N visible through the proxy. The has trap, ownKeys, and index reads all consulted the underlying source array, which still had the stale entries. The fix marks each truncated index as $DELETED in the override when a length write shrinks the array, so the override layer correctly shadows the stale source entries. Fixes solidjs#2770 Fixes solidjs#2768
🦋 Changeset detectedLatest commit: 5894f2a The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will improve performance by 36.42%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | merge |
331.5 µs | 223.5 µs | +48.29% |
| ⚡ | omit |
215.7 µs | 171.9 µs | +25.5% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing tsushanth:fix/store-proxy-invariants (5894f2a) with next (a4ca10b)
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.
Two proxy invariant bugs in the store proxy, each with a regression test.
Non-configurable property descriptor (fixes #2770)
When the underlying source object has a non-configurable own property (created with
Object.defineProperty(..., { configurable: false })), thegetOwnPropertyDescriptortrap forwarded that descriptor verbatim. JavaScript proxy invariants require that if a property is reported as non-configurable, it must actually exist as non-configurable on the proxy target. Because the proxy target is an internal node object (not the original source), the invariant was violated, andObject.keys— which calls the trap internally — threw aTypeError.The fix checks whether the property already exists as non-configurable on the proxy target before forwarding the source descriptor. If the target does not carry the property as non-configurable, the returned descriptor has
configurable: true. The same guard is applied in the optimistic-override fast path, where a previous unconditional spread ofconfigurable: truecould have the opposite problem for arraylength(whose proxy target carries it as non-configurable).Stale indices after array truncation (fixes #2768)
Writing
store.length = Non an array store updated the length signal but left indices>= Nvisible through the proxy. Thehastrap,ownKeys, and index reads all consulted the underlying source array, which still held the removed entries.The fix marks each truncated index as
$DELETEDin the override whenever alengthwrite shrinks the array. The existing override layer already interprets$DELETEDvalues as absent inhas,ownKeys, andgetKeys, so no further changes are needed downstream.