From 5894f2a90103ab4c048483a2b4205148fc785616 Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:08:35 -0700 Subject: [PATCH] fix(store): non-configurable property descriptor and stale indices after array truncation Two proxy invariant bugs in the store proxy. getOwnPropertyDescriptor (issue #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 #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 #2770 Fixes #2768 --- .changeset/fix-store-proxy-invariants.md | 5 +++ packages/solid-signals/src/store/store.ts | 31 ++++++++++++++++-- .../tests/store/createStore.test.ts | 32 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-store-proxy-invariants.md diff --git a/.changeset/fix-store-proxy-invariants.md b/.changeset/fix-store-proxy-invariants.md new file mode 100644 index 000000000..dad3d774d --- /dev/null +++ b/.changeset/fix-store-proxy-invariants.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +store: fix two proxy invariant bugs — non-configurable property descriptor and stale indices after array truncation diff --git a/packages/solid-signals/src/store/store.ts b/packages/solid-signals/src/store/store.ts index 374b9a349..b0a79cdf9 100644 --- a/packages/solid-signals/src/store/store.ts +++ b/packages/solid-signals/src/store/store.ts @@ -578,6 +578,21 @@ export const storeTraps: ProxyHandler = { override[property] = value; if (nextLength !== undefined) override.length = nextLength; } + // When shrinking an array's length, mark truncated indices as deleted in the + // override so that `has`, `ownKeys`, and index reads correctly reflect the + // shorter array rather than leaking stale entries from the underlying value. + if ( + Array.isArray(state) && + property === "length" && + typeof value === "number" && + typeof prev === "number" && + value < prev + ) { + const override = target[overrideKey] || (target[overrideKey] = Object.create(null)); + for (let i = value; i < prev; i++) { + if (i in state) override[i] = $DELETED; + } + } notifyStoreProperty(target, property, "set", value, prev, prevHas); // notify length change if (Array.isArray(state) && property !== "length" && nextLength !== undefined) { @@ -690,7 +705,9 @@ export const storeTraps: ProxyHandler = { // Get base descriptor structure, override just the value const baseDesc = getPropertyDescriptor(target[STORE_VALUE], target[STORE_OVERRIDE], property); if (baseDesc) { - return { ...baseDesc, value: target[STORE_OPTIMISTIC_OVERRIDE][property] }; + const targetDesc = Reflect.getOwnPropertyDescriptor(target, property); + const configurable = !targetDesc || targetDesc.configurable ? true : baseDesc.configurable; + return { ...baseDesc, configurable, value: target[STORE_OPTIMISTIC_OVERRIDE][property] }; } return { value: target[STORE_OPTIMISTIC_OVERRIDE][property], @@ -699,7 +716,17 @@ export const storeTraps: ProxyHandler = { configurable: true }; } - return getPropertyDescriptor(target[STORE_VALUE], target[STORE_OVERRIDE], property); + const desc = getPropertyDescriptor(target[STORE_VALUE], target[STORE_OVERRIDE], property); + // The proxy target is an internal node object, not the original source. When the + // source has a non-configurable property that does not also exist as non-configurable + // on the proxy target, the proxy invariant is violated: the engine requires that a + // property reported as non-configurable must actually be non-configurable on the + // target object. Override configurable to true only in that case. + if (desc && !desc.configurable) { + const targetDesc = Reflect.getOwnPropertyDescriptor(target, property); + if (!targetDesc || targetDesc.configurable) return { ...desc, configurable: true }; + } + return desc; }, getPrototypeOf(target) { diff --git a/packages/solid-signals/tests/store/createStore.test.ts b/packages/solid-signals/tests/store/createStore.test.ts index 4a5416d13..face8f3b0 100644 --- a/packages/solid-signals/tests/store/createStore.test.ts +++ b/packages/solid-signals/tests/store/createStore.test.ts @@ -1319,3 +1319,35 @@ describe("derived store manual writes", () => { expect(store.count).toBe(2); }); }); + +describe("Proxy invariant correctness", () => { + test("Object.keys does not throw for stores wrapping objects with non-configurable own properties", () => { + // A non-configurable property on the source must not cause the getOwnPropertyDescriptor + // proxy trap to violate the proxy invariant when the store proxy target is a plain object + // that does not have the property as non-configurable. + const source: any = {}; + Object.defineProperty(source, "id", { + value: 1, + enumerable: true, + writable: true, + configurable: false + }); + const [store] = createStore(source); + expect(() => Object.keys(store)).not.toThrow(); + expect(store.id).toBe(1); + }); + + test("Truncating array length clears stale indices from the store proxy", () => { + // Writing store.length = N must mark indices >= N as deleted so that `has`, + // index reads, and Object.keys all reflect the shorter array. + const [list, setList] = createStore([1, 2, 3]); + setList(s => { + s.length = 2; + }); + flush(); + expect(list.length).toBe(2); + expect(list[2]).toBeUndefined(); + expect(2 in list).toBe(false); + expect(Object.keys(list).filter(k => k !== "length")).not.toContain("2"); + }); +});