feat: move async store out of react component#14
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
✅ Deploy Preview for hyperdb ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR adds a new async selector store module ( ChangesAsync selector store core
React useAsyncSelector rewrite
Documentation updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Component
participant useAsyncSelector
participant CachedSelectorStoreAsync
participant SubscribableDB
Component->>useAsyncSelector: render
useAsyncSelector->>CachedSelectorStoreAsync: createCachedSelectorStoreAsync(db, input)
useAsyncSelector->>CachedSelectorStoreAsync: useSyncExternalStore(subscribe, getSnapshot)
CachedSelectorStoreAsync->>SubscribableDB: subscribe to relevant ranges
CachedSelectorStoreAsync-->>useAsyncSelector: initial snapshot (pending/success)
SubscribableDB-->>CachedSelectorStoreAsync: notify overlapping DB change
CachedSelectorStoreAsync->>CachedSelectorStoreAsync: rerun selector, update state
CachedSelectorStoreAsync-->>useAsyncSelector: notify snapshot update
useAsyncSelector-->>Component: re-render with result
Component->>useAsyncSelector: unmount
useAsyncSelector->>CachedSelectorStoreAsync: destroy()
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/hyperdb/src/hyperdb/commands/selector/async-selector-store.test.ts (1)
42-413: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSuite reads well and matches current behavior; false-positive static analysis hints noted but ignored.
The
execSyncflagged by OpenGrep at multiple lines is the DB test helper imported from../../db, not Node'schild_process.execSync— not an actual injection risk.Consider adding regression coverage for the two lifecycle/concurrency issues raised in
async-selector-store.ts: (1) unsubscribe-then-resubscribe with no intervening DB write while a run is still in-flight (currently no test exercisesstartedreset), and (2) callingrefetch()with different options while another run is already in-flight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hyperdb/src/hyperdb/commands/selector/async-selector-store.test.ts` around lines 42 - 413, Add regression coverage in createCachedSelectorStoreAsync tests for the two missing async-store edge cases: a subscriber unsubscribing and then resubscribing while a selector run is still in-flight, and a refetch() call with different options while another run is already pending. Use the existing createCachedSelectorStoreAsync, store.subscribe/unsubscribe, and store.refetch paths to verify the started state resets correctly and that overlapping runs/options behave as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/hyperdb/src/hyperdb/commands/selector/async-selector-store.ts`:
- Around line 508-559: The selector store can get stuck after a full
unsubscribe/resubscribe because the cleanup in subscribe never resets started,
so ensureStarted() and the runRevision check can both skip a needed refresh.
Update the last-listener teardown in subscribe to reset started alongside the
existing run state, and also clear any pending stale timer via clearStaleTimer()
so no background work survives after the store is fully unsubscribed. Keep the
fix centered around run(), ensureStarted(), ensureDBSubscription(), and the
subscribe cleanup path.
- Around line 364-377: The shared in-flight path in AsyncSelectorStore’s
run/refetch logic is reusing the original promise, so a later caller’s
throwOnError option can be ignored. Update the isRunning branch in
async-selector-store.ts so piggybacking refetch() calls still respect the
current caller’s options, especially when options.throwOnError is true, rather
than always inheriting the first request’s behavior. Use the run function and
inFlightResult handling to locate the change and ensure concurrent callers can
get a rejecting promise when requested.
In `@packages/hyperdb/src/react/hooks.ts`:
- Around line 258-295: The store in hooks.ts is being recreated whenever
reference-unstable option values change, because the `storeInput` memo in the
selector hook depends directly on raw `defaultValue`, `initialData`, and
`placeholderData` references. Update the memoization around `storeInput` and the
`createCachedSelectorStoreAsync` call so these options are derived through a
stable key or otherwise normalized, and reference the existing `useMemo` blocks
and `storeInput`/`store` symbols to keep the store stable across unrelated
rerenders.
---
Nitpick comments:
In `@packages/hyperdb/src/hyperdb/commands/selector/async-selector-store.test.ts`:
- Around line 42-413: Add regression coverage in createCachedSelectorStoreAsync
tests for the two missing async-store edge cases: a subscriber unsubscribing and
then resubscribing while a selector run is still in-flight, and a refetch() call
with different options while another run is already pending. Use the existing
createCachedSelectorStoreAsync, store.subscribe/unsubscribe, and store.refetch
paths to verify the started state resets correctly and that overlapping
runs/options behave as expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b8ad2ef-b427-49a0-9dd6-8875326d3257
📒 Files selected for processing (11)
README.mdpackages/hyperdb-doc/src/content/docs/database/reading-data.mdpackages/hyperdb-doc/src/content/docs/database/selectors-reactivity.mdpackages/hyperdb-doc/src/content/docs/integrations/react.mdpackages/hyperdb-doc/src/content/docs/start/llm-cheat-sheet.mdpackages/hyperdb-doc/summary.mdpackages/hyperdb/src/hyperdb/commands/selector/async-selector-store.test.tspackages/hyperdb/src/hyperdb/commands/selector/async-selector-store.tspackages/hyperdb/src/hyperdb/index.tspackages/hyperdb/src/react/hooks.test.tspackages/hyperdb/src/react/hooks.ts
Summary by CodeRabbit
New Features
useAsyncSelectornow uses the shared async selector store behavior and supports improved refetch and cleanup handling.Documentation