Scroll jump 160426#2291
Conversation
📝 WalkthroughWalkthroughThe PR introduces a light-drop hydration suspension mechanism that prevents fetching LIGHT-type drops when the component is actively scrolling or tracking an active serial target. A new Changes
Sequence Diagram(s)sequenceDiagram
participant WaveDropsAll as WaveDropsAll
participant VirtualScrollWrapper as VirtualScrollWrapper
participant IntersectionObserver as IntersectionObserver
participant FetchAPI as Fetch Service
WaveDropsAll->>WaveDropsAll: Determine suspendLightDropHydration<br/>(isScrolling || serialTarget !== null)
WaveDropsAll->>VirtualScrollWrapper: Pass suspendLightDropHydration prop
VirtualScrollWrapper->>IntersectionObserver: Register element with observer
Note over IntersectionObserver: Element enters viewport
IntersectionObserver->>IntersectionObserver: Check: isInView && !suspendLightDropHydration
alt Not Suspended (isInView && !suspendLightDropHydration)
IntersectionObserver->>FetchAPI: Call fetchAroundSerialNo(waveId, dropSerialNo)
FetchAPI-->>IntersectionObserver: Fetch response
else Suspended (isInView && suspendLightDropHydration)
IntersectionObserver->>IntersectionObserver: Skip fetch - hydration suspended
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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: 53072cc7f9
ℹ️ 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".
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/waves/drops/VirtualScrollWrapper.tsx (1)
111-158:⚠️ Potential issue | 🟡 Minor
fetchAroundSerialNohas stable identity; queuing prevents duplicate requests for the same serial.Two observations on the expanded effect:
Effect churn is not a concern:
fetchAroundSerialNois wrapped inuseCallbackwith dependencies[cancelAbort, _processAroundSerialNoQueue], both of which are themselvesuseCallback-wrapped with stable or empty dependencies. The transitive chain is solid—the effect won't unnecessarily rebuild due to function identity changes.Duplicate fetches are mostly mitigated: The
useWavePaginationqueuing system (in_processAroundSerialNoQueue) prevents re-fetching the same(waveId, serialNo)pair:
- Only one fetch per
waveIdruns concurrently (guarded byisFetching).- Subsequent calls for the same serial are coalesced to the latest pending request.
- The
determineSerialToFetchfunction avoids redundant fetches by checking the last successfully fetched range.However, when
suspendLightDropHydrationtransitions true → false, the observer recreates and fires callbacks for all in-view LIGHT drops. If multiple drops have different serial numbers, each triggers its own fetch. This is likely intentional (independent content), but confirm that the concurrent request rate is acceptable downstream. No test coverage exists for this toggle scenario.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drops/VirtualScrollWrapper.tsx` around lines 111 - 158, The effect can fire many LIGHT-drop fetches when suspendLightDropHydration flips from true→false; add a small debounce/batch guard in VirtualScrollWrapper's useEffect so inView && type === DropSize.LIGHT calls to fetchAroundSerialNo are coalesced briefly (e.g., microtask/setTimeout 50–100ms) before invoking fetchAroundSerialNo, or gate calls behind a per-wave "hydrationResumed" flag to avoid simultaneous spikes; also add a test covering the suspendLightDropHydration toggle to assert that useWavePagination's queue/_processAroundSerialNoQueue handles the burst without duplicate work and that fetchAroundSerialNo is only called once per unique (waveId, dropSerialNo) within the debounce window.
🧹 Nitpick comments (1)
components/drops/view/DropsList.tsx (1)
274-296: Consider only forwarding the flag to LIGHT drops.
suspendLightDropHydrationis forwarded to everyVirtualScrollWrapper(FULL and LIGHT) and is part of the wrapper'suseEffectdeps. Each flip of the flag therefore tears down and recreates theIntersectionObserverfor every drop in the list, even FULL drops where the flag has no effect. For long conversations with hundreds of items, each target-scroll (flag goes true → false on settle) incurs 2×N observer recreations.Minor win — only pay the dep-churn cost where it's meaningful:
♻️ Proposed refactor
<VirtualScrollWrapper scrollContainerRef={getItemData.scrollContainerRef} dropSerialNo={drop.serial_no} waveId={drop.type === DropSize.FULL ? drop.wave.id : drop.waveId} type={drop.type} - suspendLightDropHydration={suspendLightDropHydration} + suspendLightDropHydration={ + drop.type === DropSize.LIGHT ? suspendLightDropHydration : false + } >This keeps the flag stable (
false) for FULL drops so their observer effect isn't invalidated when suspension toggles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/DropsList.tsx` around lines 274 - 296, The suspendLightDropHydration flag is being passed to every VirtualScrollWrapper causing its useEffect (and IntersectionObserver) to tear down/recreate for all drops; only LIGHT drops need this prop. Change the prop passed to VirtualScrollWrapper so it evaluates to suspendLightDropHydration only when drop.type === DropSize.LIGHT (otherwise pass a stable false), i.e. update the component invocation around VirtualScrollWrapper to compute the prop per-drop and avoid forwarding suspendLightDropHydration for FULL drops; this keeps VirtualScrollWrapper's deps stable for FULL drops and prevents unnecessary observer churn.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/nextConfig.ts`:
- Line 17: The commit replaced a shared dev IP in the allowedDevOrigins array
with a personal hotspot address; revert this accidental change by restoring the
original entry (replace "172.20.10.3" with the prior "192.168.1.133") or,
better, change allowedDevOrigins to load from an environment variable (e.g.,
parse process.env.ALLOWED_DEV_ORIGINS as a comma-separated list with a safe
default array) so individual developer LAN IPs are not committed; update the
variable/constant named allowedDevOrigins accordingly and ensure any type/format
expectations still hold where allowedDevOrigins is used.
---
Outside diff comments:
In `@components/waves/drops/VirtualScrollWrapper.tsx`:
- Around line 111-158: The effect can fire many LIGHT-drop fetches when
suspendLightDropHydration flips from true→false; add a small debounce/batch
guard in VirtualScrollWrapper's useEffect so inView && type === DropSize.LIGHT
calls to fetchAroundSerialNo are coalesced briefly (e.g., microtask/setTimeout
50–100ms) before invoking fetchAroundSerialNo, or gate calls behind a per-wave
"hydrationResumed" flag to avoid simultaneous spikes; also add a test covering
the suspendLightDropHydration toggle to assert that useWavePagination's
queue/_processAroundSerialNoQueue handles the burst without duplicate work and
that fetchAroundSerialNo is only called once per unique (waveId, dropSerialNo)
within the debounce window.
---
Nitpick comments:
In `@components/drops/view/DropsList.tsx`:
- Around line 274-296: The suspendLightDropHydration flag is being passed to
every VirtualScrollWrapper causing its useEffect (and IntersectionObserver) to
tear down/recreate for all drops; only LIGHT drops need this prop. Change the
prop passed to VirtualScrollWrapper so it evaluates to suspendLightDropHydration
only when drop.type === DropSize.LIGHT (otherwise pass a stable false), i.e.
update the component invocation around VirtualScrollWrapper to compute the prop
per-drop and avoid forwarding suspendLightDropHydration for FULL drops; this
keeps VirtualScrollWrapper's deps stable for FULL drops and prevents unnecessary
observer churn.
🪄 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: 3373d668-452c-4c4b-a4f8-a4ec4ae85f2b
📒 Files selected for processing (8)
__tests__/components/waves/drops/WaveDropsAll.test.tsxcomponents/drops/view/DropsList.tsxcomponents/waves/drops/VirtualScrollWrapper.tsxcomponents/waves/drops/wave-drops-all/hooks/useWaveDropsSerialScroll.tscomponents/waves/drops/wave-drops-all/index.tsxcomponents/waves/drops/wave-drops-all/subcomponents/WaveDropsContent.tsxcomponents/waves/drops/wave-drops-all/subcomponents/WaveDropsMessageListSection.tsxconfig/nextConfig.ts
2 participants
Summary by CodeRabbit
New Features
Tests