From 30b2bc9f1cb71cbc096ba1ef7a2cb47dc9d67522 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 18 Feb 2025 14:15:52 -0500 Subject: [PATCH 1/3] Add observer methods to Fragment instances --- .../fragment-refs/EventListenerCase.js | 96 +++++++++++ .../fragment-refs/IntersectionObserverCase.js | 152 ++++++++++++++++++ .../fragment-refs/ResizeObserverCase.js | 63 ++++++++ .../fixtures/fragment-refs/index.js | 100 +----------- fixtures/dom/src/style.css | 36 +++++ .../src/client/ReactFiberConfigDOM.js | 63 ++++++++ .../__tests__/ReactDOMFragmentRefs-test.js | 142 ++++++++++++++++ .../__tests__/ReactDOMTestSelectors-test.js | 105 +++--------- .../src/__tests__/utils/IntersectionMocks.js | 77 +++++++++ scripts/error-codes/codes.json | 5 +- 10 files changed, 657 insertions(+), 182 deletions(-) create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/EventListenerCase.js create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/ResizeObserverCase.js create mode 100644 packages/react-dom/src/__tests__/utils/IntersectionMocks.js diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/EventListenerCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/EventListenerCase.js new file mode 100644 index 0000000000000..b3baf5a381d7a --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/EventListenerCase.js @@ -0,0 +1,96 @@ +import TestCase from '../../TestCase'; +import Fixture from '../../Fixture'; + +const React = window.React; +const {Fragment, useEffect, useRef, useState} = React; + +function WrapperComponent(props) { + return props.children; +} + +function handler(e) { + const text = e.currentTarget.innerText; + alert('You clicked: ' + text); +} + +export default function EventListenerCase() { + const fragmentRef = useRef(null); + const [extraChildCount, setExtraChildCount] = useState(0); + + useEffect(() => { + fragmentRef.current.addEventListener('click', handler); + + const lastFragmentRefValue = fragmentRef.current; + return () => { + lastFragmentRefValue.removeEventListener('click', handler); + }; + }); + + return ( + + +
  • Click one of the children, observe the alert
    • +
  • Add a new child, click it, observe the alert
    • +
  • Remove the event listeners, click a child, observe no alert
    • +
  • Add the event listeners back, click a child, observe the alert
    • +     + + +

    + Fragment refs can manage event listeners on the first level of host + children. This page loads with an effect that sets up click event + hanndlers on each child card. Clicking on a card will show an alert + with the card's text. +

    +

    + New child nodes will also have event listeners applied. Removed nodes + will have their listeners cleaned up. +

    +     + + +
    +
    Target count: {extraChildCount + 3}
    + + + +
    + +
    + Child A +
    +
    + Child B +
    + +
    + Child C +
    + {Array.from({length: extraChildCount}).map((_, index) => ( +
    + Extra Child {index} +
    + ))} +     +     +
    +
    +     +     + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js new file mode 100644 index 0000000000000..9edc02d69cc59 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js @@ -0,0 +1,152 @@ +import TestCase from '../../TestCase'; +import Fixture from '../../Fixture'; + +const React = window.React; +const {Fragment, useEffect, useRef, useState} = React; + +function WrapperComponent(props) { + return props.children; +} + +function ObservedChild({id}) { + return ( +
    + {id} +
    + ); +} + +const initialItems = [ + ['A', false], + ['B', false], + ['C', false], +]; + +export default function IntersectionObserverCase() { + const fragmentRef = useRef(null); + const [items, setItems] = useState(initialItems); + const addedItems = items.slice(3); + const anyOnScreen = items.some(([, onScreen]) => onScreen); + const observerRef = useRef(null); + + useEffect(() => { + if (observerRef.current === null) { + observerRef.current = new IntersectionObserver( + entries => { + setItems(prev => { + const newItems = [...prev]; + entries.forEach(entry => { + const index = newItems.findIndex( + ([id]) => id === entry.target.id + ); + newItems[index] = [entry.target.id, entry.isIntersecting]; + }); + return newItems; + }); + }, + { + threshold: [0.5], + } + ); + } + fragmentRef.current.observeUsing(observerRef.current); + + const lastFragmentRefValue = fragmentRef.current; + return () => { + lastFragmentRefValue.unobserveUsing(observerRef.current); + }; + }, []); + + return ( + + +
  • + Scroll the children into view, observe the sidebar appears and shows + which children are in the viewport +
    • +
  • + Add a new child and observe that the Intersection Observer is applied +
    • +
  • + Click Unobserve and observe that the state of children in the viewport + is no longer updated +
    • +
  • + Click Observe and observe that the state of children in the viewport + is updated again +
    • +     + + +

    + Fragment refs manage Intersection Observers on the first level of host + children. This page loads with an effect that sets up an Inersection + Observer applied to each child card. +

    +

    + New child nodes will also have the observer applied. Removed nodes + will be unobserved. +

    +     + + + + + + {anyOnScreen && ( +
    +

    + Children on screen: +

    + {items.map(item => ( +
    + {item[0]} +
    + ))} +
    + )} + + + + + + + {addedItems.map((_, index) => ( + + ))} + +     +     + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/ResizeObserverCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/ResizeObserverCase.js new file mode 100644 index 0000000000000..ecacd50921374 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/ResizeObserverCase.js @@ -0,0 +1,63 @@ +import TestCase from '../../TestCase'; +import Fixture from '../../Fixture'; + +const React = window.React; +const {Fragment, useEffect, useRef, useState} = React; + +export default function ResizeObserverCase() { + const fragmentRef = useRef(null); + const [width, setWidth] = useState([0, 0, 0]); + + useEffect(() => { + const resizeObserver = new window.ResizeObserver(entries => { + if (entries.length > 0) { + setWidth(prev => { + const newWidth = [...prev]; + entries.forEach(entry => { + const index = parseInt(entry.target.id, 10); + newWidth[index] = Math.round(entry.contentRect.width); + }); + return newWidth; + }); + } + }); + + fragmentRef.current.observeUsing(resizeObserver); + const lastFragmentRefValue = fragmentRef.current; + return () => { + lastFragmentRefValue.unobserveUsing(resizeObserver); + }; + }, []); + + return ( + + +
  • Resize the viewport width until the children respond
    • +
  • See that the width data updates as they elements resize
    • +     + + The Fragment Ref has a ResizeObserver attached which has a callback to + update the width state of each child node. + + + +
    +

    + Width: {width[0]}px +

    +
    +
    +

    + Width: {width[1]}px +

    +
    +
    +

    + Width: {width[2]}px +

    +
    +     +     +     + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/index.js b/fixtures/dom/src/components/fixtures/fragment-refs/index.js index bd4468126e43b..03d95ec30a98d 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/index.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/index.js @@ -1,104 +1,16 @@ -import Fixture from '../../Fixture'; import FixtureSet from '../../FixtureSet'; -import TestCase from '../../TestCase'; +import EventListenerCase from './EventListenerCase'; +import IntersectionObserverCase from './IntersectionObserverCase'; +import ResizeObserverCase from './ResizeObserverCase'; const React = window.React; -const {Fragment, useEffect, useRef, useState} = React; - -function WrapperComponent(props) { - return props.children; -} - -function handler(e) { - const text = e.currentTarget.innerText; - alert('You clicked: ' + text); -} export default function FragmentRefsPage() { - const fragmentRef = useRef(null); - const [extraChildCount, setExtraChildCount] = useState(0); - - React.useEffect(() => { - fragmentRef.current.addEventListener('click', handler); - - const lastFragmentRefValue = fragmentRef.current; - return () => { - lastFragmentRefValue.removeEventListener('click', handler); - }; - }); - return ( - - -
  • Click one of the children, observe the alert
    • -
  • Add a new child, click it, observe the alert
    • -
  • Remove the event listeners, click a child, observe no alert
    • -
  • - Add the event listeners back, click a child, observe the alert -
    • -     - - -

    - Fragment refs can manage event listeners on the first level of host - children. This page loads with an effect that sets up click event - hanndlers on each child card. Clicking on a card will show an alert - with the card's text. -

    -

    - New child nodes will also have event listeners applied. Removed - nodes will have their listeners cleaned up. -

    -     - - -
    -
    Target count: {extraChildCount + 3}
    - - - -
    - -
    - Child A -
    -
    - Child B -
    - -
    - Child C -
    - {Array.from({length: extraChildCount}).map((_, index) => ( -
    - Extra Child {index} -
    - ))} -     -     -
    -
    -     -     + + +     ); } diff --git a/fixtures/dom/src/style.css b/fixtures/dom/src/style.css index 568d8b4b0d1b5..30a9ed5fff972 100644 --- a/fixtures/dom/src/style.css +++ b/fixtures/dom/src/style.css @@ -322,3 +322,39 @@ tbody tr:nth-child(even) { margin: 10px; padding: 10px; } + +.observable-card { + height: 200px; + border: 1px solid black; + background: #e0e0e0; + padding: 20px; + font-size: 18px; + overflow: auto; + margin-bottom: 50px; + position: relative; +} + +.observable-card::after { + content: ""; + position: absolute; + top: 50%; + left: 0; + width: 100%; + border-top: 1px dotted red; +} + +.fixed-sidebar { + position: fixed; + top: 0; + left: 0; + height: 100%; + width: 200px; + z-index: 1000; + background-color: gray; + display: flex; + flex-direction: column; +} + +.onscreen { + background-color: green; +} diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 1e9ee657e0ca8..3031b1666c7fa 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2186,6 +2186,7 @@ type StoredEventListener = { export type FragmentInstanceType = { _fragmentFiber: Fiber, _eventListeners: null | Array, + _observer: null | IntersectionObserver | ResizeObserver, addEventListener( type: string, listener: EventListener, @@ -2197,11 +2198,14 @@ export type FragmentInstanceType = { optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void, focus(): void, + observeUsing(observer: IntersectionObserver | ResizeObserver): void, + unobserveUsing(observer: IntersectionObserver | ResizeObserver): void, }; function FragmentInstance(this: FragmentInstanceType, fragmentFiber: Fiber) { this._fragmentFiber = fragmentFiber; this._eventListeners = null; + this._observer = null; } // $FlowFixMe[prop-missing] FragmentInstance.prototype.addEventListener = function ( @@ -2284,6 +2288,62 @@ function removeEventListenerFromChild( FragmentInstance.prototype.focus = function (this: FragmentInstanceType) { traverseFragmentInstance(this._fragmentFiber, setFocusIfFocusable); }; +// $FlowFixMe[prop-missing] +FragmentInstance.prototype.observeUsing = function ( + this: FragmentInstanceType, + observer: IntersectionObserver | ResizeObserver, +): void { + if (this._observer !== null && this._observer !== observer) { + throw new Error( + 'You are attaching an observer to a fragment instance that already has one. Fragment instances ' + + 'can only have one observer. Use multiple fragment instances or first call unobserveUsing() to ' + + 'remove the previous observer.', + ); + } + this._observer = observer; + traverseFragmentInstanceChildren( + this, + this._fragmentFiber.child, + observeChild, + observer, + ); +}; +function observeChild( + child: Instance, + observer: IntersectionObserver | ResizeObserver, +) { + observer.observe(child); + return false; +} +// $FlowFixMe[prop-missing] +FragmentInstance.prototype.unobserveUsing = function ( + this: FragmentInstanceType, + observer: IntersectionObserver | ResizeObserver, +): void { + if (this._observer === null || this._observer !== observer) { + if (__DEV__) { + console.error( + 'You are calling unobserveUsing() with an observer that is not being observed with this fragment ' + + 'instance. First attach the observer with observeUsing()', + ); + } + } else { + traverseFragmentInstanceChildren( + this, + this._fragmentFiber.child, + unobserveChild, + observer, + ); + this._observer = null; + } +}; +function unobserveChild( + child: Instance, + observer: IntersectionObserver | ResizeObserver, +) { + observer.unobserve(child); + return false; +} function normalizeListenerOptions( opts: ?EventListenerOptionsOrUseCapture, @@ -2343,6 +2403,9 @@ export function commitNewChildToFragmentInstance( childElement.addEventListener(type, listener, optionsOrUseCapture); } } + if (fragmentInstance._observer !== null) { + fragmentInstance._observer.observe(childElement); + } } export function deleteChildFromFragmentInstance( diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 727fd3014201f..727df271893f3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -15,6 +15,9 @@ let act; let container; let Fragment; let Activity; +let mockIntersectionObserver; +let simulateIntersection; +let assertConsoleErrorDev; describe('FragmentRefs', () => { beforeEach(() => { @@ -24,6 +27,12 @@ describe('FragmentRefs', () => { Activity = React.unstable_Activity; ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; + const IntersectionMocks = require('./utils/IntersectionMocks'); + mockIntersectionObserver = IntersectionMocks.mockIntersectionObserver; + simulateIntersection = IntersectionMocks.simulateIntersection; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; + container = document.createElement('div'); document.body.appendChild(container); }); @@ -617,4 +626,137 @@ describe('FragmentRefs', () => { }); }); }); + + describe('observers', () => { + beforeEach(() => { + mockIntersectionObserver(); + }); + + // @gate enableFragmentRefs + it('attaches intersection observers to children', async () => { + let logs = []; + const observer = new IntersectionObserver(entries => { + entries.forEach(entry => { + logs.push(entry.target.id); + }); + }); + function Test({showB}) { + const fragmentRef = React.useRef(null); + React.useEffect(() => { + fragmentRef.current.observeUsing(observer); + const lastRefValue = fragmentRef.current; + return () => { + lastRefValue.unobserveUsing(observer); + }; + }, []); + return ( +
    + +
    A
    + {showB &&
    B
    } +     +
    + ); + } + + function simulateAllChildrenIntersecting() { + const parent = container.firstChild; + if (parent) { + const children = Array.from(parent.children).map(child => { + return [child, {y: 0, x: 0, width: 1, height: 1}, 1]; + }); + simulateIntersection(...children); + } + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + simulateAllChildrenIntersecting(); + expect(logs).toEqual(['childA']); + + // Reveal child and expect it to be observed + logs = []; + await act(() => root.render()); + simulateAllChildrenIntersecting(); + expect(logs).toEqual(['childA', 'childB']); + + // Hide child and expect it to be unobserved + logs = []; + await act(() => root.render()); + simulateAllChildrenIntersecting(); + expect(logs).toEqual(['childA']); + + // Unmount component and expect all children to be unobserved + logs = []; + await act(() => root.render(null)); + simulateAllChildrenIntersecting(); + expect(logs).toEqual([]); + }); + + // @gate enableFragmentRefs + it('errors when observeUsing() is called with multiple observers', async () => { + const fragmentRef = React.createRef(); + const observer = new IntersectionObserver(() => {}); + const observer2 = new IntersectionObserver(() => {}); + function Test() { + return ( + +
    + + ); + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + fragmentRef.current.observeUsing(observer); + // Same observer doesn't throw + fragmentRef.current.observeUsing(observer); + // Different observer throws + expect(() => { + fragmentRef.current.observeUsing(observer2); + }).toThrowError( + 'You are attaching an observer to a fragment instance that already has one. Fragment instances ' + + 'can only have one observer. Use multiple fragment instances or first call unobserveUsing() to ' + + 'remove the previous observer.', + ); + }); + + // @gate enableFragmentRefs + it('warns when unobserveUsing() is called with an observer that was not observed', async () => { + const fragmentRef = React.createRef(); + const observer = new IntersectionObserver(() => {}); + const observer2 = new IntersectionObserver(() => {}); + function Test() { + return ( + +
    + + ); + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + + // Warning when there is no attached observer + fragmentRef.current.unobserveUsing(observer); + assertConsoleErrorDev( + [ + 'You are calling unobserveUsing() with an observer that is not being observed with this fragment ' + + 'instance. First attach the observer with observeUsing()', + ], + {withoutStack: true}, + ); + + // Warning when the attached observer does not match + fragmentRef.current.observeUsing(observer); + fragmentRef.current.unobserveUsing(observer2); + assertConsoleErrorDev( + [ + 'You are calling unobserveUsing() with an observer that is not being observed with this fragment ' + + 'instance. First attach the observer with observeUsing()', + ], + {withoutStack: true}, + ); + }); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMTestSelectors-test.js b/packages/react-dom/src/__tests__/ReactDOMTestSelectors-test.js index 65bb49e06a1c8..8b6eb0d49d5e7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMTestSelectors-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMTestSelectors-test.js @@ -23,6 +23,9 @@ describe('ReactDOMTestSelectors', () => { let focusWithin; let getFindAllNodesFailureDescription; let observeVisibleRects; + let mockIntersectionObserver; + let simulateIntersection; + let setBoundingClientRect; let container; @@ -51,6 +54,10 @@ describe('ReactDOMTestSelectors', () => { container = document.createElement('div'); document.body.appendChild(container); + const IntersectionMocks = require('./utils/IntersectionMocks'); + mockIntersectionObserver = IntersectionMocks.mockIntersectionObserver; + simulateIntersection = IntersectionMocks.simulateIntersection; + setBoundingClientRect = IntersectionMocks.setBoundingClientRect; }); afterEach(() => { @@ -608,21 +615,6 @@ No matching component was found for: }); describe('findBoundingRects', () => { - // Stub out getBoundingClientRect for the specified target. - // This API is required by the test selectors but it isn't implemented by jsdom. - function setBoundingClientRect(target, {x, y, width, height}) { - target.getBoundingClientRect = function () { - return { - width, - height, - left: x, - right: x + width, - top: y, - bottom: y + height, - }; - }; - } - // @gate www || experimental it('should return a single rect for a component that returns a single root host element', async () => { const ref = React.createRef(); @@ -1223,69 +1215,10 @@ No matching component was found for: }); describe('observeVisibleRects', () => { - // Stub out getBoundingClientRect for the specified target. - // This API is required by the test selectors but it isn't implemented by jsdom. - function setBoundingClientRect(target, {x, y, width, height}) { - target.getBoundingClientRect = function () { - return { - width, - height, - left: x, - right: x + width, - top: y, - bottom: y + height, - }; - }; - } - - function simulateIntersection(...entries) { - callback( - entries.map(([target, rect, ratio]) => ({ - boundingClientRect: { - top: rect.y, - left: rect.x, - width: rect.width, - height: rect.height, - }, - intersectionRatio: ratio, - target, - })), - ); - } - - let callback; - let observedTargets; + let observerMock; beforeEach(() => { - callback = null; - observedTargets = []; - - class IntersectionObserver { - constructor() { - callback = arguments[0]; - } - - disconnect() { - callback = null; - observedTargets.splice(0); - } - - observe(target) { - observedTargets.push(target); - } - - unobserve(target) { - const index = observedTargets.indexOf(target); - if (index >= 0) { - observedTargets.splice(index, 1); - } - } - } - - // This is a broken polyfill. - // It is only intended to provide bare minimum test coverage. - // More meaningful tests will require the use of fixtures. - window.IntersectionObserver = IntersectionObserver; + observerMock = mockIntersectionObserver(); }); // @gate www || experimental @@ -1317,8 +1250,8 @@ No matching component was found for: handleVisibilityChange, ); - expect(callback).not.toBeNull(); - expect(observedTargets).toHaveLength(1); + expect(observerMock.callback).not.toBeNull(); + expect(observerMock.observedTargets).toHaveLength(1); expect(handleVisibilityChange).not.toHaveBeenCalled(); // Simulate IntersectionObserver notification. @@ -1370,8 +1303,8 @@ No matching component was found for: handleVisibilityChange, ); - expect(callback).not.toBeNull(); - expect(observedTargets).toHaveLength(2); + expect(observerMock.callback).not.toBeNull(); + expect(observerMock.observedTargets).toHaveLength(2); expect(handleVisibilityChange).not.toHaveBeenCalled(); // Simulate IntersectionObserver notification. @@ -1437,12 +1370,12 @@ No matching component was found for: handleVisibilityChange, ); - expect(callback).not.toBeNull(); - expect(observedTargets).toHaveLength(1); + expect(observerMock.callback).not.toBeNull(); + expect(observerMock.observedTargets).toHaveLength(1); expect(handleVisibilityChange).not.toHaveBeenCalled(); disconnect(); - expect(callback).toBeNull(); + expect(observerMock.callback).toBeNull(); }); // This test reuires gating because it relies on the __DEV__ only commit hook to work. @@ -1570,9 +1503,9 @@ No matching component was found for: handleVisibilityChange, ); - expect(callback).not.toBeNull(); - expect(observedTargets).toHaveLength(1); - expect(observedTargets[0]).toBe(ref1.current); + expect(observerMock.callback).not.toBeNull(); + expect(observerMock.observedTargets).toHaveLength(1); + expect(observerMock.observedTargets[0]).toBe(ref1.current); }); }); }); diff --git a/packages/react-dom/src/__tests__/utils/IntersectionMocks.js b/packages/react-dom/src/__tests__/utils/IntersectionMocks.js new file mode 100644 index 0000000000000..a6a6b68caa0c7 --- /dev/null +++ b/packages/react-dom/src/__tests__/utils/IntersectionMocks.js @@ -0,0 +1,77 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +const intersectionObserverMock = {callback: null, observedTargets: []}; + +/** + * This is a broken polyfill. + * It is only intended to provide bare minimum test coverage. + * More meaningful tests will require the use of fixtures. + */ +export function mockIntersectionObserver() { + intersectionObserverMock.callback = null; + intersectionObserverMock.observedTargets = []; + + class IntersectionObserver { + constructor() { + intersectionObserverMock.callback = arguments[0]; + } + + disconnect() { + intersectionObserverMock.callback = null; + intersectionObserverMock.observedTargets.splice(0); + } + + observe(target) { + intersectionObserverMock.observedTargets.push(target); + } + + unobserve(target) { + const index = intersectionObserverMock.observedTargets.indexOf(target); + if (index >= 0) { + intersectionObserverMock.observedTargets.splice(index, 1); + } + } + } + + window.IntersectionObserver = IntersectionObserver; + + return intersectionObserverMock; +} + +export function simulateIntersection(...entries) { + intersectionObserverMock.callback( + entries.map(([target, rect, ratio]) => ({ + boundingClientRect: { + top: rect.y, + left: rect.x, + width: rect.width, + height: rect.height, + }, + intersectionRatio: ratio, + target, + })), + ); +} + +/** + * Stub out getBoundingClientRect for the specified target. + * This API is required by the test selectors but it isn't implemented by jsdom. + */ +export function setBoundingClientRect(target, {x, y, width, height}) { + target.getBoundingClientRect = function () { + return { + width, + height, + left: x, + right: x + width, + top: y, + bottom: y + height, + }; + }; +} diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 9db8a9cb892bc..c0d120a54626c 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -537,5 +537,6 @@ "549": "Cannot start a gesture with a disconnected AnimationTimeline.", "550": "useSwipeTransition is not yet supported in react-art.", "551": "useSwipeTransition is not yet supported in React Native.", - "552": "Cannot use a useSwipeTransition() in a detached root." -} + "552": "Cannot use a useSwipeTransition() in a detached root.", + "553": "You are attaching an observer to a fragment instance that already has one. Fragment instances can only have one observer. Use multiple fragment instances or first call unobserveUsing() to remove the previous observer." +} \ No newline at end of file From 71583c71372f1114199ce4d0fbb31e5fdacf56e0 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 17 Mar 2025 10:56:09 -0400 Subject: [PATCH 2/3] Accept multiple observers per fragment --- .../fragment-refs/IntersectionObserverCase.js | 1 + .../src/client/ReactFiberConfigDOM.js | 24 ++++++++-------- .../__tests__/ReactDOMFragmentRefs-test.js | 28 ------------------- scripts/error-codes/codes.json | 5 ++-- 4 files changed, 14 insertions(+), 44 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js index 9edc02d69cc59..e087215aee4df 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/IntersectionObserverCase.js @@ -54,6 +54,7 @@ export default function IntersectionObserverCase() { const lastFragmentRefValue = fragmentRef.current; return () => { lastFragmentRefValue.unobserveUsing(observerRef.current); + observerRef.current = null; }; }, []); diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 3031b1666c7fa..bfd7b49eeca18 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2186,7 +2186,7 @@ type StoredEventListener = { export type FragmentInstanceType = { _fragmentFiber: Fiber, _eventListeners: null | Array, - _observer: null | IntersectionObserver | ResizeObserver, + _observers: null | Set, addEventListener( type: string, listener: EventListener, @@ -2205,7 +2205,7 @@ export type FragmentInstanceType = { function FragmentInstance(this: FragmentInstanceType, fragmentFiber: Fiber) { this._fragmentFiber = fragmentFiber; this._eventListeners = null; - this._observer = null; + this._observers = null; } // $FlowFixMe[prop-missing] FragmentInstance.prototype.addEventListener = function ( @@ -2293,14 +2293,10 @@ FragmentInstance.prototype.observeUsing = function ( this: FragmentInstanceType, observer: IntersectionObserver | ResizeObserver, ): void { - if (this._observer !== null && this._observer !== observer) { - throw new Error( - 'You are attaching an observer to a fragment instance that already has one. Fragment instances ' + - 'can only have one observer. Use multiple fragment instances or first call unobserveUsing() to ' + - 'remove the previous observer.', - ); + if (this._observers === null) { + this._observers = new Set(); } - this._observer = observer; + this._observers.add(observer); traverseFragmentInstanceChildren( this, this._fragmentFiber.child, @@ -2320,7 +2316,7 @@ FragmentInstance.prototype.unobserveUsing = function ( this: FragmentInstanceType, observer: IntersectionObserver | ResizeObserver, ): void { - if (this._observer === null || this._observer !== observer) { + if (this._observers === null || !this._observers.has(observer)) { if (__DEV__) { console.error( 'You are calling unobserveUsing() with an observer that is not being observed with this fragment ' + @@ -2328,13 +2324,13 @@ FragmentInstance.prototype.unobserveUsing = function ( ); } } else { + this._observers.delete(observer); traverseFragmentInstanceChildren( this, this._fragmentFiber.child, unobserveChild, observer, ); - this._observer = null; } }; function unobserveChild( @@ -2403,8 +2399,10 @@ export function commitNewChildToFragmentInstance( childElement.addEventListener(type, listener, optionsOrUseCapture); } } - if (fragmentInstance._observer !== null) { - fragmentInstance._observer.observe(childElement); + if (fragmentInstance._observers !== null) { + fragmentInstance._observers.forEach(observer => { + observer.observe(childElement); + }); } } diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 727df271893f3..e1d8532b7ee99 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -693,34 +693,6 @@ describe('FragmentRefs', () => { expect(logs).toEqual([]); }); - // @gate enableFragmentRefs - it('errors when observeUsing() is called with multiple observers', async () => { - const fragmentRef = React.createRef(); - const observer = new IntersectionObserver(() => {}); - const observer2 = new IntersectionObserver(() => {}); - function Test() { - return ( - -
    - - ); - } - - const root = ReactDOMClient.createRoot(container); - await act(() => root.render()); - fragmentRef.current.observeUsing(observer); - // Same observer doesn't throw - fragmentRef.current.observeUsing(observer); - // Different observer throws - expect(() => { - fragmentRef.current.observeUsing(observer2); - }).toThrowError( - 'You are attaching an observer to a fragment instance that already has one. Fragment instances ' + - 'can only have one observer. Use multiple fragment instances or first call unobserveUsing() to ' + - 'remove the previous observer.', - ); - }); - // @gate enableFragmentRefs it('warns when unobserveUsing() is called with an observer that was not observed', async () => { const fragmentRef = React.createRef(); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index c0d120a54626c..9db8a9cb892bc 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -537,6 +537,5 @@ "549": "Cannot start a gesture with a disconnected AnimationTimeline.", "550": "useSwipeTransition is not yet supported in react-art.", "551": "useSwipeTransition is not yet supported in React Native.", - "552": "Cannot use a useSwipeTransition() in a detached root.", - "553": "You are attaching an observer to a fragment instance that already has one. Fragment instances can only have one observer. Use multiple fragment instances or first call unobserveUsing() to remove the previous observer." -} \ No newline at end of file + "552": "Cannot use a useSwipeTransition() in a detached root." +} From 6e372d64e13721a9dedce68b64e66f944e4da78f Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 17 Mar 2025 10:58:49 -0400 Subject: [PATCH 3/3] Use traverseFragmentInstance fn from rebase --- .../src/client/ReactFiberConfigDOM.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index bfd7b49eeca18..2d34bc94005be 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2297,12 +2297,7 @@ FragmentInstance.prototype.observeUsing = function ( this._observers = new Set(); } this._observers.add(observer); - traverseFragmentInstanceChildren( - this, - this._fragmentFiber.child, - observeChild, - observer, - ); + traverseFragmentInstance(this._fragmentFiber, observeChild, observer); }; function observeChild( child: Instance, @@ -2325,12 +2320,7 @@ FragmentInstance.prototype.unobserveUsing = function ( } } else { this._observers.delete(observer); - traverseFragmentInstanceChildren( - this, - this._fragmentFiber.child, - unobserveChild, - observer, - ); + traverseFragmentInstance(this._fragmentFiber, unobserveChild, observer); } }; function unobserveChild(