Skip to content

Commit 339b321

Browse files
committed
fix(realtime): make room callbacks return their invalidations
The workspace invalidation room awaits a callback's promise before it starts the next delivery, but the type allowed any return, and the tables and workflows rooms fired their invalidations without returning them. A broadcast arriving while a list refetch was still loading invalidated it again, which cancels and restarts the in-flight fetch, so under a burst of writes a slow list could keep restarting. The callback type is now `() => Promise<unknown>`, both rooms return `Promise.all` over the same invalidations they ran before, and the resource-folder picker returns a resolved promise when it has no workspace. Autopsy: the generic hook's tests drive it with hand-written callbacks that return a pending promise, so they proved the hook waits, but nothing exercised a real binding: the tables and workflows rooms were never mounted in a test, and `() => unknown` let a void-returning callback type-check. The new tables room test mounts the real binding against a real QueryClient and fails on the old binding with a second fetch started while the first was pending.
1 parent 31f7529 commit 339b321

6 files changed

Lines changed: 116 additions & 15 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/hooks/use-workspace-invalidation-room.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const roots: Root[] = []
2929
function mount(
3030
workspaceId: string,
3131
roomType: RoomType,
32-
onChanged: () => unknown = () => {},
32+
onChanged: () => Promise<unknown> = async () => {},
3333
dedupeKey?: string
3434
): Root {
3535
function Probe() {
@@ -119,7 +119,7 @@ describe('useWorkspaceInvalidationRoom', () => {
119119
})
120120

121121
it('delivers a burst of broadcasts once at once and once after it, not once per broadcast', async () => {
122-
const onChanged = vi.fn()
122+
const onChanged = vi.fn(async () => {})
123123
mount('ws-1', 'workspace-files', onChanged)
124124

125125
for (let index = 0; index < 20; index++) broadcast('workspace-files-changed', 'ws-1')

‎apps/sim/app/workspace/[workspaceId]/hooks/use-workspace-invalidation-room.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,11 @@ interface JoinErrorPayload {
3636
}
3737

3838
/**
39-
* Runs when the room's resources changed. A returned promise is awaited, up to
40-
* {@link DELIVERY_SETTLE_CAP_MS}, before the next delivery starts.
39+
* Runs when the room's resources changed and returns the invalidations it started. The promise is
40+
* awaited, up to {@link DELIVERY_SETTLE_CAP_MS}, before the next delivery starts, so a callback
41+
* that drops it lets the next broadcast cancel and restart refetches still in flight.
4142
*/
42-
type RoomChangedCallback = () => unknown
43+
type RoomChangedCallback = () => Promise<unknown>
4344

4445
interface SharedRoomSubscription {
4546
callbacks: Map<string | symbol, Set<RoomChangedCallback>>
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
5+
import { act } from 'react'
6+
import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
10+
const { mockUseSocket } = vi.hoisted(() => ({ mockUseSocket: vi.fn() }))
11+
12+
vi.mock('@/app/workspace/providers/socket-provider', () => ({
13+
useSocket: () => mockUseSocket(),
14+
}))
15+
16+
import { INVALIDATION_COALESCE_MS } from '@/app/workspace/[workspaceId]/hooks/use-workspace-invalidation-room'
17+
import { useWorkspaceTablesRoom } from '@/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room'
18+
import { tableKeys } from '@/hooks/queries/utils/table-keys'
19+
20+
function fakeSocket() {
21+
return { connected: true, emit: vi.fn(), on: vi.fn(), off: vi.fn() }
22+
}
23+
24+
let socket: ReturnType<typeof fakeSocket>
25+
let root: Root | null = null
26+
27+
function broadcast(workspaceId: string) {
28+
for (const [name, handler] of socket.on.mock.calls) {
29+
if (name === 'workspace-tables-changed') handler({ workspaceId })
30+
}
31+
}
32+
33+
describe('useWorkspaceTablesRoom', () => {
34+
beforeEach(() => {
35+
vi.useFakeTimers()
36+
socket = fakeSocket()
37+
mockUseSocket.mockImplementation(() => ({ socket }))
38+
})
39+
40+
afterEach(() => {
41+
if (root) act(() => root?.unmount())
42+
root = null
43+
vi.useRealTimers()
44+
})
45+
46+
/*
47+
* The callback used to fire both invalidations and return nothing, so the room's delivery never
48+
* waited: a broadcast arriving while the list refetch was still loading invalidated it again,
49+
* which cancels and restarts the in-flight fetch. Under a burst of table writes a slow list never
50+
* finished loading.
51+
*/
52+
it('does not restart a list refetch still in flight when the next broadcast arrives', async () => {
53+
const pending: Array<() => void> = []
54+
const fetchList = vi.fn(
55+
() =>
56+
new Promise<string[]>((resolve) => {
57+
pending.push(() => resolve([]))
58+
})
59+
)
60+
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
61+
queryClient.setQueryData([...tableKeys.lists(), 'ws-1'], [])
62+
63+
function Probe() {
64+
useQuery({
65+
queryKey: [...tableKeys.lists(), 'ws-1'],
66+
queryFn: fetchList,
67+
staleTime: Number.POSITIVE_INFINITY,
68+
})
69+
useWorkspaceTablesRoom('ws-1')
70+
return null
71+
}
72+
const container = document.createElement('div')
73+
document.body.appendChild(container)
74+
root = createRoot(container)
75+
act(() =>
76+
root?.render(
77+
<QueryClientProvider client={queryClient}>
78+
<Probe />
79+
</QueryClientProvider>
80+
)
81+
)
82+
83+
broadcast('ws-1')
84+
await vi.advanceTimersByTimeAsync(0)
85+
expect(fetchList).toHaveBeenCalledTimes(1)
86+
87+
broadcast('ws-1')
88+
await vi.advanceTimersByTimeAsync(5 * INVALIDATION_COALESCE_MS)
89+
expect(fetchList).toHaveBeenCalledTimes(1)
90+
91+
for (const resolve of pending.splice(0)) resolve()
92+
await vi.advanceTimersByTimeAsync(INVALIDATION_COALESCE_MS)
93+
expect(fetchList).toHaveBeenCalledTimes(2)
94+
})
95+
})

‎apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import { tableKeys } from '@/hooks/queries/utils/table-keys'
1616
*/
1717
export function useWorkspaceTablesRoom(workspaceId: string): void {
1818
const queryClient = useQueryClient()
19-
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_TABLES, () => {
20-
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
21-
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') })
22-
})
19+
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_TABLES, () =>
20+
Promise.all([
21+
queryClient.invalidateQueries({ queryKey: tableKeys.lists() }),
22+
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') }),
23+
])
24+
)
2325
}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-resource-folders.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ export function useResourceFolders(
7777
useWorkspaceInvalidationRoom(
7878
isFileResource ? (workspaceId ?? '') : '',
7979
ROOM_TYPES.WORKSPACE_FILES,
80-
() => (workspaceId ? invalidateWorkspaceFileBrowsers(queryClient, workspaceId) : undefined),
80+
() =>
81+
workspaceId ? invalidateWorkspaceFileBrowsers(queryClient, workspaceId) : Promise.resolve(),
8182
WORKSPACE_FILE_BROWSER_INVALIDATION_KEY
8283
)
8384

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workspace-workflows-room.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ import { invalidateWorkflowLists } from '@/hooks/queries/utils/invalidate-workfl
2121
*/
2222
export function useWorkspaceWorkflowsRoom(workspaceId: string): void {
2323
const queryClient = useQueryClient()
24-
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_WORKFLOWS, () => {
25-
invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived'])
26-
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'active') })
27-
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'archived') })
28-
})
24+
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_WORKFLOWS, () =>
25+
Promise.all([
26+
invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived']),
27+
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'active') }),
28+
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'archived') }),
29+
])
30+
)
2931
}

0 commit comments

Comments
 (0)