Skip to content

fix(store): non-configurable property descriptor and stale indices after array truncation#2797

Open
tsushanth wants to merge 1 commit into
solidjs:nextfrom
tsushanth:fix/store-proxy-invariants
Open

fix(store): non-configurable property descriptor and stale indices after array truncation#2797
tsushanth wants to merge 1 commit into
solidjs:nextfrom
tsushanth:fix/store-proxy-invariants

Conversation

@tsushanth

Copy link
Copy Markdown

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 })), the getOwnPropertyDescriptor trap 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, and Object.keys — which calls the trap internally — threw a TypeError.

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 of configurable: true could have the opposite problem for array length (whose proxy target carries it as non-configurable).

Stale indices after array truncation (fixes #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 held the removed entries.

The fix marks each truncated index as $DELETED in the override whenever a length write shrinks the array. The existing override layer already interprets $DELETED values as absent in has, ownKeys, and getKeys, so no further changes are needed downstream.

…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-bot

changeset-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5894f2a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@solidjs/signals Patch
test-integration Patch
solid-js Patch
babel-preset-solid Patch
@solidjs/web Patch
@solidjs/html Patch
@solidjs/h Patch
@solidjs/universal Patch
@solidjs/element Patch

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

@codspeed-hq

codspeed-hq Bot commented Jun 25, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 36.42%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
✅ 116 untouched benchmarks

Performance Changes

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)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant