Skip to content

Commit 8138dab

Browse files
authored
Merge pull request #4706 from solidjs/claude/project-thread-uehqk6
fix: live server components in a windowed view, and a broadcast's late joiner sent the table twice
2 parents cf839ea + d06429b commit 8138dab

11 files changed

Lines changed: 518 additions & 77 deletions

‎.changeset/broadcast-join-once.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/web": patch
3+
---
4+
5+
A viewer that joins a running `broadcast` is sent each live range once, as it shows now. Its snapshot used to carry the frame as it first rendered and then every changed hole whole again, so a late joiner downloaded a big live table twice (about 600 ms to join a 2,000-row table, now about 120 ms).
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/web": patch
3+
---
4+
5+
A live server component's attribute whose value is still pending at the top of the render (no `<Loading>` around it) no longer writes the live-hole comment markers inside the tag or the attribute value (`<div<!--lh:1--> style="…"<!--lh:/1-->>`), which broke the element. The element keeps its attribute address, and the binding registers once the tag's pending values resolve, so later values update the attribute as an `attr` op.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"solid-js": patch
3+
"@solidjs/web": patch
4+
---
5+
6+
A `dynamic` mount of a live server component that switches arguments and comes back to its first call (a windowed view scrolling back to the slice the page was served with) shows that call again. The equals gate compared the incoming binding with the one the memo holds, which is the first resolution's, so the way back read as "nothing changed" and the mount stayed on the other address.
7+
8+
A live connection cancelled before its body's `start` arrived now reads as a death, and the loop reconnects. A remount replaying a warm store gives up on a row-op record by cancelling the new connection so the resume re-sends the hole whole; with nothing open yet, the end read as a completion and the mount was left with no live connection.

