From 53c9f81049b4440a02b5ed3edb128516821c0279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?YongSeok=20Jang=20=28=EC=9E=A5=EC=9A=A9=EC=84=9D=29?= Date: Wed, 7 May 2025 23:48:17 +0900 Subject: [PATCH 1/7] [DevTools] Use Popover API for TraceUpdates highlighting (#32614) ## Summary When using React DevTools to highlight component updates, the highlights would sometimes appear behind elements that use the browser's [top-layer](https://developer.mozilla.org/en-US/docs/Glossary/Top_layer) (such as `` elements or components using the Popover API). This made it difficult to see which components were updating when they were inside or behind top-layer elements. This PR fixes the issue by using the Popover API to ensure that highlighting appears on top of all content, including elements in the top-layer. The implementation maintains backward compatibility with browsers that don't support the Popover API. ## How did you test this change? I tested this change in the following ways: 1. Manually tested in Chrome (which supports the Popover API) with: - Created a test application with React components inside `` elements and custom elements using the Popover API - Verified that component highlighting appears above these elements when they update - Confirmed that highlighting displays correctly for nested components within top-layer elements 2. Verified backward compatibility: - Tested in browsers without Popover API support to ensure fallback behavior works correctly - Confirmed that no errors occur and highlighting still functions as before 3. Ran the React DevTools test suite: - All tests pass successfully - No regressions were introduced [demo-page](https://devtools-toplayer-demo.vercel.app/) [demo-repo](https://github.com/yongsk0066/devtools-toplayer-demo) ### AS-IS https://github.com/user-attachments/assets/dc2e1281-969f-4f61-82c3-480153916969 ### TO-BE https://github.com/user-attachments/assets/dd52ce35-816c-42f0-819b-0d5d0a8a21e5 --- .../chrome/manifest.json | 2 +- .../edge/manifest.json | 2 +- .../src/backend/views/TraceUpdates/canvas.js | 34 +++++++- .../src/app/TraceUpdatesTest/index.js | 87 +++++++++++++++++++ .../react-devtools-shell/src/app/index.js | 2 + 5 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 packages/react-devtools-shell/src/app/TraceUpdatesTest/index.js diff --git a/packages/react-devtools-extensions/chrome/manifest.json b/packages/react-devtools-extensions/chrome/manifest.json index 5cf6397bc75ef..a773bdcaa25f9 100644 --- a/packages/react-devtools-extensions/chrome/manifest.json +++ b/packages/react-devtools-extensions/chrome/manifest.json @@ -4,7 +4,7 @@ "description": "Adds React debugging tools to the Chrome Developer Tools.", "version": "6.1.1", "version_name": "6.1.1", - "minimum_chrome_version": "102", + "minimum_chrome_version": "114", "icons": { "16": "icons/16-production.png", "32": "icons/32-production.png", diff --git a/packages/react-devtools-extensions/edge/manifest.json b/packages/react-devtools-extensions/edge/manifest.json index 512dd888f7d94..333f788e90e61 100644 --- a/packages/react-devtools-extensions/edge/manifest.json +++ b/packages/react-devtools-extensions/edge/manifest.json @@ -4,7 +4,7 @@ "description": "Adds React debugging tools to the Microsoft Edge Developer Tools.", "version": "6.1.1", "version_name": "6.1.1", - "minimum_chrome_version": "102", + "minimum_chrome_version": "114", "icons": { "16": "icons/16-production.png", "32": "icons/32-production.png", diff --git a/packages/react-devtools-shared/src/backend/views/TraceUpdates/canvas.js b/packages/react-devtools-shared/src/backend/views/TraceUpdates/canvas.js index 3e01397546721..b9e2cd9068135 100644 --- a/packages/react-devtools-shared/src/backend/views/TraceUpdates/canvas.js +++ b/packages/react-devtools-shared/src/backend/views/TraceUpdates/canvas.js @@ -65,6 +65,24 @@ function drawWeb(nodeToData: Map) { drawGroupBorders(context, group); drawGroupLabel(context, group); }); + + if (canvas !== null) { + if (nodeToData.size === 0 && canvas.matches(':popover-open')) { + // $FlowFixMe[prop-missing]: Flow doesn't recognize Popover API + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API + canvas.hidePopover(); + return; + } + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API + if (canvas.matches(':popover-open')) { + // $FlowFixMe[prop-missing]: Flow doesn't recognize Popover API + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API + canvas.hidePopover(); + } + // $FlowFixMe[prop-missing]: Flow doesn't recognize Popover API + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API + canvas.showPopover(); + } } type GroupItem = { @@ -191,7 +209,15 @@ function destroyNative(agent: Agent) { function destroyWeb() { if (canvas !== null) { + if (canvas.matches(':popover-open')) { + // $FlowFixMe[prop-missing]: Flow doesn't recognize Popover API + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API + canvas.hidePopover(); + } + + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API and loses canvas nullability tracking if (canvas.parentNode != null) { + // $FlowFixMe[incompatible-call]: Flow doesn't track that canvas is non-null here canvas.parentNode.removeChild(canvas); } canvas = null; @@ -204,6 +230,9 @@ export function destroy(agent: Agent): void { function initialize(): void { canvas = window.document.createElement('canvas'); + canvas.setAttribute('popover', 'manual'); + + // $FlowFixMe[incompatible-use]: Flow doesn't recognize Popover API canvas.style.cssText = ` xx-background-color: red; xx-opacity: 0.5; @@ -213,7 +242,10 @@ function initialize(): void { position: fixed; right: 0; top: 0; - z-index: 1000000000; + background-color: transparent; + outline: none; + box-shadow: none; + border: none; `; const root = window.document.documentElement; diff --git a/packages/react-devtools-shell/src/app/TraceUpdatesTest/index.js b/packages/react-devtools-shell/src/app/TraceUpdatesTest/index.js new file mode 100644 index 0000000000000..dd847ad7aef45 --- /dev/null +++ b/packages/react-devtools-shell/src/app/TraceUpdatesTest/index.js @@ -0,0 +1,87 @@ +/** + * 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. + * + * @flow + */ + +import * as React from 'react'; +import {useRef, useState} from 'react'; + +const Counter = () => { + const [count, setCount] = useState(0); + + return ( +
+

Count: {count}

+ +
+ ); +}; + +function DialogComponent() { + const dialogRef = useRef(null); + + const openDialog = () => { + if (dialogRef.current) { + dialogRef.current.showModal(); + } + }; + + const closeDialog = () => { + if (dialogRef.current) { + dialogRef.current.close(); + } + }; + + return ( +
+ + +

Dialog Content

+ + + +
+ ); +} + +function RegularComponent() { + return ( +
+

Regular Component

+ +
+ ); +} + +export default function TraceUpdatesTest(): React.Node { + return ( +
+

TraceUpdates Test

+ +
+

Standard Component

+ +
+ +
+

Dialog Component (top-layer element)

+ +
+ +
+

How to Test:

+
    +
  1. Open DevTools Components panel
    2. +
  2. Enable "Highlight updates when components render" in settings
    3. +
  3. Click increment buttons and observe highlights
    4. +
  4. Open the dialog and test increments there as well
    5. +
+
+
+ ); +} diff --git a/packages/react-devtools-shell/src/app/index.js b/packages/react-devtools-shell/src/app/index.js index 11f13bec47b22..8de2f949bb744 100644 --- a/packages/react-devtools-shell/src/app/index.js +++ b/packages/react-devtools-shell/src/app/index.js @@ -19,6 +19,7 @@ import Toggle from './Toggle'; import ErrorBoundaries from './ErrorBoundaries'; import PartiallyStrictApp from './PartiallyStrictApp'; import SuspenseTree from './SuspenseTree'; +import TraceUpdatesTest from './TraceUpdatesTest'; import {ignoreErrors, ignoreLogs, ignoreWarnings} from './console'; import './styles.css'; @@ -112,6 +113,7 @@ function mountTestApp() { mountApp(SuspenseTree); mountApp(DeeplyNestedComponents); mountApp(Iframe); + mountApp(TraceUpdatesTest); if (shouldRenderLegacy) { mountLegacyApp(PartiallyStrictApp); From 0ff1d13b8055c801d8b9b6779958c09fd0dc63e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 7 May 2025 11:43:37 -0400 Subject: [PATCH 2/7] [Flight] Parse Stack Trace from Structured CallSite if available (#33135) This is first step to include more enclosing line/column in the parsed data. We install our own `prepareStackTrace` to collect structured callsite data and only fall back to parsing the string if it was already evaluated or if `prepareStackTrace` doesn't work in this environment. We still mirror the default V8 format for encoding the function name part. A lot of this is covered by tests already. --- .../react-server/src/ReactFlightServer.js | 17 +++ .../src/ReactFlightStackConfigV8.js | 138 ++++++++++++++++-- 2 files changed, 139 insertions(+), 16 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 98b17cc92088e..dd946280f9395 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -152,11 +152,27 @@ function defaultFilterStackFrame( ); } +// DEV-only cache of parsed and filtered stack frames. +const stackTraceCache: WeakMap = __DEV__ + ? new WeakMap() + : (null: any); + function filterStackTrace( request: Request, error: Error, skipFrames: number, ): ReactStackTrace { + const existing = stackTraceCache.get(error); + if (existing !== undefined) { + // Return a clone because the Flight protocol isn't yet resilient to deduping + // objects in the debug info. TODO: Support deduping stacks. + const clone = existing.slice(0); + for (let i = 0; i < clone.length; i++) { + // $FlowFixMe[invalid-tuple-arity] + clone[i] = clone[i].slice(0); + } + return clone; + } // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly // to save bandwidth even in DEV. We'll also replay these stacks on the client so by // stripping them early we avoid that overhead. Otherwise we'd normally just rely on @@ -183,6 +199,7 @@ function filterStackTrace( i--; } } + stackTraceCache.set(error, stack); return stack; } diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js index 981e62dbdbb5f..d2f4633cec67a 100644 --- a/packages/react-server/src/ReactFlightStackConfigV8.js +++ b/packages/react-server/src/ReactFlightStackConfigV8.js @@ -9,23 +9,104 @@ import type {ReactStackTrace} from 'shared/ReactTypes'; -import DefaultPrepareStackTrace from 'shared/DefaultPrepareStackTrace'; +let framesToSkip: number = 0; +let collectedStackTrace: null | ReactStackTrace = null; -function getStack(error: Error): string { - // We override Error.prepareStackTrace with our own version that normalizes - // the stack to V8 formatting even if the server uses other formatting. - // It also ensures that source maps are NOT applied to this since that can - // be slow we're better off doing that lazily from the client instead of - // eagerly on the server. If the stack has already been read, then we might - // not get a normalized stack and it might still have been source mapped. - const previousPrepare = Error.prepareStackTrace; - Error.prepareStackTrace = DefaultPrepareStackTrace; - try { - // eslint-disable-next-line react-internal/safe-string-coercion - return String(error.stack); - } finally { - Error.prepareStackTrace = previousPrepare; +const identifierRegExp = /^[a-zA-Z_$][0-9a-zA-Z_$]*$/; + +function getMethodCallName(callSite: CallSite): string { + const typeName = callSite.getTypeName(); + const methodName = callSite.getMethodName(); + const functionName = callSite.getFunctionName(); + let result = ''; + if (functionName) { + if ( + typeName && + identifierRegExp.test(functionName) && + functionName !== typeName + ) { + result += typeName + '.'; + } + result += functionName; + if ( + methodName && + functionName !== methodName && + !functionName.endsWith('.' + methodName) && + !functionName.endsWith(' ' + methodName) + ) { + result += ' [as ' + methodName + ']'; + } + } else { + if (typeName) { + result += typeName + '.'; + } + if (methodName) { + result += methodName; + } else { + result += ''; + } + } + return result; +} + +function collectStackTrace( + error: Error, + structuredStackTrace: CallSite[], +): string { + const result: ReactStackTrace = []; + // Collect structured stack traces from the callsites. + // We mirror how V8 serializes stack frames and how we later parse them. + for (let i = framesToSkip; i < structuredStackTrace.length; i++) { + const callSite = structuredStackTrace[i]; + let name = callSite.getFunctionName() || ''; + if (name === 'react-stack-bottom-frame') { + // Skip everything after the bottom frame since it'll be internals. + break; + } else if (callSite.isNative()) { + result.push([name, '', 0, 0]); + } else { + // We encode complex function calls as if they're part of the function + // name since we cannot simulate the complex ones and they look the same + // as function names in UIs on the client as well as stacks. + if (callSite.isConstructor()) { + name = 'new ' + name; + } else if (!callSite.isToplevel()) { + name = getMethodCallName(callSite); + } + if (name === '') { + name = ''; + } + let filename = callSite.getScriptNameOrSourceURL() || ''; + if (filename === '') { + filename = ''; + } + if (callSite.isEval() && !filename) { + const origin = callSite.getEvalOrigin(); + if (origin) { + filename = origin.toString() + ', '; + } + } + const line = callSite.getLineNumber() || 0; + const col = callSite.getColumnNumber() || 0; + result.push([name, filename, line, col]); + } } + // At the same time we generate a string stack trace just in case someone + // else reads it. Ideally, we'd call the previous prepareStackTrace to + // ensure it's in the expected format but it's common for that to be + // source mapped and since we do a lot of eager parsing of errors, it + // would be slow in those environments. We could maybe just rely on those + // environments having to disable source mapping globally to speed things up. + // For now, we just generate a default V8 formatted stack trace without + // source mapping as a fallback. + const name = error.name || 'Error'; + const message = error.message || ''; + let stack = name + ': ' + message; + for (let i = 0; i < structuredStackTrace.length; i++) { + stack += '\n at ' + structuredStackTrace[i].toString(); + } + collectedStackTrace = result; + return stack; } // This matches either of these V8 formats. @@ -39,7 +120,32 @@ export function parseStackTrace( error: Error, skipFrames: number, ): ReactStackTrace { - let stack = getStack(error); + // We override Error.prepareStackTrace with our own version that collects + // the structured data. We need more information than the raw stack gives us + // and we need to ensure that we don't get the source mapped version. + collectedStackTrace = null; + framesToSkip = skipFrames; + const previousPrepare = Error.prepareStackTrace; + Error.prepareStackTrace = collectStackTrace; + let stack; + try { + // eslint-disable-next-line react-internal/safe-string-coercion + stack = String(error.stack); + } finally { + Error.prepareStackTrace = previousPrepare; + } + + if (collectedStackTrace !== null) { + const result = collectedStackTrace; + collectedStackTrace = null; + return result; + } + + // If the stack has already been read, or this is not actually a V8 compatible + // engine then we might not get a normalized stack and it might still have been + // source mapped. Regardless we try our best to parse it. This works best if the + // environment just uses default V8 formatting and no source mapping. + if (stack.startsWith('Error: react-stack-top-frame\n')) { // V8's default formatting prefixes with the error message which we // don't want/need. From 4a702865dd0c5849c1b454091560c3ef26121611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 7 May 2025 12:34:55 -0400 Subject: [PATCH 3/7] [Flight] Encode enclosing line/column numbers and use it to align the fake function (#33136) Stacked on #33135. This encodes the line/column of the enclosing function as part of the stack traces. When that information is available. I adjusted the fake function code generation so that the beginning of the arrow function aligns with these as much as possible. This ensures that when the browser tries to look up the line/column of the enclosing function, such as for getting the function name, it gets the right one. If we can't get the enclosing line/column, then we encode it at the beginning of the file. This is likely to get a miss in the source map identifiers, which means that the function name gets extracted from the runtime name instead which is better. Another thing where this is used is the in the Performance Track. Ideally that would be fixed by https://issues.chromium.org/u/1/issues/415968771 but the enclosing information is useful for other things like the function name resolution anyway. We can also use this for the "View source for this element" in React DevTools. --- .../react-client/src/ReactFlightClient.js | 104 ++++++++++++++++-- .../src/ReactFlightReplyClient.js | 3 +- .../src/ReactFlightStackConfigV8.js | 16 ++- packages/shared/ReactTypes.js | 2 + 4 files changed, 110 insertions(+), 15 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 3234814952fad..fbf195ba7b65a 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -2226,6 +2226,8 @@ function createFakeFunction( sourceMap: null | string, line: number, col: number, + enclosingLine: number, + enclosingCol: number, environmentName: string, ): FakeFunction { // This creates a fake copy of a Server Module. It represents a module that has already @@ -2243,26 +2245,104 @@ function createFakeFunction( // This allows us to use the original source map as the source map of this fake file to // point to the original source. let code; - if (line <= 1) { - const minSize = encodedName.length + 7; + // Normalize line/col to zero based. + if (enclosingLine < 1) { + enclosingLine = 0; + } else { + enclosingLine--; + } + if (enclosingCol < 1) { + enclosingCol = 0; + } else { + enclosingCol--; + } + if (line < 1) { + line = 0; + } else { + line--; + } + if (col < 1) { + col = 0; + } else { + col--; + } + if (line < enclosingLine || (line === enclosingLine && col < enclosingCol)) { + // Protection against invalid enclosing information. Should not happen. + enclosingLine = 0; + enclosingCol = 0; + } + if (line < 1) { + // Fit everything on the first line. + const minCol = encodedName.length + 3; + let enclosingColDistance = enclosingCol - minCol; + if (enclosingColDistance < 0) { + enclosingColDistance = 0; + } + let colDistance = col - enclosingColDistance - minCol - 3; + if (colDistance < 0) { + colDistance = 0; + } code = '({' + encodedName + - ':_=>' + - ' '.repeat(col < minSize ? 0 : col - minSize) + - '_()})\n' + - comment; + ':' + + ' '.repeat(enclosingColDistance) + + '_=>' + + ' '.repeat(colDistance) + + '_()})'; + } else if (enclosingLine < 1) { + // Fit just the enclosing function on the first line. + const minCol = encodedName.length + 3; + let enclosingColDistance = enclosingCol - minCol; + if (enclosingColDistance < 0) { + enclosingColDistance = 0; + } + code = + '({' + + encodedName + + ':' + + ' '.repeat(enclosingColDistance) + + '_=>' + + '\n'.repeat(line - enclosingLine) + + ' '.repeat(col) + + '_()})'; + } else if (enclosingLine === line) { + // Fit the enclosing function and callsite on same line. + let colDistance = col - enclosingCol - 3; + if (colDistance < 0) { + colDistance = 0; + } + code = + '\n'.repeat(enclosingLine - 1) + + '({' + + encodedName + + ':\n' + + ' '.repeat(enclosingCol) + + '_=>' + + ' '.repeat(colDistance) + + '_()})'; } else { + // This is the ideal because we can always encode any position. code = - comment + - '\n'.repeat(line - 2) + + '\n'.repeat(enclosingLine - 1) + '({' + encodedName + - ':_=>\n' + - ' '.repeat(col < 1 ? 0 : col - 1) + + ':\n' + + ' '.repeat(enclosingCol) + + '_=>' + + '\n'.repeat(line - enclosingLine) + + ' '.repeat(col) + '_()})'; } + if (enclosingLine < 1) { + // If the function starts at the first line, we append the comment after. + code = code + '\n' + comment; + } else { + // Otherwise we prepend the comment on the first line. + code = comment + code; + } + if (filename.startsWith('/')) { // If the filename starts with `/` we assume that it is a file system file // rather than relative to the current host. Since on the server fully qualified @@ -2320,7 +2400,7 @@ function buildFakeCallStack( const frameKey = frame.join('-') + '-' + environmentName; let fn = fakeFunctionCache.get(frameKey); if (fn === undefined) { - const [name, filename, line, col] = frame; + const [name, filename, line, col, enclosingLine, enclosingCol] = frame; const findSourceMapURL = response._debugFindSourceMapURL; const sourceMap = findSourceMapURL ? findSourceMapURL(filename, environmentName) @@ -2331,6 +2411,8 @@ function buildFakeCallStack( sourceMap, line, col, + enclosingLine, + enclosingCol, environmentName, ); // TODO: This cache should technically live on the response since the _debugFindSourceMapURL diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index e3c4ec20ba1ab..cf21d6e9adf13 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -1350,10 +1350,11 @@ function parseStackLocation(error: Error): null | ReactCallSite { if (filename === '') { filename = ''; } + // This is really the enclosingLine/Column. const line = +(parsed[3] || parsed[6]); const col = +(parsed[4] || parsed[7]); - return [name, filename, line, col]; + return [name, filename, line, col, line, col]; } export function createServerReference, T>( diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js index d2f4633cec67a..25bc2aba9de8b 100644 --- a/packages/react-server/src/ReactFlightStackConfigV8.js +++ b/packages/react-server/src/ReactFlightStackConfigV8.js @@ -63,7 +63,7 @@ function collectStackTrace( // Skip everything after the bottom frame since it'll be internals. break; } else if (callSite.isNative()) { - result.push([name, '', 0, 0]); + result.push([name, '', 0, 0, 0, 0]); } else { // We encode complex function calls as if they're part of the function // name since we cannot simulate the complex ones and they look the same @@ -88,7 +88,17 @@ function collectStackTrace( } const line = callSite.getLineNumber() || 0; const col = callSite.getColumnNumber() || 0; - result.push([name, filename, line, col]); + const enclosingLine: number = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingLineNumber === 'function' + ? (callSite: any).getEnclosingLineNumber() || 0 + : 0; + const enclosingCol: number = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingColumnNumber === 'function' + ? (callSite: any).getEnclosingColumnNumber() || 0 + : 0; + result.push([name, filename, line, col, enclosingLine, enclosingCol]); } } // At the same time we generate a string stack trace just in case someone @@ -179,7 +189,7 @@ export function parseStackTrace( } const line = +(parsed[3] || parsed[6]); const col = +(parsed[4] || parsed[7]); - parsedFrames.push([name, filename, line, col]); + parsedFrames.push([name, filename, line, col, 0, 0]); } return parsedFrames; } diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index e199fa9e7b0b7..2172c92bfc90a 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -186,6 +186,8 @@ export type ReactCallSite = [ string, // file name TODO: model nested eval locations as nested arrays number, // line number number, // column number + number, // enclosing line number + number, // enclosing column number ]; export type ReactStackTrace = Array; From 4206fe49825787eda57a5d142640a63772ccbf2b Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 7 May 2025 12:47:28 -0400 Subject: [PATCH 4/7] Allow fragment refs to attempt focus/focusLast on nested host children (#33058) This enables `focus` and `focusLast` methods on FragmentInstances to search nested host components, depth first. Attempts focus on each child and bails if one is successful. Previously, only the first level of host children would attempt focus. Now if we have an example like ``` component MenuItem() { return () } component Menu() { return {items.map(i => )} } ``` We can target focus on the first or last a tag, rather than checking each wrapping div and then noop. --- .../fixtures/fragment-refs/FocusCase.js | 13 +++-- fixtures/dom/src/style.css | 4 ++ .../src/client/ReactFiberConfigDOM.js | 9 +++- .../__tests__/ReactDOMFragmentRefs-test.js | 54 +++++++++++++++++++ .../src/ReactFiberTreeReflection.js | 31 +++++------ 5 files changed, 88 insertions(+), 23 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js index 888107c9e07c7..baff30895c0e0 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/FocusCase.js @@ -43,11 +43,18 @@ export default function FocusCase() {
-
Unfocusable div
- +
+

Unfocusable div

+
+
+

Unfocusable div with nested focusable button

+ +
-
Unfocusable div
+
+

Unfocusable div

+
diff --git a/fixtures/dom/src/style.css b/fixtures/dom/src/style.css index 66fda7afe0cac..e507014d6829d 100644 --- a/fixtures/dom/src/style.css +++ b/fixtures/dom/src/style.css @@ -365,6 +365,10 @@ tbody tr:nth-child(even) { background-color: green; } +.highlight-focused-children * { + margin-left: 10px; +} + .highlight-focused-children *:focus { outline: 2px solid green; } diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 8584b644eff9d..1eabf7a19a90b 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -68,6 +68,7 @@ import { getFragmentParentHostFiber, getNextSiblingHostFiber, getInstanceFromHostFiber, + traverseFragmentInstanceDeeply, } from 'react-reconciler/src/ReactFiberTreeReflection'; export {detachDeletedInstance}; @@ -2698,7 +2699,7 @@ FragmentInstance.prototype.focus = function ( this: FragmentInstanceType, focusOptions?: FocusOptions, ): void { - traverseFragmentInstance( + traverseFragmentInstanceDeeply( this._fragmentFiber, setFocusOnFiberIfFocusable, focusOptions, @@ -2717,7 +2718,11 @@ FragmentInstance.prototype.focusLast = function ( focusOptions?: FocusOptions, ): void { const children: Array = []; - traverseFragmentInstance(this._fragmentFiber, collectChildren, children); + traverseFragmentInstanceDeeply( + this._fragmentFiber, + collectChildren, + children, + ); for (let i = children.length - 1; i >= 0; i--) { const child = children[i]; if (setFocusOnFiberIfFocusable(child, focusOptions)) { diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 50447e1eac677..6eb1fb3a0f346 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -145,6 +145,32 @@ describe('FragmentRefs', () => { document.activeElement.blur(); }); + // @gate enableFragmentRefs + it('focuses deeply nested focusable children, depth first', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( + + + + + ); + } + await act(() => { + root.render(); + }); + await act(() => { + fragmentRef.current.focus(); + }); + expect(document.activeElement.id).toEqual('grandchild-a'); + }); + // @gate enableFragmentRefs it('preserves document order when adding and removing children', async () => { const fragmentRef = React.createRef(); @@ -228,6 +254,34 @@ describe('FragmentRefs', () => { expect(document.activeElement.id).toEqual('child-c'); document.activeElement.blur(); }); + + // @gate enableFragmentRefs + it('focuses deeply nested focusable children, depth first', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( + + + + + ); + } + await act(() => { + root.render(); + }); + await act(() => { + fragmentRef.current.focusLast(); + }); + expect(document.activeElement.id).toEqual('grandchild-b'); + }); }); describe('blur()', () => { diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index d032d3247e475..45da707dfea23 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -354,6 +354,16 @@ export function traverseFragmentInstance( traverseVisibleHostChildren(fragmentFiber.child, false, fn, a, b, c); } +export function traverseFragmentInstanceDeeply( + fragmentFiber: Fiber, + fn: (Fiber, A, B, C) => boolean, + a: A, + b: B, + c: C, +): void { + traverseVisibleHostChildren(fragmentFiber.child, true, fn, a, b, c); +} + function traverseVisibleHostChildren( child: Fiber | null, searchWithinHosts: boolean, @@ -363,24 +373,8 @@ function traverseVisibleHostChildren( c: C, ): boolean { while (child !== null) { - if (child.tag === HostComponent) { - if (fn(child, a, b, c)) { - return true; - } - if (searchWithinHosts) { - if ( - traverseVisibleHostChildren( - child.child, - searchWithinHosts, - fn, - a, - b, - c, - ) - ) { - return true; - } - } + if (child.tag === HostComponent && fn(child, a, b, c)) { + return true; } else if ( child.tag === OffscreenComponent && child.memoizedState !== null @@ -388,6 +382,7 @@ function traverseVisibleHostChildren( // Skip hidden subtrees } else { if ( + (searchWithinHosts || child.tag !== HostComponent) && traverseVisibleHostChildren(child.child, searchWithinHosts, fn, a, b, c) ) { return true; From a437c99ff7a45025367571363653c2ad5db482a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 7 May 2025 13:02:41 -0400 Subject: [PATCH 5/7] [Flight] Clarify that location field is a FunctionLocation not a CallSite (#33141) Follow up to #33136. This clarifies in the types where the conversion happens from a CallSite which we use to simulate getting the enclosing line/col to a FunctionLocation which doesn't represent a CallSite but actually just the function which only has an enclosing line/col. --- packages/react-client/src/ReactFlightClient.js | 4 ++-- .../react-client/src/ReactFlightReplyClient.js | 8 ++++---- packages/react-server/src/ReactFlightServer.js | 14 ++++++++++---- packages/shared/ReactTypes.js | 7 +++++++ 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index fbf195ba7b65a..7d6bbd5c1fbb0 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -15,7 +15,7 @@ import type { ReactAsyncInfo, ReactTimeInfo, ReactStackTrace, - ReactCallSite, + ReactFunctionLocation, ReactErrorInfoDev, } from 'shared/ReactTypes'; import type {LazyComponent} from 'react/src/ReactLazy'; @@ -1074,7 +1074,7 @@ function loadServerReference, T>( bound: null | Thenable>, name?: string, // DEV-only env?: string, // DEV-only - location?: ReactCallSite, // DEV-only + location?: ReactFunctionLocation, // DEV-only }, parentObject: Object, key: string, diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index cf21d6e9adf13..6a0a37b787d34 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -13,7 +13,7 @@ import type { FulfilledThenable, RejectedThenable, ReactCustomFormAction, - ReactCallSite, + ReactFunctionLocation, } from 'shared/ReactTypes'; import type {LazyComponent} from 'react/src/ReactLazy'; import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences'; @@ -1248,7 +1248,7 @@ export function createBoundServerReference, T>( bound: null | Thenable>, name?: string, // DEV-only env?: string, // DEV-only - location?: ReactCallSite, // DEV-only + location?: ReactFunctionLocation, // DEV-only }, callServer: CallServerCallback, encodeFormAction?: EncodeFormActionCallback, @@ -1309,7 +1309,7 @@ const v8FrameRegExp = // filename:0:0 const jscSpiderMonkeyFrameRegExp = /(?:(.*)@)?(.*):(\d+):(\d+)/; -function parseStackLocation(error: Error): null | ReactCallSite { +function parseStackLocation(error: Error): null | ReactFunctionLocation { // This parsing is special in that we know that the calling function will always // be a module that initializes the server action. We also need this part to work // cross-browser so not worth a Config. It's DEV only so not super code size @@ -1354,7 +1354,7 @@ function parseStackLocation(error: Error): null | ReactCallSite { const line = +(parsed[3] || parsed[6]); const col = +(parsed[4] || parsed[7]); - return [name, filename, line, col, line, col]; + return [name, filename, line, col]; } export function createServerReference, T>( diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index dd946280f9395..ed6d9c443b7bc 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -62,7 +62,7 @@ import type { ReactAsyncInfo, ReactTimeInfo, ReactStackTrace, - ReactCallSite, + ReactFunctionLocation, ReactErrorInfo, ReactErrorInfoDev, } from 'shared/ReactTypes'; @@ -2089,7 +2089,7 @@ function serializeServerReference( const bound = boundArgs === null ? null : Promise.resolve(boundArgs); const id = getServerReferenceId(request.bundlerConfig, serverReference); - let location: null | ReactCallSite = null; + let location: null | ReactFunctionLocation = null; if (__DEV__) { const error = getServerReferenceLocation( request.bundlerConfig, @@ -2098,7 +2098,13 @@ function serializeServerReference( if (error) { const frames = parseStackTrace(error, 1); if (frames.length > 0) { - location = frames[0]; + const firstFrame = frames[0]; + location = [ + firstFrame[0], + firstFrame[1], + firstFrame[2], // The line and col of the callsite represents the + firstFrame[3], // enclosing line and col of the function. + ]; } } } @@ -2108,7 +2114,7 @@ function serializeServerReference( bound: null | Promise>, name?: string, // DEV-only env?: string, // DEV-only - location?: ReactCallSite, // DEV-only + location?: ReactFunctionLocation, // DEV-only } = __DEV__ && location !== null ? { diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 2172c92bfc90a..4c8ca77bb160b 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -192,6 +192,13 @@ export type ReactCallSite = [ export type ReactStackTrace = Array; +export type ReactFunctionLocation = [ + string, // function name + string, // file name TODO: model nested eval locations as nested arrays + number, // enclosing line number + number, // enclosing column number +]; + export type ReactComponentInfo = { +name: string, +env?: string, From 946da518eb2d64d808f9204a72e05892d3005f3f Mon Sep 17 00:00:00 2001 From: Niklas Mollenhauer Date: Wed, 7 May 2025 19:15:11 +0200 Subject: [PATCH 6/7] feat(compiler): implement constant folding for unary minus (#33140) ## Summary `-constant` is represented as a `UnaryExpression` node that is currently not part of constant folding. If the operand is a constant number, the node is folded to `constant * -1`. This also coerces `-0` to `0`, resulting in `0 === -0` being folded to `true`. ## How did you test this change? See attached tests --- .../src/Optimization/ConstantPropagation.ts | 17 +++++ ...onstant-propagation-unary-number.expect.md | 74 +++++++++++++++++++ .../constant-propagation-unary-number.js | 25 +++++++ .../constant-propagation-unary.expect.md | 2 +- 4 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts index 07cd419230ac4..05f4ef1ae7ffd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts @@ -327,6 +327,23 @@ function evaluateInstruction( } return null; } + case '-': { + const operand = read(constants, value.value); + if ( + operand !== null && + operand.kind === 'Primitive' && + typeof operand.value === 'number' + ) { + const result: Primitive = { + kind: 'Primitive', + value: operand.value * -1, + loc: value.loc, + }; + instr.value = result; + return result; + } + return null; + } default: return null; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.expect.md new file mode 100644 index 0000000000000..074c3214810bd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.expect.md @@ -0,0 +1,74 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +function foo() { + const a = -1; + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +function foo() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = ( + + ); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; + +``` + +### Eval output +(kind: ok)
{"value":[-2,0,true,null,null,null,null,null]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.js new file mode 100644 index 0000000000000..8ef8ec0e6c79a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary-number.js @@ -0,0 +1,25 @@ +import {Stringify} from 'shared-runtime'; + +function foo() { + const a = -1; + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], + isComponent: false, +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md index 3ba5ea6bb9553..aaea767643b06 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/constant-propagation-unary.expect.md @@ -58,7 +58,7 @@ function foo() { n0: true, n1: false, n2: false, - n3: !-1, + n3: false, s0: true, s1: false, s2: false, From 8a8df5dbdd57bf63d5156c1a9cba21ac6106b83d Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 7 May 2025 14:00:59 -0400 Subject: [PATCH 7/7] Add dispatchEvent to fragment instances (#32813) `fragmentInstance.dispatchEvent(evt)` calls `element.dispatchEvent(evt)` on the fragment's host parent. This mimics bubbling if the `fragmentInstance` could receive an event itself. If the parent is disconnected, there is a dev warning and no event is dispatched. --- .../fragment-refs/EventDispatchCase.js | 157 ++++ .../fixtures/fragment-refs/index.js | 2 + .../src/client/ReactFiberConfigDOM.js | 38 + .../__tests__/ReactDOMFragmentRefs-test.js | 841 ++++++++++-------- 4 files changed, 673 insertions(+), 365 deletions(-) create mode 100644 fixtures/dom/src/components/fixtures/fragment-refs/EventDispatchCase.js diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/EventDispatchCase.js b/fixtures/dom/src/components/fixtures/fragment-refs/EventDispatchCase.js new file mode 100644 index 0000000000000..ef5a959021332 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/fragment-refs/EventDispatchCase.js @@ -0,0 +1,157 @@ +import TestCase from '../../TestCase'; +import Fixture from '../../Fixture'; + +const React = window.React; +const {Fragment, useRef, useState} = React; + +function WrapperComponent(props) { + return props.children; +} + +const initialState = { + child: false, + parent: false, + grandparent: false, +}; + +export default function EventListenerCase() { + const fragmentRef = useRef(null); + const [clickedState, setClickedState] = useState({...initialState}); + const [fragmentEventFired, setFragmentEventFired] = useState(false); + const [bubblesState, setBubblesState] = useState(true); + + function setClick(id) { + setClickedState(prev => ({...prev, [id]: true})); + } + + function fragmentClickHandler(e) { + setFragmentEventFired(true); + } + + return ( + + +
  • + Each box has regular click handlers, you can click each one to observe + the status changing through standard bubbling. +
    • +
  • Clear the clicked state
    • +
  • + Click the "Dispatch click event" button to dispatch a click event on + the Fragment. The event will be dispatched on the Fragment's parent, + so the child will not change state. +
    • +
  • + Click the "Add event listener" button to add a click event listener on + the Fragment. This registers a handler that will turn the child blue + on click. +
    • +
  • + Now click the "Dispatch click event" button again. You can see that it + will fire the Fragment's event handler in addition to bubbling the + click from the parent. +
    • +
  • + If you turn off bubbling, only the Fragment's event handler will be + called. +
    • +     + + +

    + Dispatching an event on a Fragment will forward the dispatch to its + parent for the standard case. You can observe when dispatching that + the parent handler is called in additional to bubbling from there. A + delay is added to make the bubbling more clear.{' '} +

    +

    + When there have been event handlers added to the Fragment, the + Fragment's event handler will be called in addition to bubbling from + the parent. Without bubbling, only the Fragment's event handler will + be called. +

    +     + + + + + + + + + +
    { + setTimeout(() => { + setClick('grandparent'); + }, 200); + }} + className="card"> + Fragment grandparent - clicked:{' '} + {clickedState.grandparent ? 'true' : 'false'} +
    { + setTimeout(() => { + setClick('parent'); + }, 100); + }} + className="card"> + Fragment parent - clicked: {clickedState.parent ? 'true' : 'false'} + +
    { + setClick('child'); + }}> + Fragment child - clicked:{' '} + {clickedState.child ? 'true' : 'false'} +
    +     +
    +
    +     +     + ); +} diff --git a/fixtures/dom/src/components/fixtures/fragment-refs/index.js b/fixtures/dom/src/components/fixtures/fragment-refs/index.js index b84f273177d3a..23b440938cf7a 100644 --- a/fixtures/dom/src/components/fixtures/fragment-refs/index.js +++ b/fixtures/dom/src/components/fixtures/fragment-refs/index.js @@ -1,5 +1,6 @@ import FixtureSet from '../../FixtureSet'; import EventListenerCase from './EventListenerCase'; +import EventDispatchCase from './EventDispatchCase'; import IntersectionObserverCase from './IntersectionObserverCase'; import ResizeObserverCase from './ResizeObserverCase'; import FocusCase from './FocusCase'; @@ -11,6 +12,7 @@ export default function FragmentRefsPage() { return ( + diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 1eabf7a19a90b..373f33ba03021 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -2598,6 +2598,7 @@ export type FragmentInstanceType = { listener: EventListener, optionsOrUseCapture?: EventListenerOptionsOrUseCapture, ): void, + dispatchEvent(event: Event): boolean, focus(focusOptions?: FocusOptions): void, focusLast(focusOptions?: FocusOptions): void, blur(): void, @@ -2695,6 +2696,43 @@ function removeEventListenerFromChild( return false; } // $FlowFixMe[prop-missing] +FragmentInstance.prototype.dispatchEvent = function ( + this: FragmentInstanceType, + event: Event, +): boolean { + const parentHostFiber = getFragmentParentHostFiber(this._fragmentFiber); + if (parentHostFiber === null) { + return true; + } + const parentHostInstance = + getInstanceFromHostFiber(parentHostFiber); + const eventListeners = this._eventListeners; + if ( + (eventListeners !== null && eventListeners.length > 0) || + !event.bubbles + ) { + const temp = document.createTextNode(''); + if (eventListeners) { + for (let i = 0; i < eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = eventListeners[i]; + temp.addEventListener(type, listener, optionsOrUseCapture); + } + } + parentHostInstance.appendChild(temp); + const cancelable = temp.dispatchEvent(event); + if (eventListeners) { + for (let i = 0; i < eventListeners.length; i++) { + const {type, listener, optionsOrUseCapture} = eventListeners[i]; + temp.removeEventListener(type, listener, optionsOrUseCapture); + } + } + parentHostInstance.removeChild(temp); + return cancelable; + } else { + return parentHostInstance.dispatchEvent(event); + } +}; +// $FlowFixMe[prop-missing] FragmentInstance.prototype.focus = function ( this: FragmentInstanceType, focusOptions?: FocusOptions, diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index 6eb1fb3a0f346..e7e1d053a7a3e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -353,410 +353,346 @@ describe('FragmentRefs', () => { }); }); - describe('event listeners', () => { - // @gate enableFragmentRefs - it('adds and removes event listeners from children', async () => { - const parentRef = React.createRef(); - const fragmentRef = React.createRef(); - const childARef = React.createRef(); - const childBRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); - - let logs = []; + describe('events', () => { + describe('add/remove event listeners', () => { + // @gate enableFragmentRefs + it('adds and removes event listeners from children', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); - function handleFragmentRefClicks() { - logs.push('fragmentRef'); - } + let logs = []; - function Test() { - React.useEffect(() => { - fragmentRef.current.addEventListener( - 'click', - handleFragmentRefClicks, - ); + function handleFragmentRefClicks() { + logs.push('fragmentRef'); + } - return () => { - fragmentRef.current.removeEventListener( + function Test() { + React.useEffect(() => { + fragmentRef.current.addEventListener( 'click', handleFragmentRefClicks, ); - }; - }, []); - return ( -
    - - <>Text -
    A
    - <> -
    B
    - -     -
    - ); - } - await act(() => { - root.render(); - }); + return () => { + fragmentRef.current.removeEventListener( + 'click', + handleFragmentRefClicks, + ); + }; + }, []); + return ( +
    + + <>Text +
    A
    + <> +
    B
    + +     +
    + ); + } - childARef.current.addEventListener('click', () => { - logs.push('A'); - }); + await act(() => { + root.render(); + }); - childBRef.current.addEventListener('click', () => { - logs.push('B'); - }); + childARef.current.addEventListener('click', () => { + logs.push('A'); + }); - // Clicking on the parent should not trigger any listeners - parentRef.current.click(); - expect(logs).toEqual([]); + childBRef.current.addEventListener('click', () => { + logs.push('B'); + }); - // Clicking a child triggers its own listeners and the Fragment's - childARef.current.click(); - expect(logs).toEqual(['fragmentRef', 'A']); + // Clicking on the parent should not trigger any listeners + parentRef.current.click(); + expect(logs).toEqual([]); - logs = []; + // Clicking a child triggers its own listeners and the Fragment's + childARef.current.click(); + expect(logs).toEqual(['fragmentRef', 'A']); - childBRef.current.click(); - expect(logs).toEqual(['fragmentRef', 'B']); + logs = []; - logs = []; + childBRef.current.click(); + expect(logs).toEqual(['fragmentRef', 'B']); - fragmentRef.current.removeEventListener('click', handleFragmentRefClicks); + logs = []; - childARef.current.click(); - expect(logs).toEqual(['A']); + fragmentRef.current.removeEventListener( + 'click', + handleFragmentRefClicks, + ); - logs = []; + childARef.current.click(); + expect(logs).toEqual(['A']); - childBRef.current.click(); - expect(logs).toEqual(['B']); - }); + logs = []; - // @gate enableFragmentRefs - it('adds and removes event listeners from children with multiple fragments', async () => { - const fragmentRef = React.createRef(); - const nestedFragmentRef = React.createRef(); - const nestedFragmentRef2 = React.createRef(); - const childARef = React.createRef(); - const childBRef = React.createRef(); - const childCRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); + childBRef.current.click(); + expect(logs).toEqual(['B']); + }); - await act(() => { - root.render( -
    - -
    A
    -
    - -
    B
    + // @gate enableFragmentRefs + it('adds and removes event listeners from children with multiple fragments', async () => { + const fragmentRef = React.createRef(); + const nestedFragmentRef = React.createRef(); + const nestedFragmentRef2 = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const childCRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( +
    + +
    A
    +
    + +
    B
    +     +
    + +
    C
     -
    - -
    C
     -     -
    , - ); - }); +
    , + ); + }); - let logs = []; + let logs = []; - function handleFragmentRefClicks() { - logs.push('fragmentRef'); - } + function handleFragmentRefClicks() { + logs.push('fragmentRef'); + } - function handleNestedFragmentRefClicks() { - logs.push('nestedFragmentRef'); - } + function handleNestedFragmentRefClicks() { + logs.push('nestedFragmentRef'); + } - function handleNestedFragmentRef2Clicks() { - logs.push('nestedFragmentRef2'); - } + function handleNestedFragmentRef2Clicks() { + logs.push('nestedFragmentRef2'); + } - fragmentRef.current.addEventListener('click', handleFragmentRefClicks); - nestedFragmentRef.current.addEventListener( - 'click', - handleNestedFragmentRefClicks, - ); - nestedFragmentRef2.current.addEventListener( - 'click', - handleNestedFragmentRef2Clicks, - ); + fragmentRef.current.addEventListener('click', handleFragmentRefClicks); + nestedFragmentRef.current.addEventListener( + 'click', + handleNestedFragmentRefClicks, + ); + nestedFragmentRef2.current.addEventListener( + 'click', + handleNestedFragmentRef2Clicks, + ); - childBRef.current.click(); - // Event bubbles to the parent fragment - expect(logs).toEqual(['nestedFragmentRef', 'fragmentRef']); + childBRef.current.click(); + // Event bubbles to the parent fragment + expect(logs).toEqual(['nestedFragmentRef', 'fragmentRef']); - logs = []; + logs = []; - childARef.current.click(); - expect(logs).toEqual(['fragmentRef']); + childARef.current.click(); + expect(logs).toEqual(['fragmentRef']); - logs = []; - childCRef.current.click(); - expect(logs).toEqual(['fragmentRef', 'nestedFragmentRef2']); + logs = []; + childCRef.current.click(); + expect(logs).toEqual(['fragmentRef', 'nestedFragmentRef2']); - logs = []; + logs = []; - fragmentRef.current.removeEventListener('click', handleFragmentRefClicks); - nestedFragmentRef.current.removeEventListener( - 'click', - handleNestedFragmentRefClicks, - ); - childCRef.current.click(); - expect(logs).toEqual(['nestedFragmentRef2']); - }); + fragmentRef.current.removeEventListener( + 'click', + handleFragmentRefClicks, + ); + nestedFragmentRef.current.removeEventListener( + 'click', + handleNestedFragmentRefClicks, + ); + childCRef.current.click(); + expect(logs).toEqual(['nestedFragmentRef2']); + }); - // @gate enableFragmentRefs - it('adds an event listener to a newly added child', async () => { - const fragmentRef = React.createRef(); - const childRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); - let showChild; + // @gate enableFragmentRefs + it('adds an event listener to a newly added child', async () => { + const fragmentRef = React.createRef(); + const childRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let showChild; - function Component() { - const [shouldShowChild, setShouldShowChild] = React.useState(false); - showChild = () => { - setShouldShowChild(true); - }; + function Component() { + const [shouldShowChild, setShouldShowChild] = React.useState(false); + showChild = () => { + setShouldShowChild(true); + }; - return ( -
    - -
    A
    - {shouldShowChild && ( -
    - B -
    - )} -     -
    - ); - } + return ( +
    + +
    A
    + {shouldShowChild && ( +
    + B +
    + )} +     +
    + ); + } - await act(() => { - root.render(); - }); + await act(() => { + root.render(); + }); - expect(fragmentRef.current).not.toBe(null); - expect(childRef.current).toBe(null); + expect(fragmentRef.current).not.toBe(null); + expect(childRef.current).toBe(null); - let hasClicked = false; - fragmentRef.current.addEventListener('click', () => { - hasClicked = true; - }); + let hasClicked = false; + fragmentRef.current.addEventListener('click', () => { + hasClicked = true; + }); - await act(() => { - showChild(); - }); - expect(childRef.current).not.toBe(null); + await act(() => { + showChild(); + }); + expect(childRef.current).not.toBe(null); - childRef.current.click(); - expect(hasClicked).toBe(true); - }); + childRef.current.click(); + expect(hasClicked).toBe(true); + }); - // @gate enableFragmentRefs - it('applies event listeners to host children nested within non-host children', async () => { - const fragmentRef = React.createRef(); - const childRef = React.createRef(); - const nestedChildRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); + // @gate enableFragmentRefs + it('applies event listeners to host children nested within non-host children', async () => { + const fragmentRef = React.createRef(); + const childRef = React.createRef(); + const nestedChildRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( -
    - -
    Host A
    - + await act(() => { + root.render( +
    + +
    Host A
    -
    Host B
    + +
    Host B
    +     - -     -
    , - ); - }); - const logs = []; - fragmentRef.current.addEventListener('click', e => { - logs.push(e.target.textContent); - }); - - expect(logs).toEqual([]); - childRef.current.click(); - expect(logs).toEqual(['Host A']); - nestedChildRef.current.click(); - expect(logs).toEqual(['Host A', 'Host B']); - }); - - // @gate enableFragmentRefs - it('allows adding and cleaning up listeners in effects', async () => { - const root = ReactDOMClient.createRoot(container); - - let logs = []; - function logClick(e) { - logs.push(e.currentTarget.id); - } - - let rerender; - let removeEventListeners; - - function Test() { - const fragmentRef = React.useRef(null); - // eslint-disable-next-line no-unused-vars - const [_, setState] = React.useState(0); - rerender = () => { - setState(p => p + 1); - }; - removeEventListeners = () => { - fragmentRef.current.removeEventListener('click', logClick); - }; - React.useEffect(() => { - fragmentRef.current.addEventListener('click', logClick); - - return removeEventListeners; +     +
    , + ); + }); + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.textContent); }); - return ( - -
    - - ); - } - - // The event listener was applied - await act(() => root.render()); - expect(logs).toEqual([]); - document.querySelector('#child-a').click(); - expect(logs).toEqual(['child-a']); + expect(logs).toEqual([]); + childRef.current.click(); + expect(logs).toEqual(['Host A']); + nestedChildRef.current.click(); + expect(logs).toEqual(['Host A', 'Host B']); + }); - // The event listener can be removed and re-added - logs = []; - await act(rerender); - document.querySelector('#child-a').click(); - expect(logs).toEqual(['child-a']); - }); + // @gate enableFragmentRefs + it('allows adding and cleaning up listeners in effects', async () => { + const root = ReactDOMClient.createRoot(container); - // @gate enableFragmentRefs - it('does not apply removed event listeners to new children', async () => { - const root = ReactDOMClient.createRoot(container); - const fragmentRef = React.createRef(null); - function Test() { - return ( - -
    - - ); - } + let logs = []; + function logClick(e) { + logs.push(e.currentTarget.id); + } - let logs = []; - function logClick(e) { - logs.push(e.currentTarget.id); - } - await act(() => { - root.render(); - }); - fragmentRef.current.addEventListener('click', logClick); - const childA = document.querySelector('#child-a'); - childA.click(); - expect(logs).toEqual(['child-a']); + let rerender; + let removeEventListeners; - logs = []; - fragmentRef.current.removeEventListener('click', logClick); - childA.click(); - expect(logs).toEqual([]); - }); + function Test() { + const fragmentRef = React.useRef(null); + // eslint-disable-next-line no-unused-vars + const [_, setState] = React.useState(0); + rerender = () => { + setState(p => p + 1); + }; + removeEventListeners = () => { + fragmentRef.current.removeEventListener('click', logClick); + }; + React.useEffect(() => { + fragmentRef.current.addEventListener('click', logClick); - // @gate enableFragmentRefs - it('applies event listeners to portaled children', async () => { - const fragmentRef = React.createRef(); - const childARef = React.createRef(); - const childBRef = React.createRef(); - const root = ReactDOMClient.createRoot(container); + return removeEventListeners; + }); - function Test() { - return ( - -
    - {createPortal(
    , document.body)} - - ); - } + return ( + +
    + + ); + } - await act(() => { - root.render(); - }); + // The event listener was applied + await act(() => root.render()); + expect(logs).toEqual([]); + document.querySelector('#child-a').click(); + expect(logs).toEqual(['child-a']); - const logs = []; - fragmentRef.current.addEventListener('click', e => { - logs.push(e.target.id); + // The event listener can be removed and re-added + logs = []; + await act(rerender); + document.querySelector('#child-a').click(); + expect(logs).toEqual(['child-a']); }); - childARef.current.click(); - expect(logs).toEqual(['child-a']); - - logs.length = 0; - childBRef.current.click(); - expect(logs).toEqual(['child-b']); - }); - - describe('with activity', () => { - // @gate enableFragmentRefs && enableActivity - it('does not apply event listeners to hidden trees', async () => { - const parentRef = React.createRef(); - const fragmentRef = React.createRef(); + // @gate enableFragmentRefs + it('does not apply removed event listeners to new children', async () => { const root = ReactDOMClient.createRoot(container); - + const fragmentRef = React.createRef(null); function Test() { return ( -
    - -
    Child 1
    - -
    Child 2
    -     -
    Child 3
    -     -
    + +
    + ); } + let logs = []; + function logClick(e) { + logs.push(e.currentTarget.id); + } await act(() => { root.render(); }); + fragmentRef.current.addEventListener('click', logClick); + const childA = document.querySelector('#child-a'); + childA.click(); + expect(logs).toEqual(['child-a']); - const logs = []; - fragmentRef.current.addEventListener('click', e => { - logs.push(e.target.textContent); - }); - - const [child1, child2, child3] = parentRef.current.children; - child1.click(); - child2.click(); - child3.click(); - expect(logs).toEqual(['Child 1', 'Child 3']); + logs = []; + fragmentRef.current.removeEventListener('click', logClick); + childA.click(); + expect(logs).toEqual([]); }); - // @gate enableFragmentRefs && enableActivity - it('applies event listeners to visible trees', async () => { - const parentRef = React.createRef(); + // @gate enableFragmentRefs + it('applies event listeners to portaled children', async () => { const fragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); const root = ReactDOMClient.createRoot(container); function Test() { return ( -
    - -
    Child 1
    - -
    Child 2
    -     -
    Child 3
    -     -
    + +
    + {createPortal( +
    , + document.body, + )} + ); } @@ -766,67 +702,242 @@ describe('FragmentRefs', () => { const logs = []; fragmentRef.current.addEventListener('click', e => { - logs.push(e.target.textContent); + logs.push(e.target.id); }); - const [child1, child2, child3] = parentRef.current.children; - child1.click(); - child2.click(); - child3.click(); - expect(logs).toEqual(['Child 1', 'Child 2', 'Child 3']); + childARef.current.click(); + expect(logs).toEqual(['child-a']); + + logs.length = 0; + childBRef.current.click(); + expect(logs).toEqual(['child-b']); + }); + + describe('with activity', () => { + // @gate enableFragmentRefs && enableActivity + it('does not apply event listeners to hidden trees', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
    + +
    Child 1
    + +
    Child 2
    +     +
    Child 3
    +     +
    + ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.textContent); + }); + + const [child1, child2, child3] = parentRef.current.children; + child1.click(); + child2.click(); + child3.click(); + expect(logs).toEqual(['Child 1', 'Child 3']); + }); + + // @gate enableFragmentRefs && enableActivity + it('applies event listeners to visible trees', async () => { + const parentRef = React.createRef(); + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test() { + return ( +
    + +
    Child 1
    + +
    Child 2
    +     +
    Child 3
    +     +
    + ); + } + + await act(() => { + root.render(); + }); + + const logs = []; + fragmentRef.current.addEventListener('click', e => { + logs.push(e.target.textContent); + }); + + const [child1, child2, child3] = parentRef.current.children; + child1.click(); + child2.click(); + child3.click(); + expect(logs).toEqual(['Child 1', 'Child 2', 'Child 3']); + }); + + // @gate enableFragmentRefs && enableActivity + it('handles Activity modes switching', async () => { + const fragmentRef = React.createRef(); + const fragmentRef2 = React.createRef(); + const parentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + function Test({mode}) { + return ( +
    + + +
    Child
    + +
    Child 2
    +     +     +     +
    + ); + } + + await act(() => { + root.render(); + }); + + let logs = []; + fragmentRef.current.addEventListener('click', () => { + logs.push('clicked 1'); + }); + fragmentRef2.current.addEventListener('click', () => { + logs.push('clicked 2'); + }); + parentRef.current.lastChild.click(); + expect(logs).toEqual(['clicked 1', 'clicked 2']); + + logs = []; + await act(() => { + root.render(); + }); + parentRef.current.firstChild.click(); + parentRef.current.lastChild.click(); + expect(logs).toEqual([]); + + logs = []; + await act(() => { + root.render(); + }); + parentRef.current.lastChild.click(); + // Event order is flipped here because the nested child re-registers first + expect(logs).toEqual(['clicked 2', 'clicked 1']); + }); }); + }); - // @gate enableFragmentRefs && enableActivity - it('handles Activity modes switching', async () => { + describe('dispatchEvent()', () => { + // @gate enableFragmentRefs + it('fires events on the host parent if bubbles=true', async () => { const fragmentRef = React.createRef(); - const fragmentRef2 = React.createRef(); - const parentRef = React.createRef(); const root = ReactDOMClient.createRoot(container); + let logs = []; + + function handleClick(e) { + logs.push([e.type, e.target.id, e.currentTarget.id]); + } - function Test({mode}) { + function Test({isMounted}) { return ( -
    - - -
    Child
    - -
    Child 2
    +
    +
    + {isMounted && ( + +
    + Hi +
    - - + )} +
    ); } await act(() => { - root.render(); + root.render(); }); - let logs = []; - fragmentRef.current.addEventListener('click', () => { - logs.push('clicked 1'); - }); - fragmentRef2.current.addEventListener('click', () => { - logs.push('clicked 2'); - }); - parentRef.current.lastChild.click(); - expect(logs).toEqual(['clicked 1', 'clicked 2']); + let isCancelable = !fragmentRef.current.dispatchEvent( + new MouseEvent('click', {bubbles: true}), + ); + expect(logs).toEqual([ + ['click', 'parent', 'parent'], + ['click', 'parent', 'grandparent'], + ]); + expect(isCancelable).toBe(false); - logs = []; + const fragmentInstanceHandle = fragmentRef.current; await act(() => { - root.render(); + root.render(); }); - parentRef.current.firstChild.click(); - parentRef.current.lastChild.click(); + logs = []; + isCancelable = !fragmentInstanceHandle.dispatchEvent( + new MouseEvent('click', {bubbles: true}), + ); expect(logs).toEqual([]); + expect(isCancelable).toBe(false); logs = []; + isCancelable = !fragmentInstanceHandle.dispatchEvent( + new MouseEvent('click', {bubbles: false}), + ); + expect(logs).toEqual([]); + expect(isCancelable).toBe(false); + }); + + // @gate enableFragmentRefs + it('fires events on self, and only self if bubbles=false', async () => { + const fragmentRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + let logs = []; + + function handleClick(e) { + logs.push([e.type, e.target.id, e.currentTarget.id]); + } + + function Test() { + return ( +
    + +
    + ); + } + await act(() => { - root.render(); + root.render(); }); - parentRef.current.lastChild.click(); - // Event order is flipped here because the nested child re-registers first - expect(logs).toEqual(['clicked 2', 'clicked 1']); + + fragmentRef.current.addEventListener('click', handleClick); + + fragmentRef.current.dispatchEvent( + new MouseEvent('click', {bubbles: true}), + ); + expect(logs).toEqual([ + ['click', undefined, undefined], + ['click', 'parent', 'parent'], + ]); + + logs = []; + + fragmentRef.current.dispatchEvent( + new MouseEvent('click', {bubbles: false}), + ); + expect(logs).toEqual([['click', undefined, undefined]]); }); }); });