Conversation
📝 WalkthroughWalkthroughThe PR refactors wave management testing infrastructure, introduces reactive window-focus refetching across data hooks, consolidates foreground refresh logic in MyStreamContext using useEffectEvent, implements a cooldown mechanism for paginated queries, and makes pinned-wave operations conditional on authentication state. Changes
Sequence DiagramsequenceDiagram
actor User as Browser/App
participant Ctx as MyStreamContext
participant Handlers as Refresh Handlers
participant Queries as React Query Hooks
User->>Ctx: Window focus event / Visibility change
Ctx->>Handlers: refreshWaveViewOnForeground()
Handlers->>Handlers: Check dedupe cooldown
alt Cooldown active
Handlers->>Handlers: Skip refresh
else Cooldown expired
Handlers->>Ctx: Conditionally re-register active wave
Handlers->>Queries: refetch main waves
Handlers->>Queries: refetch DM waves
Handlers->>Queries: reset new drops counters
Queries-->>Handlers: Query state updated
end
User->>Ctx: Websocket reconnect
Ctx->>Handlers: refreshWaveViewOnForeground(dedupe, resetNewDrops)
Handlers->>Queries: Execute refresh with params
Queries-->>User: Updated wave data
User->>Ctx: App foreground (Capacitor)
Ctx->>Handlers: refreshWaveViewOnForeground(resetNewDrops: true)
Handlers->>Queries: refetch + reset counters
Queries-->>User: Updated UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa513f49b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const isWaveMuted = useCallback((waveId: string): boolean => { | ||
| const wave = wavesRef.current.find((w) => w.id === waveId); | ||
| const isWaveMuted = useEffectEvent((waveId: string): boolean => { |
There was a problem hiding this comment.
useEffectEvent returns a new callable each render, so replacing isWaveMuted with it makes this prop unstable. In useWaveRealtimeUpdater, processIncomingDrop depends on isWaveMuted, and that callback is then passed to useWebSocketMessage, whose effect re-subscribes on callback identity changes. When MyStreamProvider re-renders frequently, DROP websocket subscriptions are repeatedly torn down/re-added, creating small windows where incoming realtime messages can be missed.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
__tests__/contexts/wave/MyStreamContext.test.tsx (3)
428-434: Rerender with new QueryClient doesn't preserve component state.Similar to the earlier test, creating a new
QueryClientin the rerender doesn't simulate a Capacitor foreground transition accurately—it remounts everything. The reset mocks are called because of mount effects, not because of the state transition being detected.For accurate behavior testing, preserve the QueryClient:
♻️ Suggested fix
- const { rerender } = renderWithProvider(<div>child</div>); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + const { rerender } = render( + <QueryClientProvider client={queryClient}> + <MyStreamProvider> + <div>child</div> + </MyStreamProvider> + </QueryClientProvider> + ); mockRegisterWave.mockClear(); // ... other mock clears ... mockCapacitorState = { isCapacitor: true, isActive: true }; rerender( - <QueryClientProvider client={new QueryClient()}> + <QueryClientProvider client={queryClient}> <MyStreamProvider> <div>child</div> </MyStreamProvider> </QueryClientProvider> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/contexts/wave/MyStreamContext.test.tsx` around lines 428 - 434, The test is remounting components by creating a new QueryClient in the rerender, so mount effects run and mocks reset; instead preserve the original QueryClient instance across rerenders to simulate a foreground transition. Modify the test to reuse the QueryClient created before the initial render (keep the QueryClient variable outside and pass the same instance into QueryClientProvider during rerender) so that MyStreamProvider and its state are not remounted and the transition behavior is tested accurately.
153-161: Configure QueryClient for test stability.Creating
QueryClientwithout disabling retries can cause flaky tests if queries fail intermittently. Consider configuring defaults optimized for testing:♻️ Suggested improvement
const renderWithProvider = (children: React.ReactNode = null) => { - const queryClient = new QueryClient(); + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); return render( <QueryClientProvider client={queryClient}> <MyStreamProvider>{children}</MyStreamProvider> </QueryClientProvider> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/contexts/wave/MyStreamContext.test.tsx` around lines 153 - 161, renderWithProvider constructs a QueryClient without test-friendly options which can make tests flaky; update the creation of QueryClient inside renderWithProvider to pass defaultOptions that disable retries for queries/mutations (e.g., retry: false) and consider setting cacheTime/staleTime or using a silent logger if needed for tests, so change the QueryClient instantiation referenced in renderWithProvider to include these testing defaults before wrapping children with QueryClientProvider and MyStreamProvider.
222-228: Rerender with new QueryClient remounts the entire tree.Creating a new
QueryClientduringrerenderbreaks the continuity of the component tree—it effectively unmounts and remounts everything rather than simulating a prop change. WhilemockUnsubscribewill still be called (due to unmount cleanup), this doesn't accurately test the behavior of changingwaveIdwhile the context persists.Consider preserving the original
QueryClientacross rerenders:♻️ Suggested fix
- const { rerender } = renderWithProvider(<Messages waveId="wave" />); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + const { rerender } = render( + <QueryClientProvider client={queryClient}> + <MyStreamProvider> + <Messages waveId="wave" /> + </MyStreamProvider> + </QueryClientProvider> + ); expect(screen.getByText("a")).toBeInTheDocument(); act(() => listeners.wave({ id: "wave", drops: ["b"] })); expect(screen.getByText("b")).toBeInTheDocument(); rerender( - <QueryClientProvider client={new QueryClient()}> + <QueryClientProvider client={queryClient}> <MyStreamProvider> <Messages waveId="other" /> </MyStreamProvider> </QueryClientProvider> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/contexts/wave/MyStreamContext.test.tsx` around lines 222 - 228, The test is remounting the tree by creating a new QueryClient inside rerender, so change the test to reuse the original QueryClient instance instead of calling new QueryClient() during rerender; create a single QueryClient before the initial render (used by QueryClientProvider) and pass that same instance into the rerender call so QueryClientProvider, MyStreamProvider and Messages (and their mockUnsubscribe behavior) remain mounted and only the waveId prop changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useWaveBoostedDrops.ts`:
- Around line 42-44: In the useWaveBoostedDrops hook update the react-query
config so window-focus refetching matches the other hooks: change
refetchOnWindowFocus from true to "always" in the query options (alongside
STALE_TIME and REFETCH_INTERVAL) so the hook will immediately refetch on tab
focus rather than only when the cache is stale.
In `@hooks/useWavesOverview.ts`:
- Around line 131-145: The runWithCooldown function can allow a pending timeout
to fire after you immediately call action() when remainingCooldown === 0; fix
this by clearing any pending timeout before the immediate branch: call
clearScheduledAction(timeoutRef) (and ensure timeoutRef.current is set to null
by that helper) right before invoking action() in the remainingCooldown === 0
case so the old scheduled timer cannot later trigger a duplicate
refetch/fetchNextPage.
- Around line 156-170: The current fetchNextPage and refetch callbacks wrap
calls with "void query.fetchNextPage().catch(...)" and "void
query.refetch().catch(...)" which discards the Promise and breaks callers that
await these exported hook methods; change both callbacks (fetchNextPage and
refetch) to return the Promise instead by removing the void and returning the
result of runWithCooldown(...) so the inner arrow returns
query.fetchNextPage().catch(() => { /* keep existing comment */ }) and
query.refetch().catch(() => { /* keep existing comment */ }), ensuring the
useCallback wrappers (fetchNextPage and refetch) now return Promise<void> and
remain compatible with callers awaiting them; keep references to
fetchNextPageTimeoutRef, refetchTimeoutRef, runWithCooldown, and query
unchanged.
---
Nitpick comments:
In `@__tests__/contexts/wave/MyStreamContext.test.tsx`:
- Around line 428-434: The test is remounting components by creating a new
QueryClient in the rerender, so mount effects run and mocks reset; instead
preserve the original QueryClient instance across rerenders to simulate a
foreground transition. Modify the test to reuse the QueryClient created before
the initial render (keep the QueryClient variable outside and pass the same
instance into QueryClientProvider during rerender) so that MyStreamProvider and
its state are not remounted and the transition behavior is tested accurately.
- Around line 153-161: renderWithProvider constructs a QueryClient without
test-friendly options which can make tests flaky; update the creation of
QueryClient inside renderWithProvider to pass defaultOptions that disable
retries for queries/mutations (e.g., retry: false) and consider setting
cacheTime/staleTime or using a silent logger if needed for tests, so change the
QueryClient instantiation referenced in renderWithProvider to include these
testing defaults before wrapping children with QueryClientProvider and
MyStreamProvider.
- Around line 222-228: The test is remounting the tree by creating a new
QueryClient inside rerender, so change the test to reuse the original
QueryClient instance instead of calling new QueryClient() during rerender;
create a single QueryClient before the initial render (used by
QueryClientProvider) and pass that same instance into the rerender call so
QueryClientProvider, MyStreamProvider and Messages (and their mockUnsubscribe
behavior) remain mounted and only the waveId prop changes.
🪄 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
Run ID: 709a4056-c12e-4018-903a-191bfbe3acd4
📒 Files selected for processing (10)
__tests__/contexts/wave/MyStreamContext.test.tsx__tests__/hooks/usePinnedWavesServer.test.tsx__tests__/hooks/useWaveData.test.tsx__tests__/hooks/useWavesOverview.test.tsxcomponents/user/layout/userPageVisibility.tscontexts/wave/MyStreamContext.tsxhooks/usePinnedWavesServer.tshooks/useWaveBoostedDrops.tshooks/useWaveData.tshooks/useWavesOverview.ts
| staleTime: STALE_TIME, | ||
| refetchInterval: REFETCH_INTERVAL, | ||
| refetchOnWindowFocus: true, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the useWaveBoostedDrops.ts file
find . -name "useWaveBoostedDrops.ts" -type fRepository: 6529-Collections/6529seize-frontend
Length of output: 109
🏁 Script executed:
# List files in hooks directory to confirm structure
git ls-files | grep -E "hooks/.*\.ts" | head -20Repository: 6529-Collections/6529seize-frontend
Length of output: 1547
🏁 Script executed:
# Read the useWaveBoostedDrops.ts file
cat -n ./hooks/useWaveBoostedDrops.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 1940
🏁 Script executed:
# Search for STALE_TIME and REFETCH_INTERVAL constants
rg "STALE_TIME|REFETCH_INTERVAL" --no-heading -nRepository: 6529-Collections/6529seize-frontend
Length of output: 3547
🏁 Script executed:
# Search for other uses of refetchOnWindowFocus in the codebase to check for "always" pattern
rg "refetchOnWindowFocus" -A 1 -B 1Repository: 6529-Collections/6529seize-frontend
Length of output: 5287
🏁 Script executed:
# Check if this file was modified in recent commits to understand PR context
git log --oneline --all -- "hooks/useWaveBoostedDrops.ts" | head -5Repository: 6529-Collections/6529seize-frontend
Length of output: 157
🏁 Script executed:
# Check if usePinnedWavesServer was recently added/modified to understand if "always" is new in this PR
git log --oneline --all -- "hooks/usePinnedWavesServer.ts" | head -5Repository: 6529-Collections/6529seize-frontend
Length of output: 279
🏁 Script executed:
# Check useWaveData for context on when "always" was added
git log --oneline --all -- "hooks/useWaveData.ts" | head -5Repository: 6529-Collections/6529seize-frontend
Length of output: 267
🏁 Script executed:
# Check what changed in the wip commit for usePinnedWavesServer
git show aa513f49:hooks/usePinnedWavesServer.ts 2>/dev/null | grep -A 2 "refetchOnWindowFocus" || echo "File not in wip commit or error"Repository: 6529-Collections/6529seize-frontend
Length of output: 121
🏁 Script executed:
# Check what changed in the wip commit for useWaveData
git show aa513f49:hooks/useWaveData.ts 2>/dev/null | grep -A 2 "refetchOnWindowFocus" || echo "File not in wip commit or error"Repository: 6529-Collections/6529seize-frontend
Length of output: 228
🏁 Script executed:
# Look at the actual diff for the current changes
git diff HEAD -- hooks/useWaveBoostedDrops.ts hooks/useWaveData.ts hooks/usePinnedWavesServer.ts 2>/dev/null || git diff aa513f49^..aa513f49 -- hooks/useWaveBoostedDrops.ts hooks/useWaveData.ts hooks/usePinnedWavesServer.ts 2>/dev/null || echo "Check git status"Repository: 6529-Collections/6529seize-frontend
Length of output: 61
🏁 Script executed:
# Check git status and recent changes
git status --shortRepository: 6529-Collections/6529seize-frontend
Length of output: 92
🏁 Script executed:
# Check the state of useWaveBoostedDrops in the wip commit
git show aa513f49:hooks/useWaveBoostedDrops.ts | grep -A 3 "refetchOnWindowFocus"Repository: 6529-Collections/6529seize-frontend
Length of output: 185
🏁 Script executed:
# Check if there are any test files or comments that explain the intended behavior
rg "refetchOnWindowFocus.*always" -B 5 -A 2Repository: 6529-Collections/6529seize-frontend
Length of output: 2065
🏁 Script executed:
# Look at the PR or commit message for context on this change
git log -1 aa513f49 --format=fullerRepository: 6529-Collections/6529seize-frontend
Length of output: 327
refetchOnWindowFocus: true only refetches stale queries; consider using "always" for consistency with other hooks in this PR.
Line 44 only refetches when data is stale. With staleTime: 60000, a tab restored within that minute will keep showing cached boosted drops until the next 30s polling interval fires. Other hooks in this PR (useWaveData, usePinnedWavesServer) use refetchOnWindowFocus: "always" for immediate foreground freshness—this should match that pattern.
Suggested change
- refetchOnWindowFocus: true,
+ refetchOnWindowFocus: "always",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| staleTime: STALE_TIME, | |
| refetchInterval: REFETCH_INTERVAL, | |
| refetchOnWindowFocus: true, | |
| staleTime: STALE_TIME, | |
| refetchInterval: REFETCH_INTERVAL, | |
| refetchOnWindowFocus: "always", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useWaveBoostedDrops.ts` around lines 42 - 44, In the
useWaveBoostedDrops hook update the react-query config so window-focus
refetching matches the other hooks: change refetchOnWindowFocus from true to
"always" in the query options (alongside STALE_TIME and REFETCH_INTERVAL) so the
hook will immediately refetch on tab focus rather than only when the cache is
stale.
| const runWithCooldown = useCallback( | ||
| (timeoutRef: TimeoutRef, action: () => void) => { | ||
| const remainingCooldown = getRemainingCooldown(); | ||
| if (remainingCooldown === 0) { | ||
| action(); | ||
| return; | ||
| } | ||
|
|
||
| clearScheduledAction(timeoutRef); | ||
|
|
||
| timeoutRef.current = setTimeout(() => { | ||
| timeoutRef.current = null; | ||
| action(); | ||
| }, remainingCooldown); | ||
| }, |
There was a problem hiding this comment.
Clear any pending timeout before the immediate branch.
If a cooldown timer was scheduled while the tab was backgrounded, browser timer throttling can delay it past the cooldown boundary. A later call then runs action() immediately, but the old timeout is still live and can fire afterward, producing a duplicate refetch() / fetchNextPage().
Suggested change
const runWithCooldown = useCallback(
(timeoutRef: TimeoutRef, action: () => void) => {
+ clearScheduledAction(timeoutRef);
const remainingCooldown = getRemainingCooldown();
if (remainingCooldown === 0) {
action();
return;
}
-
- clearScheduledAction(timeoutRef);
timeoutRef.current = setTimeout(() => {
timeoutRef.current = null;
action();
}, remainingCooldown);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const runWithCooldown = useCallback( | |
| (timeoutRef: TimeoutRef, action: () => void) => { | |
| const remainingCooldown = getRemainingCooldown(); | |
| if (remainingCooldown === 0) { | |
| action(); | |
| return; | |
| } | |
| clearScheduledAction(timeoutRef); | |
| timeoutRef.current = setTimeout(() => { | |
| timeoutRef.current = null; | |
| action(); | |
| }, remainingCooldown); | |
| }, | |
| const runWithCooldown = useCallback( | |
| (timeoutRef: TimeoutRef, action: () => void) => { | |
| clearScheduledAction(timeoutRef); | |
| const remainingCooldown = getRemainingCooldown(); | |
| if (remainingCooldown === 0) { | |
| action(); | |
| return; | |
| } | |
| timeoutRef.current = setTimeout(() => { | |
| timeoutRef.current = null; | |
| action(); | |
| }, remainingCooldown); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useWavesOverview.ts` around lines 131 - 145, The runWithCooldown
function can allow a pending timeout to fire after you immediately call action()
when remainingCooldown === 0; fix this by clearing any pending timeout before
the immediate branch: call clearScheduledAction(timeoutRef) (and ensure
timeoutRef.current is set to null by that helper) right before invoking action()
in the remainingCooldown === 0 case so the old scheduled timer cannot later
trigger a duplicate refetch/fetchNextPage.
| const fetchNextPage = useCallback(() => { | ||
| runWithCooldown(fetchNextPageTimeoutRef, () => { | ||
| void query.fetchNextPage().catch(() => { | ||
| // Error surfaced via query state | ||
| }); | ||
| }); | ||
| }, [query, runWithCooldown]); | ||
|
|
||
| const refetch = useCallback(() => { | ||
| runWithCooldown(refetchTimeoutRef, () => { | ||
| void query.refetch().catch(() => { | ||
| // Error surfaced via query state | ||
| }); | ||
| }); | ||
| }, [query, runWithCooldown]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "useWavesOverview.ts"Repository: 6529-Collections/6529seize-frontend
Length of output: 104
🏁 Script executed:
wc -l hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 108
🏁 Script executed:
sed -n '150,185p' hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 1024
🏁 Script executed:
head -n 20 hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 987
🏁 Script executed:
sed -n '1,50p' hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 1789
🏁 Script executed:
sed -n '170,185p' hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 444
🏁 Script executed:
rg -A 20 "export const useWavesOverview" hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 674
🏁 Script executed:
sed -n '30,100p' hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 2365
🏁 Script executed:
rg "useWavesOverview" --type ts --type tsx -B 2 -A 5 | head -100Repository: 6529-Collections/6529seize-frontend
Length of output: 106
🏁 Script executed:
rg "useWavesOverview" -A 5 -B 2 | head -150Repository: 6529-Collections/6529seize-frontend
Length of output: 7151
🏁 Script executed:
rg "await.*fetchNextPage\|await.*refetch" --max-count 20Repository: 6529-Collections/6529seize-frontend
Length of output: 61
🏁 Script executed:
rg "fetchNextPage\|refetch" hooks/ -B 3 -A 3 | grep -v "^--$" | head -200Repository: 6529-Collections/6529seize-frontend
Length of output: 61
🏁 Script executed:
rg "fetchNextPage" -A 2 -B 2 | head -100Repository: 6529-Collections/6529seize-frontend
Length of output: 4286
🏁 Script executed:
cat types/waves.types.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 4912
🏁 Script executed:
rg "WaveOutcomeDistributionState\|useWavesOverview.*return\|interface.*Overview" types/waves.types.ts -A 5Repository: 6529-Collections/6529seize-frontend
Length of output: 61
🏁 Script executed:
sed -n '170,185p' hooks/useWavesOverview.tsRepository: 6529-Collections/6529seize-frontend
Length of output: 444
Make fetchNextPage() and refetch() awaitable by removing the void wrapping.
The methods are wrapped with void query.fetchNextPage().catch(...) and void query.refetch().catch(...), which discards the promise and converts the return type from Promise<void> to void. This silently breaks the API for callers who await these methods—they will continue immediately instead of waiting for the operation to complete. Code in the codebase (e.g., useWaveDropsLeaderboard.ts) already attempts to await fetchNextPage(), making this a breaking change for an exported hook.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useWavesOverview.ts` around lines 156 - 170, The current fetchNextPage
and refetch callbacks wrap calls with "void query.fetchNextPage().catch(...)"
and "void query.refetch().catch(...)" which discards the Promise and breaks
callers that await these exported hook methods; change both callbacks
(fetchNextPage and refetch) to return the Promise instead by removing the void
and returning the result of runWithCooldown(...) so the inner arrow returns
query.fetchNextPage().catch(() => { /* keep existing comment */ }) and
query.refetch().catch(() => { /* keep existing comment */ }), ensuring the
useCallback wrappers (fetchNextPage and refetch) now return Promise<void> and
remain compatible with callers awaiting them; keep references to
fetchNextPageTimeoutRef, refetchTimeoutRef, runWithCooldown, and query
unchanged.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/contexts/wave/MyStreamContext.test.tsx`:
- Around line 442-473: The test mounts a fresh QueryClient on rerender so
prevIsActiveRef resets and the foreground-transition branch never runs; fix by
creating a single QueryClient instance before the first render and reuse that
same instance for both the initial render and the rerender (i.e., const qc = new
QueryClient(); use <QueryClientProvider client={qc}> around MyStreamProvider on
initial render and again in rerender) so prevIsActiveRef in MyStreamProvider
correctly sees the previous isActive value and the effect runs.
🪄 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
Run ID: 69806bcd-e952-4124-bd96-b68b4f7c1e1f
📒 Files selected for processing (5)
__tests__/contexts/wave/MyStreamContext.test.tsx__tests__/hooks/usePinnedWavesServer.test.tsx__tests__/hooks/useWavesList.test.tsxcontexts/wave/MyStreamContext.tsxhooks/useWavesList.ts
✅ Files skipped from review due to trivial changes (1)
- tests/hooks/usePinnedWavesServer.test.tsx
| it("resets all new-drop counters when the native app returns to foreground", () => { | ||
| mockActiveWaveId = "wave-1"; | ||
| mockCapacitorState = { isCapacitor: true, isActive: false }; | ||
|
|
||
| const { rerender } = renderWithProvider(<div>child</div>); | ||
|
|
||
| mockRegisterWave.mockClear(); | ||
| mockMainWavesRefetch.mockClear(); | ||
| mockDmMainWavesRefetch.mockClear(); | ||
| mockRefetchAllWaves.mockClear(); | ||
| mockRefetchAllDmWaves.mockClear(); | ||
| mockResetMainWavesNewDropsCount.mockClear(); | ||
| mockResetDmWavesNewDropsCount.mockClear(); | ||
|
|
||
| mockCapacitorState = { isCapacitor: true, isActive: true }; | ||
|
|
||
| rerender( | ||
| <QueryClientProvider client={new QueryClient()}> | ||
| <MyStreamProvider> | ||
| <div>child</div> | ||
| </MyStreamProvider> | ||
| </QueryClientProvider> | ||
| ); | ||
|
|
||
| expect(unsubscribe).toHaveBeenCalled(); | ||
| expect(mockRegisterWave).toHaveBeenCalledWith("wave-1", true); | ||
| expect(mockRefetchAllWaves).toHaveBeenCalledTimes(1); | ||
| expect(mockRefetchAllDmWaves).toHaveBeenCalledTimes(1); | ||
| expect(mockMainWavesRefetch).not.toHaveBeenCalled(); | ||
| expect(mockDmMainWavesRefetch).not.toHaveBeenCalled(); | ||
| expect(mockResetMainWavesNewDropsCount).toHaveBeenCalledTimes(1); | ||
| expect(mockResetDmWavesNewDropsCount).toHaveBeenCalledTimes(1); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if the test passes with the current implementation by checking if similar patterns exist elsewhere
# and examining how other tests handle the rerender scenario
# Check how other tests in this file handle rerenders with QueryClient
rg -n "rerender.*QueryClient" __tests__/contexts/wave/MyStreamContext.test.tsx -A2 -B2Repository: 6529-Collections/6529seize-frontend
Length of output: 61
🏁 Script executed:
# First, let's check the test file around the specified lines
cat -n __tests__/contexts/wave/MyStreamContext.test.tsx | sed -n '440,475p'Repository: 6529-Collections/6529seize-frontend
Length of output: 1567
🏁 Script executed:
# Check the MyStreamProvider implementation
find . -name "MyStreamContext.tsx" -o -name "MyStreamProvider.tsx" -o -name "*MyStream*" -type f | grep -v node_modules | grep -v __tests__Repository: 6529-Collections/6529seize-frontend
Length of output: 1214
🏁 Script executed:
# Look for the MyStreamProvider implementation and its effect/ref usage
rg -n "prevIsActiveRef" . --type tsx --type tsRepository: 6529-Collections/6529seize-frontend
Length of output: 106
🏁 Script executed:
# Search for prevIsActiveRef without file type restrictions
rg -n "prevIsActiveRef" .Repository: 6529-Collections/6529seize-frontend
Length of output: 330
🏁 Script executed:
# Examine the MyStreamContext.tsx file to understand the provider implementation
cat -n ./contexts/wave/MyStreamContext.tsx | head -100Repository: 6529-Collections/6529seize-frontend
Length of output: 4380
🏁 Script executed:
# Check how renderWithProvider is implemented
rg -n "renderWithProvider" __tests__/contexts/wave/MyStreamContext.test.tsx -B5 | head -30Repository: 6529-Collections/6529seize-frontend
Length of output: 981
🏁 Script executed:
# Get full renderWithProvider implementation
cat -n __tests__/contexts/wave/MyStreamContext.test.tsx | sed -n '153,180p'Repository: 6529-Collections/6529seize-frontend
Length of output: 1136
🏁 Script executed:
# Check the full effect logic in MyStreamContext around the condition
cat -n ./contexts/wave/MyStreamContext.tsx | sed -n '120,260p'Repository: 6529-Collections/6529seize-frontend
Length of output: 5190
🏁 Script executed:
# Look at how other tests handle state transitions and rerenders
cat -n __tests__/contexts/wave/MyStreamContext.test.tsx | sed -n '200,240p'Repository: 6529-Collections/6529seize-frontend
Length of output: 1655
🏁 Script executed:
# Check if there are other tests that test Capacitor state transitions
rg -n "mockCapacitorState.*isActive" __tests__/contexts/wave/MyStreamContext.test.tsx -B3 -A10Repository: 6529-Collections/6529seize-frontend
Length of output: 2101
Fix the Capacitor foreground transition test to use the same QueryClient instance across rerenders.
The test creates a new QueryClient on rerender (line 459), which mounts a fresh MyStreamProvider instance. This causes prevIsActiveRef to initialize with the current isActive value (true), making the condition !prevIsActiveRef.current && isActive evaluate to false and skipping the effect body.
Keep the same QueryClient instance:
Diff
+ const queryClient = new QueryClient();
const { rerender } = renderWithProvider(<div>child</div>);
mockRegisterWave.mockClear();
...
rerender(
- <QueryClientProvider client={new QueryClient()}>
+ <QueryClientProvider client={queryClient}>
<MyStreamProvider>
<div>child</div>
</MyStreamProvider>
</QueryClientProvider>
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@__tests__/contexts/wave/MyStreamContext.test.tsx` around lines 442 - 473, The
test mounts a fresh QueryClient on rerender so prevIsActiveRef resets and the
foreground-transition branch never runs; fix by creating a single QueryClient
instance before the first render and reuse that same instance for both the
initial render and the rerender (i.e., const qc = new QueryClient(); use
<QueryClientProvider client={qc}> around MyStreamProvider on initial render and
again in rerender) so prevIsActiveRef in MyStreamProvider correctly sees the
previous isActive value and the effect runs.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests