Skip to content

Commit cbc754c

Browse files
tellahonpub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6wnpub14vtk7pvazqrq9639qu7e560wnqtl0d53ca4gjuvq6jzf3k2el23qqlwa7f
authored
perf(timeline): gate heavy message render behind useDeferredValue (#1022)
Signed-off-by: Taylor Ho <taylorkmho@gmail.com> Co-authored-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub14vtk7pvazqrq9639qu7e560wnqtl0d53ca4gjuvq6jzf3k2el23qqlwa7f <ab176f059d100602ea25073d9a69ee9817f7b691c76a897180d48498d959faa2@sprout-oss.stage.blox.sqprod.co>
1 parent 1164865 commit cbc754c

10 files changed

Lines changed: 701 additions & 31 deletions

desktop/src/features/messages/lib/threadPanel.test.mjs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,114 @@ test("buildThreadPanelData keeps direct comments unindented", () => {
103103
);
104104
});
105105

106+
// Per-id stabilization: thread rows feed `MessageRow` a depth-normalized copy
107+
// of each reply. When `timelineMessages` churns (typing/presence) but the
108+
// reply objects survive by reference, rebuilding the thread panel must hand
109+
// `MessageRow` the SAME normalized object reference so the row/markdown memo
110+
// hits — instead of a fresh `{ ...reply, depth }` spread every render.
111+
test("thread reply objects keep identity across unrelated timelineMessages churn", () => {
112+
const root = message({ id: "root", createdAt: 1 });
113+
const replyA = message({
114+
id: "a",
115+
createdAt: 2,
116+
parentId: "root",
117+
rootId: "root",
118+
depth: 1,
119+
tags: [["e", "root", "", "reply"]],
120+
});
121+
const replyB = message({
122+
id: "b",
123+
createdAt: 3,
124+
parentId: "a",
125+
rootId: "root",
126+
depth: 2,
127+
tags: [["e", "a", "", "reply"]],
128+
});
129+
130+
// First render of the thread.
131+
const first = buildThreadPanelData(
132+
[root, replyA, replyB],
133+
"root",
134+
"root",
135+
new Set(["a"]),
136+
);
137+
138+
// An unrelated channel churn produces a NEW `timelineMessages` array, but the
139+
// reply objects themselves are reused by reference (only their position in
140+
// the surrounding array changed — e.g. a presence ping or typing indicator
141+
// that the snapshot layer leaves the reply identities intact for).
142+
const churned = [
143+
message({ id: "noise", createdAt: 99 }),
144+
root,
145+
replyA,
146+
replyB,
147+
];
148+
const second = buildThreadPanelData(churned, "root", "root", new Set(["a"]));
149+
150+
const firstById = new Map(
151+
first.visibleReplies.map((entry) => [entry.message.id, entry.message]),
152+
);
153+
const secondById = new Map(
154+
second.visibleReplies.map((entry) => [entry.message.id, entry.message]),
155+
);
156+
157+
assert.ok(firstById.size > 0, "expected at least one visible reply");
158+
for (const [id, normalized] of firstById) {
159+
assert.strictEqual(
160+
secondById.get(id),
161+
normalized,
162+
`normalized reply ${id} must be the SAME object reference across an unrelated churn (memo hit)`,
163+
);
164+
// Depth must still reach the row correctly via the cached object.
165+
assert.equal(
166+
typeof normalized.depth,
167+
"number",
168+
`normalized reply ${id} must carry a numeric depth`,
169+
);
170+
}
171+
});
172+
173+
test("thread reply objects recompute when the source reply object is replaced", () => {
174+
const root = message({ id: "root", createdAt: 1 });
175+
const reply = message({
176+
id: "a",
177+
createdAt: 2,
178+
parentId: "root",
179+
rootId: "root",
180+
depth: 1,
181+
tags: [["e", "root", "", "reply"]],
182+
});
183+
184+
const first = buildThreadPanelData([root, reply], "root", "root", new Set());
185+
186+
// A genuine edit/refresh: the reply is a brand-new object (new identity).
187+
const editedReply = message({
188+
id: "a",
189+
createdAt: 2,
190+
parentId: "root",
191+
rootId: "root",
192+
depth: 1,
193+
body: "edited body",
194+
tags: [["e", "root", "", "reply"]],
195+
});
196+
const second = buildThreadPanelData(
197+
[root, editedReply],
198+
"root",
199+
"root",
200+
new Set(),
201+
);
202+
203+
const firstA = first.visibleReplies.find((e) => e.message.id === "a");
204+
const secondA = second.visibleReplies.find((e) => e.message.id === "a");
205+
assert.ok(firstA && secondA, "expected reply 'a' in both renders");
206+
assert.notStrictEqual(
207+
secondA.message,
208+
firstA.message,
209+
"a replaced source reply must produce a fresh normalized object",
210+
);
211+
assert.equal(secondA.message.body, "edited body");
212+
});
213+
106214
test("buildThreadPanelDataFromIndex matches direct panel data", () => {
107215
const root = message({ id: "root", createdAt: 1 });
108216
const directComment = message({

desktop/src/features/messages/lib/threadPanel.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,47 @@ function normalizeHeadMessage(message: TimelineMessage): TimelineMessage {
4747
};
4848
}
4949

50+
// Thread rows feed `MessageRow` a depth-normalized copy of each reply. Building
51+
// that copy fresh (`{ ...message, depth }`) on every render hands `MessageRow` a
52+
// new object identity every time `timelineMessages` churns (typing/presence),
53+
// even when the reply and its depth are byte-identical — which defeats the
54+
// row/markdown memo and forces a ~1.4ms/row re-parse on threads where the main
55+
// timeline (which passes the raw stable ref) stays cheap.
56+
//
57+
// Mirror the main list's per-id context memoization (`videoReviewContextById`):
58+
// cache the normalized object keyed on the source reply identity + depth, so an
59+
// unrelated channel churn that leaves a reply (and its tree position) intact
60+
// reuses the exact same object reference and the memo hits.
61+
//
62+
// Keyed on the source `reply` reference via a WeakMap: a new `timelineMessages`
63+
// set produces new reply objects (genuine recompute), and stale entries are
64+
// collected automatically when the old message set is dropped.
65+
const normalizedInlineReplyCache = new WeakMap<
66+
TimelineMessage,
67+
Map<number, TimelineMessage>
68+
>();
69+
5070
function normalizeInlineReplyMessage(
5171
message: TimelineMessage,
5272
depth: number,
5373
): TimelineMessage {
54-
return {
74+
let byDepth = normalizedInlineReplyCache.get(message);
75+
if (!byDepth) {
76+
byDepth = new Map<number, TimelineMessage>();
77+
normalizedInlineReplyCache.set(message, byDepth);
78+
}
79+
80+
const cached = byDepth.get(depth);
81+
if (cached) {
82+
return cached;
83+
}
84+
85+
const normalized: TimelineMessage = {
5586
...message,
5687
depth,
5788
};
89+
byDepth.set(depth, normalized);
90+
return normalized;
5891
}
5992

6093
function buildDirectChildrenByParentId(messages: TimelineMessage[]) {

0 commit comments

Comments
 (0)