‎packages/solid/src/client/dynamic.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,18 @@ export function dynamicCore(
8989
// stale binds the fresh mount to the first call's resident store (the SSR
9090
// payload) while the refetched response warms a store nothing reads.
9191
let deliveredAddress: string | undefined;
92+
// The value a resolution just kept (and delivered past), for the gate.
93+
let kept: any;
9294
const resolveBinding = (next: any, prev: any) => {
9395
const binding = bindingOf(next);
9496
if (!binding) return next;
9597
deliveredAddress = binding.address;
9698
const prevBinding = bindingOf(prev);
9799
if (prevBinding && prevBinding.component === binding.component) {
98100
for (const deliver of sites) deliver(binding.address);
101+
// Delivered: the gate that sees this kept value next must not read
102+
// its (first-resolution) address as a new one.
103+
kept = prev;
99104
return prev;
100105
}
101106
return next;
@@ -116,12 +121,22 @@ export function dynamicCore(
116121
// nothing delivered (no site mounted) a differing address is a plain
117122
// change — nothing is kept, so nothing is lost by swapping.
118123
const sameInstance = (prev: any, next: any) => {
119-
if (prev === next) return true;
124+
if (next === kept) {
125+
kept = undefined;
126+
if (prev === next) return true;
127+
}
120128
const held = bindingOf(prev);
121129
const incoming = bindingOf(next);
122-
if (!held || !incoming || held.component !== incoming.component) return false;
123-
if (held.address === incoming.address) return true;
124-
if (deliveredAddress === undefined) return false;
130+
if (!held || !incoming || held.component !== incoming.component) return prev === next;
131+
// Nothing delivered yet: the held address is the one showing.
132+
if (deliveredAddress === undefined) return held.address === incoming.address;
133+
// Otherwise the delivered address is, and the held one says nothing: an
134+
// instance that moved away from its first call and comes back to it
135+
// receives a binding whose address equals the held one, and must still
136+
// be delivered there — often as the very binding object it holds (one
137+
// per address), so identity says nothing either. (A windowed live view
138+
// scrolling back to the slice the page was served with stayed on the
139+
// slice it had scrolled to.)
125140
if (incoming.address !== deliveredAddress) {
126141
deliveredAddress = incoming.address;
127142
for (const deliver of sites) deliver(incoming.address);

‎packages/solid/test/dynamic-component.spec.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,51 @@ describe("dynamicComponent", () => {
8787
expect(calls).toBe(1);
8888
expect(address()).toBe("b");
8989
});
90+
91+
test("a live source coming back to its first call's address delivers it again", async () => {
92+
// A `live` server component's loop yields one binding object per
93+
// address, straight to the factory memo's equals gate. The memo HOLDS
94+
// its first resolution, so an instance that moved to another address
95+
// and comes back receives the very binding it holds: the gate has to
96+
// deliver it all the same (a windowed view scrolling back to the slice
97+
// the page was served with stayed on the one it scrolled to).
98+
let calls = 0;
99+
let address!: () => string;
100+
const mount = (_props: { n: string }, next: () => string) => {
101+
calls++;
102+
address = next;
103+
return "mounted";
104+
};
105+
const bindings = { a: binding(mount, "a"), b: binding(mount, "b") };
106+
const liveOf = (key: "a" | "b") => ({
107+
async *[Symbol.asyncIterator]() {
108+
yield bindings[key];
109+
await new Promise(() => {});
110+
}
111+
});
112+
const [which, setWhich] = createSignal<"a" | "b">("a");
113+
let view!: () => string;
114+
createRoot(() => {
115+
const Comp = dynamicComponent(() => liveOf(which()) as any);
116+
view = Comp({ n: "1" }) as () => string;
117+
});
118+
const settle = async () => {
119+
for (let i = 0; i < 5; i++) {
120+
await Promise.resolve();
121+
flush();
122+
}
123+
};
124+
await settle();
125+
expect(view()).toBe("mounted");
126+
expect(address()).toBe("a");
127+
setWhich("b");
128+
flush();
129+
await settle();
130+
expect(address()).toBe("b");
131+
setWhich("a");
132+
flush();
133+
await settle();
134+
expect(address()).toBe("a");
135+
expect(calls).toBe(1);
136+
});
90137
});

‎packages/web/frames/src/frame-sink.ts‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3321,19 +3321,19 @@ function snapshot(shared, have) {
33213321
if (!open || open.id !== id) out.push((open = { id, version, r: {} }));
33223322
open.r[k] = record;
33233323
};
3324-
if (!conditional) {
3325-
for (const { id, version, k, record } of shared.records.values()) put(id, version, k, record);
3326-
return out;
3327-
}
33283324
// The root and each fragment as they show now: every later hole record
3329-
// spliced into whichever of them holds its range, in write order.
3325+
// spliced into whichever of them holds its range, in write order (a
3326+
// container written after a hole record already shows it, or newer).
33303327
const containers = [];
3331-
for (const e of shared.records.values())
3332-
if (e.id === shared.id && (e.k === "" || /^seg:[^:]+$/.test(e.k)) && e.record)
3333-
containers.push({ e, value: e.record.value });
3334-
for (const { k, record } of shared.records.values()) {
3335-
if (!k.startsWith("hole:lh:") || k.endsWith(":error") || !record || record.value == null)
3328+
const spliced = new Set();
3329+
for (const entry of shared.records.values()) {
3330+
const { id, k, record } = entry;
3331+
if (id !== shared.id || !record) continue;
3332+
if (k === "" || /^seg:[^:]+$/.test(k)) {
3333+
if (typeof record.value === "string") containers.push({ e: entry, value: record.value });
33363334
continue;
3335+
}
3336+
if (!k.startsWith("hole:lh:") || k.endsWith(":error") || record.value == null) continue;
33373337
const n = k.slice(8);
33383338
const startMark = `<!--lh:${n}-->`;
33393339
const endMark = `<!--lh:/${n}-->`;
@@ -3343,9 +3343,37 @@ function snapshot(shared, have) {
33433343
const end = c.value.indexOf(endMark, start);
33443344
if (end < 0) continue;
33453345
c.value = c.value.slice(0, start + startMark.length) + record.value + c.value.slice(end);
3346+
spliced.add(entry);
33463347
break;
33473348
}
33483349
}
3350+
if (!conditional) {
3351+
// The frame whole, its containers showing their holes' latest content:
3352+
// a joiner is sent each range once, not as it first rendered and again
3353+
// as it shows now. The root keeps its digest (holes don't move the
3354+
// skeleton); its hole ledger is what it now holds.
3355+
for (const e of shared.records.values()) {
3356+
const { id, version, k, record } = e;
3357+
if (spliced.has(e)) continue;
3358+
const c = containers.find(c => c.e === e);
3359+
if (c && c.value !== record.value) {
3360+
const holes = {};
3361+
if (record.holes)
3362+
for (const key in record.holes)
3363+
if (key.startsWith("lha:")) holes[key] = record.holes[key];
3364+
put(
3365+
id,
3366+
version,
3367+
k,
3368+
withHoles(
3369+
{ ...record, value: c.value, digest: k ? textDigest(c.value) : record.digest },
3370+
holeDigests(c.value, null, holes)
3371+
)
3372+
);
3373+
} else put(id, version, k, record);
3374+
}
3375+
return out;
3376+
}
33493377
for (const e of shared.records.values()) {
33503378
const { id, version, k, record } = e;
33513379
if (id !== shared.id) {

‎packages/web/frames/src/wire-tier.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,13 @@ export function connect(
352352
const dead = error || cancelled || new Error("Frame stream ended before the frame completed.");
353353
connection.done = true;
354354
resolveEnd({
355-
open: open.size,
355+
// A cancel is always a death the loop reconnects from (supersession,
356+
// or a mount giving up on a row-op record so the resume re-sends the
357+
// hole whole), even when it lands before the body's `start` did: a
358+
// remount replaying a warm store gives up as its new connection is
359+
// still at the header, and an end with nothing open would read as
360+
// completion, leaving the mount with no live connection at all.
361+
open: cancelled !== undefined ? Math.max(open.size, 1) : open.size,
356362
error: dead,
357363
sweep: () => {
358364
for (const [id, version] of open)

0 commit comments

Comments
 (0)