diff --git a/.changeset/live-for-async-reads.md b/.changeset/live-for-async-reads.md new file mode 100644 index 0000000000..b0ea206f9c --- /dev/null +++ b/.changeset/live-for-async-reads.md @@ -0,0 +1,5 @@ +--- +"solid-js": patch +--- + +Server `For` in a live hole: a row that read an async memo or projection besides its item, or whose item was edited in place, now renders again on each sweep instead of reusing stale markup. diff --git a/packages/solid/src/server/signals.ts b/packages/solid/src/server/signals.ts index 3f01bfa116..70f9c8bc0c 100644 --- a/packages/solid/src/server/signals.ts +++ b/packages/solid/src/server/signals.ts @@ -685,6 +685,9 @@ interface ServerComputation { // hook (`ctx.commitEpoch`) only exists where a binding ledger is live. sync?: boolean; epoch?: number; + // The value can change between live-hole sweeps: the result is async, or + // a compute read something that is (see asyncReads). + moving?: boolean; // Runs when the owner disposes the compute (see `armDispose`), for work // that cannot wait for its next pull to notice `disposed`: a pumped // iterator parked on a `next()` that will not settle until the world @@ -1178,6 +1181,13 @@ function warnServerWrite(category: "signal" | "store" | "optimistic"): void { }); } +// Reads of values that can change between two live-hole sweeps, counted so +// a sweep can tell a `For` row that read one (see mapArray). Only async +// sources move on the server — a plain signal or store has no writer once +// the render runs — so this counts reads of async memos and async +// projections, and of any memo whose last compute read one of those. +let asyncReads = 0; + export function createSignal(): Signal; export function createSignal(value: Exclude, options?: SignalOptions): Signal; // Commit #0 (loadingValue) removes the uninitialized window: never undefined @@ -1324,10 +1334,12 @@ export function createMemo( resetOwnerForRerun(owner); const prev = sharedConfig.context; sharedConfig.context = ctx; + const reads = asyncReads; try { return runWithOwner(owner, () => runWithObserver(comp, () => comp.compute(comp.value))); } finally { sharedConfig.context = prev; + if (asyncReads !== reads) comp.moving = true; } }; @@ -1346,8 +1358,10 @@ export function createMemo( typeof result === "object" && (typeof (result as any)[Symbol.asyncIterator] === "function" || typeof (result as any).then === "function") - ) + ) { + comp.moving = true; armDispose(); + } processResult( comp, result, @@ -1412,6 +1426,7 @@ export function createMemo( // as epoch comparison, no subscriber graph needed. update(); } + if (comp.moving) asyncReads++; if (comp.errored) { // A client hole read outside a Loading discovery pass must fail loudly // rather than wedge the stream (see clientHoleRead). Identity check on @@ -1472,6 +1487,8 @@ function createSyncMemo( let cached = false; // The commit epoch `value`/`error` was computed at (frame renders only). let epoch: number | undefined; + // See ServerComputation.moving. + let moving = false; function pull(): T | undefined { // Inlined `runWithOwner` — avoids the per-pull `() => compute(value)` @@ -1487,6 +1504,7 @@ function createSyncMemo( // created by the eventual successful pull drift ahead of the client's // (#2801, generalized to all retry paths in #2900). resetOwnerForRerun(owner); + const reads = asyncReads; try { value = compute(value) as T; error = undefined; @@ -1504,6 +1522,7 @@ function createSyncMemo( throw err; } finally { currentOwner = prev; + if (asyncReads !== reads) moving = true; } } @@ -1516,6 +1535,7 @@ function createSyncMemo( } return (() => { + if (moving) asyncReads++; if (cached && (!ctx?.commitEpoch || ctx.commitEpoch() === epoch)) { if (errored) throw error; return value; @@ -2473,7 +2493,7 @@ function createPendingProxy( let status: 0 | 1 | 2 = 0; let error: any; let readTarget: T = state; - const gate = () => { + const open = () => { if (status > 1) { // A derive that landed on a client hole (settleServerAsync's FINAL // reclassification, #3659) errors with the tagged NotReady: the same @@ -2487,6 +2507,11 @@ function createPendingProxy( if (source === CLIENT_HOLE) clientHoleRead(); throw new NotReadyError(source); }; + // Every read counts as an async read (see asyncReads), ready or not. + const gate = () => { + asyncReads++; + open(); + }; const proxy = new Proxy(state, { get(obj, key, receiver) { gate(); @@ -3160,15 +3185,21 @@ export function mapArray( // (a live response, a standing source) re-evaluates its whole value on // every commit, and without this every row re-ran `mapFn` each time — a // list's render cost paid per write. During a sweep a row whose item at - // the same index is unchanged — the same reference, or a shallow-equal - // object, since a live source usually hands back a fresh snapshot — reuses - // the row it rendered on the previous sweep. Sweeps only: a - // sweep's evaluation is mint-suppressed and owner-free (the engine's + // the same index is unchanged reuses the row it rendered on the previous + // sweep. Unchanged is deep equality with a copy of the item taken when the + // row rendered — a live source usually hands back a fresh snapshot, and a + // server store may edit the same object in place. A row reused must also + // have read no async source while rendering (an async memo or projection, + // or a memo derived from one: see asyncReads), since only those change + // between sweeps; such a row renders again every sweep. This is the + // client's rule at row grain: a row is kept while its item is, and what + // else it read re-runs it. Sweeps + // only: a sweep's evaluation is mint-suppressed and owner-free (the engine's // impurity gate closes any hole that creates owners), so a row from one // sweep carries no markers or bindings that another could duplicate. The // first render and its retries never read or fill the cache, so their // ids and minting are exactly as before. - let kept: { items: readonly T[]; rows: U[] } | null = null; + let kept: { items: unknown[]; rows: U[]; pure: boolean[] } | null = null; const read = createMemo( () => { const items = list(); @@ -3176,6 +3207,8 @@ export function mapArray( const live = (sharedConfig.context as any)?.liveHoles; const sweeping = !!(live && live.sweeping); const prev = sweeping ? kept : null; + const snaps: unknown[] = []; + const pure: boolean[] = []; // Reset per pass so retries re-synthesize row/fallback ids from 0, // like the client's fresh row owners on its first successful pass. rowOwner._childCount = 0; @@ -3186,10 +3219,18 @@ export function mapArray( const origId = ownerId(rowOwner); try { for (let i = 0, len = items.length; i < len; i++) { - if (prev && i < prev.items.length && sameItem(prev.items[i], items[i])) { + if ( + prev && + i < prev.items.length && + prev.pure[i] && + sameItem(prev.items[i], items[i]) + ) { s.push(prev.rows[i]); + snaps.push(prev.items[i]); + pure.push(true); continue; } + const reads = asyncReads; if (origId !== undefined) { rowOwner.id = formatChildId(origId, i); } @@ -3210,6 +3251,10 @@ export function mapArray( ? (mapFn as (v: T, i: Accessor) => U)(items[i], () => i) : (mapFn as (v: T) => U)(items[i]) ); + if (sweeping) { + snaps.push(copyItem(items[i], 0)); + pure.push(asyncReads === reads); + } } } finally { rowOwner.id = origId; @@ -3220,7 +3265,7 @@ export function mapArray( const fo = runWithOwner(rowOwner as unknown as Owner, () => createOwner()); s.push(runWithOwner(fo, () => options.fallback!()) as U); } - kept = sweeping && items && items.length ? { items: items.slice(), rows: s } : null; + kept = sweeping && items && items.length ? { items: snaps, rows: s, pure } : null; return s; }, { sync: true } @@ -3228,15 +3273,30 @@ export function mapArray( return read; } -// A sweep's row reuse test (see mapArray): the same value, or two plain -// objects/arrays whose own entries are the same values. -function sameItem(a: unknown, b: unknown): boolean { - if (a === b) return true; +// A sweep's row reuse test (see mapArray): `a` is the copy `copyItem` took +// when the row rendered, `b` the item now; deep equality over plain objects +// and arrays. Past the depth limit (or a cycle) the copy holds NOT_COPIED, +// which equals nothing, so the row renders again. +const NOT_COPIED = {}; +const COPY_DEPTH = 8; +function copyItem(v: any, depth: number): unknown { + if (!v || typeof v !== "object") return v; + if (depth > COPY_DEPTH) return NOT_COPIED; + if (Array.isArray(v)) return v.map(x => copyItem(x, depth + 1)); + const proto = Object.getPrototypeOf(v); + // Class instances (a Date, a Map) are not copied: the row renders again. + if (proto !== Object.prototype && proto !== null) return NOT_COPIED; + const out: Record = {}; + for (const k in v) out[k] = copyItem(v[k], depth + 1); + return out; +} +function sameItem(a: any, b: any): boolean { + if (a === b) return a !== NOT_COPIED; if (!a || !b || typeof a !== "object" || typeof b !== "object") return false; - if (Array.isArray(a) !== Array.isArray(b)) return false; + if (a === NOT_COPIED || Array.isArray(a) !== Array.isArray(b)) return false; const ka = Object.keys(a); if (ka.length !== Object.keys(b).length) return false; - for (const k of ka) if ((a as any)[k] !== (b as any)[k] || !(k in b)) return false; + for (const k of ka) if (!(k in b) || !sameItem(a[k], b[k])) return false; return true; } diff --git a/packages/web/test/server/live-hole-for-rows-4056.spec.tsx b/packages/web/test/server/live-hole-for-rows-4056.spec.tsx index 7b50a1006d..79e3b4cf39 100644 --- a/packages/web/test/server/live-hole-for-rows-4056.spec.tsx +++ b/packages/web/test/server/live-hole-for-rows-4056.spec.tsx @@ -9,7 +9,7 @@ * previous sweep. The emitted markup is unchanged. */ import { expect, test } from "vitest"; -import { createMemo, For } from "solid-js"; +import { createMemo, createStore, For } from "solid-js"; import { Loading } from "@solidjs/web"; import { renderServerComponent } from "../../frames/src/frame-sink.js"; @@ -186,3 +186,161 @@ test("a row moved to another index renders again", async () => { ); expect(rendered.slice(-2)).toEqual(["B", "A"]); }); + +test("a row that read a memo besides its item renders again on every sweep", async () => { + type View = { items: Message[]; sel: string }; + const ch = channel(); + const rendered: string[] = []; + const Thread = () => { + const view = createMemo(() => ch.iterable); + return ( + wait

}> +
    + + {m => { + rendered.push(m.text); + return
  1. {view().sel === m.id ? "*" : m.text}
    2. ; + }} +     +
