fix(store): snapshot() and store materialization keep symbol-keyed props

.changeset/preserve-store-symbol-snapshots.md

@@ -0,0 +1,7 @@
+---
+"@solidjs/signals": patch
+---
+
+Preserve enumerable symbol-keyed properties when snapshotting and deeply
+tracking stores, including array metadata and untouched nested values. Make
+setter drafts writable when the store type is readonly.

packages/solid-signals/src/store/reconcile.ts

@@ -53,6 +53,8 @@ function addEnumSymbols(o: any, syms: symbol[], keys: Set<PropertyKey>) {
 }
 
 function getAllKeys(value, override, next) {
+  // Symbols are merged explicitly below; keep the common string-key path on
+  // Object.keys() and avoid reflecting the base symbols twice.
   const keys = getKeys(value, override) as PropertyKey[];
   const nextKeys = Object.keys(next);
   // `value` can be a wrapped store (store-in-store) whose ownKeys trap tracks;

packages/solid-signals/src/store/store.ts

@@ -48,7 +48,7 @@ export type Store<T> = Readonly<T>;
  * projection form — `createStore(fn, seed, { key })` or `createProjection` —
  * whose derive function reconciles its return by `options.key`.
  */
-export type StoreSetter<T> = (fn: (state: T) => T | void) => void;
+export type StoreSetter<T> = (fn: (state: Mutable<T>) => Mutable<T> | void) => void;
 /** Tuple returned by the plain `createStore(initialValue)` form. */
 export type StoreReturn<T> = [get: Store<T>, set: StoreSetter<T>];
 /** Tuple returned by the derived `createStore(fn, seed, options?)` form. */
@@ -64,6 +64,8 @@ export interface ProjectionOptions extends StoreOptions {
   key?: string | ((item: NonNullable<any>) => any);
 }
 export type NoFn<T> = T extends Function ? never : T;
+/** Makes the setter draft's top-level properties writable; {@link Store} keeps the getter readonly. */
+export type Mutable<T> = { -readonly [K in keyof T]: T[K] };
 
 type DataNode = Signal<any>;
 type DataNodes = Record<PropertyKey, DataNode>;
@@ -186,7 +188,7 @@ function unwrapStoreValue(value: any, map?: Map<any, any>, lookup?: WeakMap<any,
   map.set(value, result);
   lookup = target[STORE_LOOKUP] ?? storeLookup;
 
-  for (const key of getKeys(source, override)) {
+  for (const key of getStoreKeys(source, override)) {
     if (isArray && key === "length") continue;
     const next = key in override ? override[key] : source[key];
     if (next !== $DELETED) result[key] = unwrapStoreValue(next, map, lookup);
@@ -204,6 +206,29 @@ export function ownEnumerableKeys(o: object): (string | symbol)[] {
   return Reflect.ownKeys(o).filter(k => Object.prototype.propertyIsEnumerable.call(o, k));
 }
 
+// Plain-object variant that keeps Object.keys() as the fast path and only pays
+// descriptor checks for symbols. Do not use this on store proxies: splitting
+// strings/symbols would invoke their ownKeys trap twice.
+function ownEnumerableKeysPlain(o: object): (string | symbol)[] {
+  const keys: (string | symbol)[] = Object.keys(o);
+  const symbols = Object.getOwnPropertySymbols(o);
+  for (let i = 0, len = symbols.length; i < len; i++) {
+    const symbol = symbols[i];
+    if (Object.prototype.propertyIsEnumerable.call(o, symbol)) keys.push(symbol);
+  }
+  return keys;
+}
+
+function ownEnumerableSymbols(o: object): symbol[] {
+  const symbols = Object.getOwnPropertySymbols(o);
+  const result: symbol[] = [];
+  for (let i = 0, len = symbols.length; i < len; i++) {
+    const symbol = symbols[i];
+    if (Object.prototype.propertyIsEnumerable.call(o, symbol)) result.push(symbol);
+  }
+  return result;
+}
+
 /**
  * Single chokepoint for the store's layered value resolution: returns the
  * override layer (optimistic first, then regular) that shadows `property`, or
@@ -378,16 +403,29 @@ function walkAffectsScope(
     collectRecordNodes(target[STORE_NODE], found);
     collectRecordNodes(target[STORE_HAS], found);
     override = mergedOverlay(target);
-    lookup = target[STORE_LOOKUP] ?? lookup;
+    // Carry the effective lookup into untouched descendants. Default stores
+    // use the global lookup just like snapshotImpl; without it, nested raw
+    // objects fall back to string-only enumeration and symbol branches vanish.
+    lookup = target[STORE_LOOKUP] ?? lookup ?? storeLookup;
   }
   if (Array.isArray(raw)) {
     const len = override?.length ?? raw.length;
     for (let i = 0; i < len; i++) {
       const v = override && i in override ? override[i] : raw[i];
       if (v !== $DELETED) walkAffectsScope(v, entry, found, lookup, visited);
     }
+    // Arrays can also carry symbol metadata. Enumerate symbols separately to
+    // avoid scanning large index lists twice. Outside a store tree, retain the
+    // existing index-only walk.
+    const symbols = target || lookup ? getStoreSymbols(raw, override) : [];
+    for (let i = 0, l = symbols.length; i < l; i++) {
+      const key = symbols[i];
+      const desc = getPropertyDescriptor(raw, override, key);
+      if (!desc || desc.get) continue;
+      walkAffectsScope(desc.value, entry, found, lookup, visited);
+    }
   } else {
-    const keys = getKeys(raw, override);
+    const keys = target || lookup ? getStoreKeys(raw, override) : getKeys(raw, override);
     for (let i = 0, l = keys.length; i < l; i++) {
       const desc = getPropertyDescriptor(raw, override, keys[i]);
       if (!desc || desc.get) continue;
@@ -507,17 +545,26 @@ export function mergedOverlay(target: StoreNode): Record<PropertyKey, any> | und
   return override && opt ? { ...override, ...opt } : (opt ?? override);
 }
 
-export function getKeys(
+function getKeysImpl(
   source: Record<PropertyKey, any>,
   override: Record<PropertyKey, any> | undefined,
-  enumerable: boolean = true
+  enumerable: boolean,
+  symbols: boolean
 ): PropertyKey[] {
   // Plain objects can't trigger proxy traps — only pay for the untrack
   // closure when the source is itself a wrapped store (store-in-store).
   const baseKeys = (source as any)[$TARGET]
-    ? untrack(() => (enumerable ? Object.keys(source) : Reflect.ownKeys(source)))
+    ? untrack(() =>
+        enumerable
+          ? symbols
+            ? ownEnumerableKeys(source)
+            : Object.keys(source)
+          : Reflect.ownKeys(source)
+      )
     : enumerable
-      ? Object.keys(source)
+      ? symbols
+        ? ownEnumerableKeysPlain(source)
+        : Object.keys(source)
       : Reflect.ownKeys(source);
   if (!override) return baseKeys;
   const keys = new Set(baseKeys);
@@ -529,6 +576,39 @@ export function getKeys(
   return Array.from(keys);
 }
 
+export function getKeys(
+  source: Record<PropertyKey, any>,
+  override: Record<PropertyKey, any> | undefined,
+  enumerable: boolean = true
+): PropertyKey[] {
+  return getKeysImpl(source, override, enumerable, false);
+}
+
+export function getStoreKeys(
+  source: Record<PropertyKey, any>,
+  override: Record<PropertyKey, any> | undefined
+): PropertyKey[] {
+  return getKeysImpl(source, override, true, true);
+}
+
+export function getStoreSymbols(
+  source: Record<PropertyKey, any>,
+  override: Record<PropertyKey, any> | undefined
+): symbol[] {
+  const symbols = (source as any)[$TARGET]
+    ? untrack(() => ownEnumerableSymbols(source))
+    : ownEnumerableSymbols(source);
+  if (!override) return symbols;
+  const result = new Set(symbols);
+  const overrideSymbols = Object.getOwnPropertySymbols(override);
+  for (let i = 0, len = overrideSymbols.length; i < len; i++) {
+    const symbol = overrideSymbols[i];
+    if (override[symbol] !== $DELETED) result.add(symbol);
+    else result.delete(symbol);
+  }
+  return Array.from(result);
+}
+
 export function getPropertyDescriptor(
   source: Record<PropertyKey, any>,
   override: Record<PropertyKey, any> | undefined,
@@ -1048,7 +1128,7 @@ export function storeSetter<T extends object>(store: Store<T>, fn: (draft: T) =>
  *
  * @returns `[store: Store<T>, setStore: StoreSetter<T>]`
  */
-export function createStore<T extends object = {}>(store: NoFn<T> | Store<NoFn<T>>): StoreReturn<T>;
+export function createStore<T extends object = {}>(store: NoFn<T>): StoreReturn<T>;
 export function createStore<T extends object = {}>(
   fn: (store: T) => void | T | Promise<void | T> | AsyncIterable<void | T>,
   store: Partial<T> | Store<NoFn<T>>,

packages/solid-signals/src/store/utils.ts

@@ -7,6 +7,8 @@ import {
   $TARGET,
   $TRACK,
   getKeys,
+  getStoreKeys,
+  getStoreSymbols,
   getPropertyDescriptor,
   isWrappable,
   mergedOverlay,
@@ -62,14 +64,31 @@ function snapshotImpl<T>(
         result[i] = unwrapped;
       }
     }
+    // Enumerate array symbols separately to avoid scanning indices twice.
+    // Spread copies omit symbols, so assign them after the numeric walk.
+    const symbols = lookup ? getStoreSymbols(item, override) : [];
+    for (let i = 0, l = symbols.length; i < l; i++) {
+      const prop = symbols[i];
+      const desc = getPropertyDescriptor(item, override, prop);
+      if (!desc || desc.get) continue;
+      v = override && prop in override ? override[prop] : item[prop];
+      if (track && isWrappable(v)) wrap(v, target);
+      unwrapped = snapshotImpl(v, track, map, lookup);
+      if (unwrapped !== v || result) {
+        if (!result) map.set(item, (result = Object.assign([...item], item)));
+        result[prop] = unwrapped;
+      }
+    }
     // Deleted trailing slots are skipped above, so restore length to preserve
     // holes instead of truncating the copy (#2846) — mirrors unwrapStoreValue.
     if (result) result.length = len;
   } else if (!override) {
     // Specialized walk for the common no-overlay case (from #2756): the own
     // descriptor gives the value directly, so each property is read once with
     // no overlay membership checks.
-    const keys = getKeys(item, undefined);
+    // A lookup means this object belongs to an immutable store backing tree,
+    // even if that nested value has not needed its own proxy yet.
+    const keys = lookup ? getStoreKeys(item, undefined) : getKeys(item, undefined);
     for (let i = 0, l = keys.length; i < l; i++) {
       const prop = keys[i];
       const desc = Object.getOwnPropertyDescriptor(item, prop)!;
@@ -85,7 +104,9 @@ function snapshotImpl<T>(
       }
     }
   } else {
-    const keys = getKeys(item, override);
+    // An override only exists on a store record, and the target branch above
+    // always set `lookup` alongside it — so this branch is always store-keyed.
+    const keys = getStoreKeys(item, override);
     for (let i = 0, l = keys.length; i < l; i++) {
       let prop = keys[i];
       const desc = getPropertyDescriptor(item, override, prop)!;

packages/solid-signals/tests/question-scoped-pending.test.ts

@@ -612,6 +612,49 @@ describe("affects — captured proxies (#2882)", () => {
     expect(isPending(() => tags.primary)).toBe(false);
   });
 
+  it("affects(plain store) covers an untouched symbol-keyed grandchild", async () => {
+    const meta = Symbol("meta");
+    const [state] = createStore({ outer: { [meta]: { value: 1 } } });
+
+    let resolveIt!: () => void;
+    const act = action(function* () {
+      affects(state);
+      yield new Promise<void>(r => (resolveIt = r));
+    });
+
+    const done = act();
+    flush();
+    const child = state.outer[meta];
+    expect(isPending(() => child.value)).toBe(true);
+
+    resolveIt();
+    await done;
+    flush();
+    expect(isPending(() => child.value)).toBe(false);
+  });
+
+  it("affects(array store) covers symbol-keyed metadata", async () => {
+    const meta = Symbol("meta");
+    const initial = Object.assign([{ value: 0 }], { [meta]: { value: 1 } });
+    const [state] = createStore(initial);
+
+    let resolveIt!: () => void;
+    const act = action(function* () {
+      affects(state);
+      yield new Promise<void>(r => (resolveIt = r));
+    });
+
+    const done = act();
+    flush();
+    const metadata = state[meta];
+    expect(isPending(() => metadata.value)).toBe(true);
+
+    resolveIt();
+    await done;
+    flush();
+    expect(isPending(() => metadata.value)).toBe(false);
+  });
+
   it("keyless mark on a nested record covers its captured subtree, not siblings", async () => {
     const [state] = createStore<{ rows: Row[] }>({ rows: seedRows() });
     const tags0 = state.rows[0].tags;