mirror of
https://github.com/block/buzz.git
synced 2026-10-30 05:51:46 +00:00
fix(desktop): don't cancel the fetch the timeline is waiting on
Converging review found this at P1, two lenses independently, both with
probes against the installed query-core source. Reproduced here.
The prefetch marker records that a channel had a prefetch requested, not
which fetch is in flight. On a cold channel `prefetchQuery` does not start a
fetch at all — TanStack dedupes onto the request already running (query.js
honours `cancelRefetch` only when `state.data !== undefined`). So when the
hover dwell fires after the click, which is the ordinary case for a decisive
click inside the 100ms dwell, the marker labels the mounted timeline's own
fetch a "prefetch", and the post-subscribe refresh cancelled it.
Probe, real modules and a real QueryObserver:
no hover: { calls: 1, mountFetchAborted: false }
hover then click: { calls: 2, mountFetchAborted: true }
That is two sequential relay round trips with the skeleton held across both,
making a hovered channel slower to open than an unhovered one — on the exact
path this stack exists to speed up.
The refresh now waits for the in-flight fetch instead of cancelling it. The
gap it was closing still closes: the invalidate below re-reads the window
after the subscription exists, it just no longer discards the round trip the
user is watching. #6455's one-fetch contract for a cold channel with no
prefetch is unchanged.
The guard's own comment claimed a cold mount fetch "is still parked on the
hydration gate" and so could not be affected. That is false in steady state —
`channelHeadHydration` is an already-resolved promise after boot.
Adds the regression test; it fails on the cancelling version.
Signed-off-by: Max Lampert <maxwell@squareup.com>
This commit is contained in:
@@ -467,3 +467,62 @@ test("gap refresh refetches after an in-flight prefetch settles (no dedupe)", as
|
||||
client.clear();
|
||||
resetChannelWindowPrefetches();
|
||||
});
|
||||
|
||||
test("the post-subscribe refresh never aborts the fetch the timeline is watching", async () => {
|
||||
// Hover dwell fires AFTER the click: prefetchQuery does not start a fetch,
|
||||
// it dedupes onto the mount fetch the user is already watching a skeleton
|
||||
// for. The prefetch marker cannot tell those apart, so the refresh must
|
||||
// wait for the in-flight fetch rather than cancel it — cancelling made a
|
||||
// hovered channel slower to open than an unhovered one.
|
||||
resetChannelWindowPrefetches();
|
||||
const client = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false } },
|
||||
});
|
||||
const channelId = "chan-hover-then-click";
|
||||
const queryKey = channelMessagesKey(channelId);
|
||||
let calls = 0;
|
||||
let mountSignal = null;
|
||||
let release;
|
||||
const gate = new Promise((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
const options = {
|
||||
queryKey,
|
||||
queryFn: async ({ signal }) => {
|
||||
calls += 1;
|
||||
if (calls === 1) {
|
||||
mountSignal = signal;
|
||||
await gate;
|
||||
}
|
||||
return [event(`fetch-${calls}`, 100 + calls)];
|
||||
},
|
||||
staleTime: 300_000,
|
||||
};
|
||||
|
||||
// The click mounts the screen and starts the fetch it will paint.
|
||||
const observer = new QueryObserver(client, options);
|
||||
const unsubscribe = observer.subscribe(() => {});
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
|
||||
// The dwell fires afterwards and merely joins that fetch.
|
||||
markChannelPrefetchStarted(channelId);
|
||||
void client
|
||||
.prefetchQuery(options)
|
||||
.finally(() => markChannelPrefetchSettled(channelId));
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
|
||||
const refresh = refreshChannelWindowMessages(client, channelId);
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
release();
|
||||
await Promise.allSettled([refresh]);
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
assert.equal(
|
||||
mountSignal?.aborted,
|
||||
false,
|
||||
"the mounted timeline's own fetch must not be aborted",
|
||||
);
|
||||
unsubscribe();
|
||||
client.clear();
|
||||
resetChannelWindowPrefetches();
|
||||
});
|
||||
|
||||
@@ -47,11 +47,14 @@ export async function refreshChannelWindowMessages(
|
||||
await query.promise?.catch(() => {});
|
||||
} else if (hasInFlightChannelPrefetch(channelId)) {
|
||||
// A hover prefetch reached the relay before this subscription existed, so
|
||||
// its snapshot can miss events that landed in between. Cancel it so the
|
||||
// invalidate below issues a fetch that starts after this refresh; a cold
|
||||
// mount fetch is still parked on the hydration gate above, carries no such
|
||||
// gap, and keeps deduping.
|
||||
await queryClient.cancelQueries({ queryKey, exact: true });
|
||||
// its snapshot can miss events that landed in between — the invalidate
|
||||
// below re-reads the window after the subscription. Wait for the in-flight
|
||||
// fetch rather than cancelling it: by the time this runs the mounted
|
||||
// timeline has very likely joined that same fetch (TanStack dedupes onto a
|
||||
// cold query's in-flight request rather than starting a second one), so
|
||||
// cancelling would abort the fetch the user is watching a skeleton for and
|
||||
// make a hovered channel slower to open than an unhovered one.
|
||||
await query?.promise?.catch(() => {});
|
||||
}
|
||||
await queryClient.invalidateQueries(
|
||||
{ queryKey, exact: true, refetchType: "active" },
|
||||
|
||||
Reference in New Issue
Block a user