+ + ); + }; + const { chunks, done } = consume(renderServerComponent(Thread, { frame: { id: "f" } })); + await tick(); + const a = { id: "a", text: "A" }; + const b = { id: "b", text: "B" }; + ch.push({ items: [a, b], sel: "a" }); + await tick(20); + ch.push({ items: [a, b], sel: "b" }); + await tick(20); + ch.push({ items: [a, b], sel: "x" }); + await tick(20); + ch.end(); + await done; + + // The selection moves with every push though no item changed. + expect(chunks.filter(c => c.type === "hole").map(c => c.html)).toEqual([ + "
  • A
  • *
    • ", + "
  • A
  • B
    • " + ]); + expect(rendered.length).toBeGreaterThan(2); +}); + +test("an item edited in place is not the item the row rendered", async () => { + const ch = channel(); + const rendered: string[] = []; + const Thread = () => { + const view = createMemo(() => ch.iterable); + return ( + wait

    }> +
      + + {m => { + rendered.push(m.text); + return
    1. {m.text}
      2. ; + }} +       +
    +     + ); + }; + const { chunks, done } = consume(renderServerComponent(Thread, { frame: { id: "f" } })); + await tick(); + // One object, edited between pushes — as a store updated in place hands back. + const a = { id: "a", text: "A" }; + const list = [a]; + ch.push(list); + await tick(20); + list.push({ id: "b", text: "B" }); + ch.push(list); + await tick(20); + a.text = "AA"; + ch.push(list); + await tick(20); + ch.end(); + await done; + + expect(chunks.filter(c => c.type === "hole").map(c => c.html)).toEqual([ + "
  • A
  • B
    • ", + "
  • AA
  • B
    • " + ]); + expect(rendered).toEqual(["A", "B", "AA"]); +}); + +test("a row whose memos read only its item is still reused", async () => { + const ch = channel(); + const rendered: string[] = []; + const Thread = () => { + const view = createMemo(() => ch.iterable); + return ( + wait

    }> +
      + + {m => { + rendered.push(m.text); + const loud = createMemo(() => m.text.toUpperCase()); + return
    1. {m.text.length > 1 ? {loud()} : {m.text}}
      2. ; + }} +       +
    +     + ); + }; + const { chunks, done } = consume(renderServerComponent(Thread, { frame: { id: "f" } })); + await tick(); + ch.push([{ id: "a", text: "a" }]); + await tick(20); + ch.push([ + { id: "a", text: "a" }, + { id: "b", text: "bb" } + ]); + await tick(20); + ch.end(); + await done; + + expect(chunks.filter(c => c.type === "hole").map(c => c.html)).toEqual([ + "
  • a
  • BB
    • " + ]); + expect(rendered).toEqual(["a", "bb"]); +}); + +test("a row that read an async store besides its item renders again on every sweep", async () => { + const items = channel(); + const sel = channel(); + const Thread = () => { + const view = createMemo(() => items.iterable); + const [state] = createStore<{ sel?: string }>(async function* (draft) { + for await (const s of sel.iterable) { + draft.sel = s; + yield; + } + }, {}); + return ( + wait

    }> +
      + {m =>
    1. {state.sel === m.id ? "*" : m.text}
      2. }       +
    +     + ); + }; + const { chunks, done } = consume(renderServerComponent(Thread, { frame: { id: "f" } })); + await tick(); + const a = { id: "a", text: "A" }; + const b = { id: "b", text: "B" }; + sel.push("a"); + items.push([a, b]); + await tick(20); + sel.push("b"); + await tick(20); + items.end(); + sel.end(); + await done; + + expect( + chunks + .filter(c => c.type === "hole") + .map(c => c.html) + .at(-1) + ).toBe("
  • A
  • *
    • "); +});