From cfae0a1f785cd5ea7dbb5349e988586f899c4c48 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 8 Sep 2026 14:28:10 +0200 Subject: [PATCH] fix(journey): stop the load-more observer rebuilding itself on every render ScrollTrigger kept onVisible in its effect dependencies, and both callers hand in a fresh arrow on every render: the picker its loadMorePhotos, the gallery its setVisibleCount step. So every unrelated re-render, a photo being ticked included, disconnected the IntersectionObserver and observed again, and each observe() replays the initial callback. The callback moves into a ref. loading stays a dependency, because re-observing once a page has landed is the only thing that still produces a callback when the sentinel never left the 200px margin, so folding that into a ref too would leave paging dead after the first page. FE-COMP-JSCROLLTRIG-009 pins that, 007 and 008 pin the rebuild itself; both are red on the old component. Markup, classes and the signature are untouched. --- .../JourneyDetailPageScrollTrigger.test.tsx | 42 ++++++++++++++++++- .../JourneyDetailPageScrollTrigger.tsx | 15 ++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/client/src/components/Journey/JourneyDetailPageScrollTrigger.test.tsx b/client/src/components/Journey/JourneyDetailPageScrollTrigger.test.tsx index e74216da8..eba644556 100644 --- a/client/src/components/Journey/JourneyDetailPageScrollTrigger.test.tsx +++ b/client/src/components/Journey/JourneyDetailPageScrollTrigger.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-JSCROLLTRIG-001 to FE-COMP-JSCROLLTRIG-006 +// FE-COMP-JSCROLLTRIG-001 to FE-COMP-JSCROLLTRIG-009 import { render } from '../../../tests/helpers/render' import { resetAllStores } from '../../../tests/helpers/store' @@ -87,4 +87,44 @@ describe('ScrollTrigger', () => { unmount() expect(observers[0].disconnect).toHaveBeenCalled() }) + + it('FE-COMP-JSCROLLTRIG-007: a fresh callback identity alone does not rebuild the observer', () => { + // Both callers hand in a new arrow every render; rebuilding for that churn + // replays the initial callback and pulls pages nobody asked for. + const { rerender } = render() + expect(observers).toHaveLength(1) + + rerender() + rerender() + + expect(observers).toHaveLength(1) + expect(observers[0].disconnect).not.toHaveBeenCalled() + }) + + it('FE-COMP-JSCROLLTRIG-008: the surviving observer calls the newest callback, not the one it was built with', () => { + const first = vi.fn() + const second = vi.fn() + const { rerender } = render() + rerender() + + intersect(observers[0], true) + + expect(first).not.toHaveBeenCalled() + expect(second).toHaveBeenCalledTimes(1) + }) + + it('FE-COMP-JSCROLLTRIG-009: a settled page really re-observes the sentinel', () => { + // A browser observer only fires on a threshold crossing plus once per + // observe(). A sentinel that never leaves the 200px margin therefore only + // gets another callback because the loading flip rebuilds the observer, so + // loading must stay a dependency rather than moving into a ref as well. + const onVisible = vi.fn() + const { rerender } = render() + expect(observers).toHaveLength(1) + + rerender() + + expect(observers).toHaveLength(2) + expect(observers[0].disconnect).toHaveBeenCalled() + }) }) diff --git a/client/src/components/Journey/JourneyDetailPageScrollTrigger.tsx b/client/src/components/Journey/JourneyDetailPageScrollTrigger.tsx index 4470b3692..dc5828b92 100644 --- a/client/src/components/Journey/JourneyDetailPageScrollTrigger.tsx +++ b/client/src/components/Journey/JourneyDetailPageScrollTrigger.tsx @@ -2,13 +2,24 @@ import { useEffect, useRef } from 'react' export function ScrollTrigger({ onVisible, loading }: { onVisible: () => void; loading: boolean }) { const ref = useRef(null) + // Both callers hand in a fresh arrow on every render — the picker its + // loadMorePhotos, the gallery its setVisibleCount step. With onVisible in the + // effect deps, every unrelated re-render tore the observer down and observed + // again, and each observe() replays the initial callback. The ref keeps the + // observer out of that churn. + // + // loading stays a dependency on purpose: re-observing after a page has landed + // is the only thing that still produces a callback when the sentinel never + // left the 200px margin. + const onVisibleRef = useRef(onVisible) + onVisibleRef.current = onVisible useEffect(() => { const el = ref.current if (!el) return - const obs = new IntersectionObserver(([entry]) => { if (entry.isIntersecting && !loading) onVisible() }, { rootMargin: '200px' }) + const obs = new IntersectionObserver(([entry]) => { if (entry.isIntersecting && !loading) onVisibleRef.current() }, { rootMargin: '200px' }) obs.observe(el) return () => obs.disconnect() - }, [onVisible, loading]) + }, [loading]) return (