fix: properly remove scroll listener in tracing beam#278
fix: properly remove scroll listener in tracing beam#278Aaron-zon wants to merge 1 commit intounovue:mainfrom
Conversation
📝 WalkthroughWalkthroughA one-line bug fix in TracingBeam.vue corrects the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
app/components/inspira/ui/tracing-beam/TracingBeam.vue (1)
60-67: Optional: also disconnect theResizeObserveron unmount.While addressing listener cleanup, consider also disconnecting the
ResizeObservercreated inonMountedto avoid a lingering observer on the detached content node. Hoist it out of the closure soonUnmountedcan reach it.♻️ Proposed refactor
+let resizeObserver: ResizeObserver | undefined; + onMounted(() => { window.addEventListener("scroll", updateScrollYProgress); window.addEventListener("resize", updateScrollYProgress); updateScrollYProgress(); - const resizeObserver = new ResizeObserver(() => { + resizeObserver = new ResizeObserver(() => { updateSVGHeight(); }); resizeObserver.observe(tracingBeamContentRef.value!); updateSVGHeight(); }); onUnmounted(() => { window.removeEventListener("scroll", updateScrollYProgress); window.removeEventListener("resize", updateScrollYProgress); + resizeObserver?.disconnect(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/inspira/ui/tracing-beam/TracingBeam.vue` around lines 60 - 67, The ResizeObserver created in the onMounted closure (named resizeObserver) is not disconnected on component unmount; hoist the ResizeObserver variable (resizeObserver) so it’s defined in the component scope (beside tracingBeamContentRef and updateSVGHeight) and create it inside onMounted, then add an onUnmounted handler that calls resizeObserver.disconnect() to stop observing tracingBeamContentRef.value and avoid a lingering observer; ensure you still call resizeObserver.observe(tracingBeamContentRef.value!) after creation and that updateSVGHeight() usage remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/components/inspira/ui/tracing-beam/TracingBeam.vue`:
- Around line 60-67: The ResizeObserver created in the onMounted closure (named
resizeObserver) is not disconnected on component unmount; hoist the
ResizeObserver variable (resizeObserver) so it’s defined in the component scope
(beside tracingBeamContentRef and updateSVGHeight) and create it inside
onMounted, then add an onUnmounted handler that calls
resizeObserver.disconnect() to stop observing tracingBeamContentRef.value and
avoid a lingering observer; ensure you still call
resizeObserver.observe(tracingBeamContentRef.value!) after creation and that
updateSVGHeight() usage remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04b01c1e-e1d2-46c2-90a3-6669ed067b25
📒 Files selected for processing (1)
app/components/inspira/ui/tracing-beam/TracingBeam.vue
In
onMounted, thescrollevent is bound towindow, so inonUnmountedit should also be removed fromwindow.Summary by CodeRabbit
Release Notes