diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 5bd3357d19..3d4d17fdf6 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -650,14 +650,6 @@ "count": 2 } }, - "src/components/hooks/useFullscreen.ts": { - "@typescript-eslint/no-floating-promises": { - "count": 2 - }, - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, "src/components/hooks/useLandingEntry.native.ts": { "react-hooks/set-state-in-effect": { "count": 1 diff --git a/src/components/Post/Embed/VideoEmbed/ActiveVideoWebContext.tsx b/src/components/Post/Embed/VideoEmbed/ActiveVideoWebContext.tsx index d0f2660ba4..90f399eb21 100644 --- a/src/components/Post/Embed/VideoEmbed/ActiveVideoWebContext.tsx +++ b/src/components/Post/Embed/VideoEmbed/ActiveVideoWebContext.tsx @@ -108,10 +108,15 @@ export function useActiveVideoWeb() { return { active: activeViewId === id, - setActive: () => { + setActive: useCallback(() => { setActiveView(id) - }, + }, [setActiveView, id]), currentActiveView: activeViewId, - sendPosition: (y: number) => sendViewPosition(id, y), + sendPosition: useCallback( + (y: number) => { + sendViewPosition(id, y) + }, + [sendViewPosition, id], + ), } } diff --git a/src/components/hooks/useFullscreen.ts b/src/components/hooks/useFullscreen.ts index 107095ef84..3578dd9fc4 100644 --- a/src/components/hooks/useFullscreen.ts +++ b/src/components/hooks/useFullscreen.ts @@ -1,10 +1,4 @@ -import { - useCallback, - useEffect, - useRef, - useState, - useSyncExternalStore, -} from 'react' +import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react' import {IS_WEB, IS_WEB_FIREFOX, IS_WEB_SAFARI} from '#/env' @@ -13,28 +7,38 @@ function fullscreenSubscribe(onChange: () => void) { return () => document.removeEventListener('fullscreenchange', onChange) } +function getFullscreenSnapshot() { + return Boolean(document.fullscreenElement) +} + export function useFullscreen(ref?: React.RefObject) { if (!IS_WEB) throw new Error("'useFullscreen' is a web-only hook") - const isFullscreen = useSyncExternalStore(fullscreenSubscribe, () => - Boolean(document.fullscreenElement), + const isFullscreen = useSyncExternalStore( + fullscreenSubscribe, + getFullscreenSnapshot, ) const scrollYRef = useRef(null) - const [prevIsFullscreen, setPrevIsFullscreen] = useState(isFullscreen) + // Tracked via a ref rather than state so that reacting to a fullscreen change + // never schedules its own render. Scheduling a render in response to the + // external store value (the old `setPrevIsFullscreen` pattern) was a seed for + // the commit-phase update loop reported in APP-315 / APP-5PP / APP-7ZB. + const prevIsFullscreenRef = useRef(isFullscreen) const toggleFullscreen = useCallback(() => { if (isFullscreen) { - document.exitFullscreen() + void document.exitFullscreen() } else { if (!ref) throw new Error('No ref provided') if (!ref.current) return scrollYRef.current = window.scrollY - ref.current.requestFullscreen() + void ref.current.requestFullscreen() } }, [isFullscreen, ref]) useEffect(() => { + const prevIsFullscreen = prevIsFullscreenRef.current if (prevIsFullscreen === isFullscreen) return - setPrevIsFullscreen(isFullscreen) + prevIsFullscreenRef.current = isFullscreen // Chrome has an issue where it doesn't scroll back to the top after exiting fullscreen // Let's play it safe and do it if not FF or Safari, since anything else will probably be chromium @@ -46,7 +50,7 @@ export function useFullscreen(ref?: React.RefObject) { } }, 100) } - }, [isFullscreen, prevIsFullscreen]) + }, [isFullscreen]) return [isFullscreen, toggleFullscreen] as const }