From bd5b1b7639b818a3fbd33ce83bf022a6f9f27b55 Mon Sep 17 00:00:00 2001 From: lauren Date: Wed, 27 Aug 2025 17:58:44 -0400 Subject: [PATCH 1/3] [compiler] Emit better error for unsupported syntax `this` (#34322) --- .../src/HIR/HIRBuilder.ts | 16 ++++++++++++++++ .../compiler/ecma/error.reserved-words.expect.md | 15 ++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts index 272b5fc0752d6..78c756f812d06 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts @@ -323,6 +323,22 @@ export default class HIRBuilder { ], }); } + if (node.name === 'this') { + CompilerError.throwDiagnostic({ + severity: ErrorSeverity.UnsupportedJS, + category: ErrorCategory.UnsupportedSyntax, + reason: '`this` is not supported syntax', + description: + 'React Compiler does not support compiling functions that use `this`', + details: [ + { + kind: 'error', + message: '`this` was used here', + loc: node.loc ?? GeneratedSource, + }, + ], + }); + } const originalName = node.name; let name = originalName; let index = 0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ecma/error.reserved-words.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ecma/error.reserved-words.expect.md index a6ee8a798b583..986fb8a5b26f1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ecma/error.reserved-words.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ecma/error.reserved-words.expect.md @@ -24,9 +24,18 @@ function useThing(fn) { ``` Found 1 error: -Error: Expected a non-reserved identifier name - -`this` is a reserved word in JavaScript and cannot be used as an identifier name. +Error: `this` is not supported syntax + +React Compiler does not support compiling functions that use `this` + +error.reserved-words.ts:8:28 + 6 | + 7 | if (ref.current === null) { +> 8 | ref.current = function (this: unknown, ...args) { + | ^^^^^^^^^^^^^ `this` was used here + 9 | return fnRef.current.call(this, ...args); + 10 | }; + 11 | } ``` \ No newline at end of file From 3434ff4f4b89ad9388c6109312ef95c14652ae21 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 27 Aug 2025 18:05:57 -0400 Subject: [PATCH 2/3] Add scrollIntoView to fragment instances (#32814) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds `experimental_scrollIntoView(alignToTop)`. It doesn't yet support `scrollIntoView(options)`. Cases: - No host children: Without host children, we represent the virtual space of the Fragment by attempting to scroll to the nearest edge by using its siblings. If the preferred sibling is not found, we'll try the other side, and then the parent. - 1 or more host children: In order to handle the case of children spread between multiple scroll containers, we scroll to each child in reverse order based on the `alignToTop` flag. Due to the complexity of multiple scroll containers and dealing with portals, I've added this under a separate feature flag with an experimental prefix. We may stabilize it along with the other APIs, but this allows us to not block the whole feature on it. This PR was previously implementing a much more complex approach to handling multiple scroll containers and portals. We're going to start with the simple loop and see if we can find any concrete use cases where that doesn't suffice. 01f31d43013ba7f6f54fd8a36990bbafc3c3cc68 is the diff between approaches here. --- .../fixtures/fragment-refs/FocusCase.js | 2 +- .../fragment-refs/GetClientRectsCase.js | 2 +- .../fragment-refs/ScrollIntoViewCase.js | 184 ++++++++++ .../ScrollIntoViewCaseComplex.js | 50 +++ .../fragment-refs/ScrollIntoViewCaseSimple.js | 14 + .../ScrollIntoViewTargetElement.js | 18 + .../fixtures/fragment-refs/index.js | 2 + fixtures/dom/src/index.js | 15 +- .../src/client/ReactFiberConfigDOM.js | 134 +++++--- .../__tests__/ReactDOMFragmentRefs-test.js | 319 ++++++++++++++++++ .../src/ReactFiberTreeReflection.js | 50 +++ packages/shared/ReactFeatureFlags.js | 1 + .../ReactFeatureFlags.native-fb-dynamic.js | 1 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/error-codes/codes.json | 3 +- 21 files changed, 755 insertions(+), 47 deletions(-) create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCase.js create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseComplex.js create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseSimple.js create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewTargetElement.js diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js index baff30895c0e0..efd9157b4a5ce 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js @@ -3,7 +3,7 @@ import Fixture from '../../Fixture'; const React = window.React; -const {Fragment, useEffect, useRef, useState} = React; +const {Fragment, useRef} = React; export default function FocusCase() { const fragmentRef = useRef(null); diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/GetClientRectsCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/GetClientRectsCase.js index 7b20a0a2e0d67..563f2ad054294 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/GetClientRectsCase.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/GetClientRectsCase.js @@ -2,7 +2,7 @@ import TestCase from '../../TestCase'; import Fixture from '../../Fixture'; const React = window.React; -const {Fragment, useEffect, useRef, useState} = React; +const {Fragment, useRef, useState} = React; export default function GetClientRectsCase() { const fragmentRef = useRef(null); diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCase.js new file mode 100644 index 0000000000000..3b1f21ef686aa --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCase.js @@ -0,0 +1,184 @@ +import TestCase from '../../TestCase'; +import Fixture from '../../Fixture'; +import ScrollIntoViewCaseComplex from './ScrollIntoViewCaseComplex'; +import ScrollIntoViewCaseSimple from './ScrollIntoViewCaseSimple'; +import ScrollIntoViewTargetElement from './ScrollIntoViewTargetElement'; + +const React = window.React; +const {Fragment, useRef, useState, useEffect} = React; +const ReactDOM = window.ReactDOM; + +function Controls({ + alignToTop, + setAlignToTop, + scrollVertical, + exampleType, + setExampleType, +}) { + return ( +
+ +
+ +
+
+ +
+
+ ); +} + +export default function ScrollIntoViewCase() { + const [exampleType, setExampleType] = useState('simple'); + const [alignToTop, setAlignToTop] = useState(true); + const [caseInViewport, setCaseInViewport] = useState(false); + const fragmentRef = useRef(null); + const testCaseRef = useRef(null); + const noChildRef = useRef(null); + const scrollContainerRef = useRef(null); + + const scrollVertical = () => { + fragmentRef.current.experimental_scrollIntoView(alignToTop); + }; + + const scrollVerticalNoChildren = () => { + noChildRef.current.experimental_scrollIntoView(alignToTop); + }; + + useEffect(() => { + const observer = new IntersectionObserver(entries => { + entries.forEach(entry => { + if (entry.isIntersecting) { + setCaseInViewport(true); + } else { + setCaseInViewport(false); + } + }); + }); + testCaseRef.current.observeUsing(observer); + + const lastRef = testCaseRef.current; + return () => { + lastRef.unobserveUsing(observer); + observer.disconnect(); + }; + }); + + return ( + + + +
  • Toggle alignToTop and click the buttons to scroll
    • +     + +

    When the Fragment has children:

    +

    + In order to handle the case where children are split between + multiple scroll containers, we call scrollIntoView on each child in + reverse order. +

    +

    When the Fragment does not have children:

    +

    + The Fragment still represents a virtual space. We can scroll to the + nearest edge by selecting the host sibling before if + alignToTop=false, or after if alignToTop=true|undefined. We'll fall + back to the other sibling or parent in the case that the preferred + sibling target doesn't exist. +

    +     + + + + + {exampleType === 'simple' && ( + + + + )} + {exampleType === 'horizontal' && ( +
    + + + +
    + )} + {exampleType === 'multiple' && ( + +
    + + + + + )} + {exampleType === 'empty' && ( + + + + + + )} + + + + + + + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseComplex.js b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseComplex.js new file mode 100644 index 0000000000000..a0ea612d09c40 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseComplex.js @@ -0,0 +1,50 @@ +import ScrollIntoViewTargetElement from './ScrollIntoViewTargetElement'; + +const React = window.React; +const {Fragment, useRef, useState, useEffect} = React; +const ReactDOM = window.ReactDOM; + +export default function ScrollIntoViewCaseComplex({ + caseInViewport, + scrollContainerRef, +}) { + const [didMount, setDidMount] = useState(false); + // Hack to portal child into the scroll container + // after the first render. This is to simulate a case where + // an item is portaled into another scroll container. + useEffect(() => { + if (!didMount) { + setDidMount(true); + } + }, []); + return ( + + {caseInViewport && ( + + )} + {didMount && + ReactDOM.createPortal( + , + scrollContainerRef.current + )} + + + + {caseInViewport && ( + + )} + + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseSimple.js b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseSimple.js new file mode 100644 index 0000000000000..ee61cd16290f9 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewCaseSimple.js @@ -0,0 +1,14 @@ +import ScrollIntoViewTargetElement from './ScrollIntoViewTargetElement'; + +const React = window.React; +const {Fragment} = React; + +export default function ScrollIntoViewCaseSimple() { + return ( + + + + + + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewTargetElement.js b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewTargetElement.js new file mode 100644 index 0000000000000..f61668c5cf525 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/ScrollIntoViewTargetElement.js @@ -0,0 +1,18 @@ +const React = window.React; + +export default function ScrollIntoViewTargetElement({color, id, top}) { + return ( +
    + {id} +
    + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/index.js b/fixtures/dom/src/components/fixtures/fragment-refs/index.js index 23b440938cf7a..c560b59fbec6a 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/index.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/index.js @@ -5,6 +5,7 @@ import IntersectionObserverCase from './IntersectionObserverCase'; import ResizeObserverCase from './ResizeObserverCase'; import FocusCase from './FocusCase'; import GetClientRectsCase from './GetClientRectsCase'; +import ScrollIntoViewCase from './ScrollIntoViewCase'; const React = window.React; @@ -17,6 +18,7 @@ export default function FragmentRefsPage() { + ); } diff --git a/fixtures/dom/src/index.js b/fixtures/dom/src/index.js index 7a23ba2acf944..a334311be0c4b 100644 --- a/fixtures/dom/src/index.js +++ b/fixtures/dom/src/index.js @@ -2,14 +2,23 @@ import './polyfills'; import loadReact, {isLocal} from './react-loader'; if (isLocal()) { - Promise.all([import('react'), import('react-dom/client')]) - .then(([React, ReactDOMClient]) => { - if (React === undefined || ReactDOMClient === undefined) { + Promise.all([ + import('react'), + import('react-dom'), + import('react-dom/client'), + ]) + .then(([React, ReactDOM, ReactDOMClient]) => { + if ( + React === undefined || + ReactDOM === undefined || + ReactDOMClient === undefined + ) { throw new Error( 'Unable to load React. Build experimental and then run `yarn dev` again' ); } window.React = React; + window.ReactDOM = ReactDOM; window.ReactDOMClient = ReactDOMClient; }) .then(() => import('./components/App')) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index e3dac2e27e206..74b03d8fe0577 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -37,17 +37,6 @@ import {runWithFiberInDEV} from 'react-reconciler/src/ReactCurrentFiber'; import hasOwnProperty from 'shared/hasOwnProperty'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; -import { - isFiberContainedByFragment, - isFiberFollowing, - isFiberPreceding, - isFragmentContainedByFiber, - traverseFragmentInstance, - getFragmentParentHostFiber, - getInstanceFromHostFiber, - traverseFragmentInstanceDeeply, - fiberIsPortaledIntoHost, -} from 'react-reconciler/src/ReactFiberTreeReflection'; export { setCurrentUpdatePriority, @@ -69,6 +58,18 @@ import { markNodeAsHoistable, isOwnedInstance, } from './ReactDOMComponentTree'; +import { + traverseFragmentInstance, + getFragmentParentHostFiber, + getInstanceFromHostFiber, + isFiberFollowing, + isFiberPreceding, + getFragmentInstanceSiblings, + traverseFragmentInstanceDeeply, + fiberIsPortaledIntoHost, + isFiberContainedByFragment, + isFragmentContainedByFiber, +} from 'react-reconciler/src/ReactFiberTreeReflection'; import {compareDocumentPositionForEmptyFragment} from 'shared/ReactDOMFragmentRefShared'; export {detachDeletedInstance}; @@ -123,6 +124,7 @@ import { enableSrcObject, enableViewTransition, enableHydrationChangeEvent, + enableFragmentRefsScrollIntoView, } from 'shared/ReactFeatureFlags'; import { HostComponent, @@ -2813,6 +2815,7 @@ export type FragmentInstanceType = { composed: boolean, }): Document | ShadowRoot | FragmentInstanceType, compareDocumentPosition(otherNode: Instance): number, + scrollIntoView(alignToTop?: boolean): void, }; function FragmentInstance(this: FragmentInstanceType, fragmentFiber: Fiber) { @@ -2899,6 +2902,38 @@ function removeEventListenerFromChild( instance.removeEventListener(type, listener, optionsOrUseCapture); return false; } +function normalizeListenerOptions( + opts: ?EventListenerOptionsOrUseCapture, +): string { + if (opts == null) { + return '0'; + } + + if (typeof opts === 'boolean') { + return `c=${opts ? '1' : '0'}`; + } + + return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`; +} +function indexOfEventListener( + eventListeners: Array, + type: string, + listener: EventListener, + optionsOrUseCapture: void | EventListenerOptionsOrUseCapture, +): number { + for (let i = 0; i < eventListeners.length; i++) { + const item = eventListeners[i]; + if ( + item.type === type && + item.listener === listener && + normalizeListenerOptions(item.optionsOrUseCapture) === + normalizeListenerOptions(optionsOrUseCapture) + ) { + return i; + } + } + return -1; +} // $FlowFixMe[prop-missing] FragmentInstance.prototype.dispatchEvent = function ( this: FragmentInstanceType, @@ -3214,38 +3249,55 @@ function validateDocumentPositionWithFiberTree( return false; } -function normalizeListenerOptions( - opts: ?EventListenerOptionsOrUseCapture, -): string { - if (opts == null) { - return '0'; - } - - if (typeof opts === 'boolean') { - return `c=${opts ? '1' : '0'}`; - } - - return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`; -} +if (enableFragmentRefsScrollIntoView) { + // $FlowFixMe[prop-missing] + FragmentInstance.prototype.experimental_scrollIntoView = function ( + this: FragmentInstanceType, + alignToTop?: boolean, + ): void { + if (typeof alignToTop === 'object') { + throw new Error( + 'FragmentInstance.experimental_scrollIntoView() does not support ' + + 'scrollIntoViewOptions. Use the alignToTop boolean instead.', + ); + } + // First, get the children nodes + const children: Array = []; + traverseFragmentInstance(this._fragmentFiber, collectChildren, children); + + const resolvedAlignToTop = alignToTop !== false; + + // If there are no children, we can use the parent and siblings to determine a position + if (children.length === 0) { + const hostSiblings = getFragmentInstanceSiblings(this._fragmentFiber); + const targetFiber = resolvedAlignToTop + ? hostSiblings[1] || + hostSiblings[0] || + getFragmentParentHostFiber(this._fragmentFiber) + : hostSiblings[0] || hostSiblings[1]; + + if (targetFiber === null) { + if (__DEV__) { + console.warn( + 'You are attempting to scroll a FragmentInstance that has no ' + + 'children, siblings, or parent. No scroll was performed.', + ); + } + return; + } + const target = getInstanceFromHostFiber(targetFiber); + target.scrollIntoView(alignToTop); + return; + } -function indexOfEventListener( - eventListeners: Array, - type: string, - listener: EventListener, - optionsOrUseCapture: void | EventListenerOptionsOrUseCapture, -): number { - for (let i = 0; i < eventListeners.length; i++) { - const item = eventListeners[i]; - if ( - item.type === type && - item.listener === listener && - normalizeListenerOptions(item.optionsOrUseCapture) === - normalizeListenerOptions(optionsOrUseCapture) - ) { - return i; + let i = resolvedAlignToTop ? children.length - 1 : 0; + while (i !== (resolvedAlignToTop ? -1 : children.length)) { + const child = children[i]; + const instance = getInstanceFromHostFiber(child); + instance.scrollIntoView(alignToTop); + i += resolvedAlignToTop ? -1 : 1; } - } - return -1; + }; } export function createFragmentInstance( diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 521792d62e95f..35c10fe0f073e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -1836,4 +1836,323 @@ describe('FragmentRefs', () => { }); }); }); + + describe('scrollIntoView', () => { + function expectLast(arr, test) { + expect(arr[arr.length - 1]).toBe(test); + } + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView + it('does not yet support options', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + expect(() => { + fragmentRef.current.experimental_scrollIntoView({block: 'start'}); + }).toThrowError( + 'FragmentInstance.experimental_scrollIntoView() does not support ' + + 'scrollIntoViewOptions. Use the alignToTop boolean instead.', + ); + }); + + describe('with children', () => { + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView + it('settles scroll on the first child by default, or if alignToTop=true', async () => { + const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + +
    + A +
    +
    + B +
    +     , + ); + }); + + let logs = []; + childARef.current.scrollIntoView = jest.fn().mockImplementation(() => { + logs.push('childA'); + }); + childBRef.current.scrollIntoView = jest.fn().mockImplementation(() => { + logs.push('childB'); + }); + + // Default call + fragmentRef.current.experimental_scrollIntoView(); + expectLast(logs, 'childA'); + logs = []; + // alignToTop=true + fragmentRef.current.experimental_scrollIntoView(true); + expectLast(logs, 'childA'); + }); + + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView + it('calls scrollIntoView on the last child if alignToTop is false', async () => { + const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + +
    A
    +
    B
    +     , + ); + }); + + const logs = []; + childARef.current.scrollIntoView = jest.fn().mockImplementation(() => { + logs.push('childA'); + }); + childBRef.current.scrollIntoView = jest.fn().mockImplementation(() => { + logs.push('childB'); + }); + + fragmentRef.current.experimental_scrollIntoView(false); + expectLast(logs, 'childB'); + }); + + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView + it('handles portaled elements -- same scroll container', async () => { + const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( + + {createPortal( +
    + A +
    , + document.body, + )} + +
    + B +
    +     + ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + childARef.current.scrollIntoView = jest.fn().mockImplementation(() => { + logs.push('childA'); + }); + childBRef.current.scrollIntoView = jest.fn().mockImplementation(() => { + logs.push('childB'); + }); + + // Default call + fragmentRef.current.experimental_scrollIntoView(); + expectLast(logs, 'childA'); + }); + + // @gate enableFragmentRefs && enableFragmentRefsScrollIntoView + it('handles portaled elements -- different scroll container', async () => { + const fragmentRef = React.createRef(); + const headerChildRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const childCRef = React.createRef(); + const scrollContainerRef = React.createRef(); + const scrollContainerNestedRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test({mountFragment}) { + return ( + <> +