Skip to content

Commit c659dc9

Browse files
Fix useTable isReady reverting to false after first row event (#4580)
Fixes #4559. ## Bug The `subscribe` callback passed to `useSyncExternalStore` captures `computeSnapshot` in its closure but did not list it as a dependency. When `subscribeApplied` flips to `true`: 1. `computeSnapshot` is recreated with `subscribeApplied = true` 2. But `subscribe` still holds the **stale** closure that captured `subscribeApplied = false` 3. On the next row event, the stale `computeSnapshot` writes `[rows, false]` into `lastSnapshotRef` 4. `isReady` drops to `false` permanently ## Fix Add `computeSnapshot` to the `subscribe` dependency array so event handlers always use the current snapshot function. Note: PR #4499 previously fixed a related issue where the cached snapshot was stale after `subscribeApplied` changed. This fix addresses the remaining case where the `subscribe` closure itself was stale. --------- Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com> Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
1 parent 61be6e6 commit c659dc9

1 file changed

Lines changed: 5 additions & 0 deletions

File tree

crates/bindings-typescript/src/react/useTable.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ export function useTable<TableDef extends UntypedTableDef>(
104104
) as Prettify<UseTableRowType>[])
105105
: (Array.from(table.iter()) as Prettify<UseTableRowType>[]);
106106
return [result, subscribeApplied];
107+
// TODO: investigating refactoring so that this is no longer necessary, as we have had genuine bugs with missed deps.
108+
// See https://github.com/clockworklabs/SpacetimeDB/pull/4580.
107109
// eslint-disable-next-line react-hooks/exhaustive-deps
108110
}, [connectionState, accessorName, querySql, subscribeApplied]);
109111

@@ -205,11 +207,14 @@ export function useTable<TableDef extends UntypedTableDef>(
205207
table.removeOnUpdate?.(onUpdate);
206208
};
207209
},
210+
// TODO: investigating refactoring so that this is no longer necessary, as we have had genuine bugs with missed deps.
211+
// See https://github.com/clockworklabs/SpacetimeDB/pull/4580.
208212
// eslint-disable-next-line react-hooks/exhaustive-deps
209213
[
210214
connectionState,
211215
accessorName,
212216
querySql,
217+
computeSnapshot,
213218
callbacks?.onDelete,
214219
callbacks?.onInsert,
215220
callbacks?.onUpdate,

0 commit comments

Comments
 (0)