From 62208bee5ad7e447d42459ace8c0edcb7c4f9197 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 2 Jan 2025 14:07:21 +0000 Subject: [PATCH 1/5] DevTools: fork FastRefresh test for <18 versions of React (#31893) We currently have a failing test for React DevTools against React 17. This started failing in https://github.com/facebook/react/pull/30899, where we changed logic for error tracking and started relying on `onPostCommitFiberRoot` hook. Looking at https://github.com/facebook/react/pull/21183, `onPostCommitFiberRoot` was shipped in 18, which means that any console errors / warnings emitted in passive effects won't be recorded by React DevTools for React < 18. --- .../FastRefreshDevToolsIntegration-test.js | 77 ++++++++++++++++++- packages/react-devtools-shared/src/hook.js | 1 + 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js index ff07f0ef16f8a..a80a11fde12d4 100644 --- a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js +++ b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js @@ -186,8 +186,83 @@ describe('Fast Refresh', () => { expect(getContainer().firstChild).not.toBe(element); }); + // @reactVersion < 18.0 // @reactVersion >= 16.9 - it('should not break when there are warnings in between patching', () => { + it('should not break when there are warnings in between patching (before post commit hook)', () => { + withErrorsOrWarningsIgnored(['Expected:'], () => { + render(` + const {useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + console.warn("Expected: warning during render"); + return null; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); + + withErrorsOrWarningsIgnored(['Expected:'], () => { + patch(` + const {useEffect, useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + console.warn("Expected: warning during render"); + return null; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ⚠ + `); + + withErrorsOrWarningsIgnored(['Expected:'], () => { + patch(` + const {useEffect, useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + useEffect(() => { + console.error("Expected: error during effect"); + }); + console.warn("Expected: warning during render"); + return null; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); + + withErrorsOrWarningsIgnored(['Expected:'], () => { + patch(` + const {useEffect, useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + console.warn("Expected: warning during render"); + return null; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); + }); + + // @reactVersion >= 18.0 + it('should not break when there are warnings in between patching (with post commit hook)', () => { withErrorsOrWarningsIgnored(['Expected:'], () => { render(` const {useState} = React; diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index d754140f96541..699d23094571b 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -648,6 +648,7 @@ export function installHook( checkDCE, onCommitFiberUnmount, onCommitFiberRoot, + // React v18.0+ onPostCommitFiberRoot, setStrictMode, From c8c89fab5beaa481d132318437b2651ec89440c3 Mon Sep 17 00:00:00 2001 From: lauren Date: Thu, 2 Jan 2025 11:24:26 -0500 Subject: [PATCH 2/5] [compiler] Update rollup plugins (#31919) Update our various compiler rollup plugins. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31919). * #31927 * #31918 * #31917 * #31916 * __->__ #31919 --- compiler/package.json | 6 +- .../rollup.config.js | 1 + .../rollup.config.js | 1 + .../rollup.config.js | 1 + .../react-compiler-runtime/rollup.config.js | 1 + compiler/yarn.lock | 75 ++++++++----------- 6 files changed, 38 insertions(+), 47 deletions(-) diff --git a/compiler/package.json b/compiler/package.json index c05e0e70d3e35..b25031b9967d8 100644 --- a/compiler/package.json +++ b/compiler/package.json @@ -26,11 +26,11 @@ "react-is": "0.0.0-experimental-4beb1fd8-20241118" }, "devDependencies": { - "@rollup/plugin-commonjs": "^25.0.7", + "@rollup/plugin-commonjs": "^28.0.2", "@rollup/plugin-json": "^6.1.0", - "@rollup/plugin-node-resolve": "^15.2.3", + "@rollup/plugin-node-resolve": "^16.0.0", "@rollup/plugin-terser": "^0.4.4", - "@rollup/plugin-typescript": "^11.1.6", + "@rollup/plugin-typescript": "^12.1.2", "@tsconfig/strictest": "^2.0.5", "concurrently": "^7.4.0", "folder-hash": "^4.0.4", diff --git a/compiler/packages/babel-plugin-react-compiler/rollup.config.js b/compiler/packages/babel-plugin-react-compiler/rollup.config.js index 77e785c4641f2..b95cc89b39b13 100644 --- a/compiler/packages/babel-plugin-react-compiler/rollup.config.js +++ b/compiler/packages/babel-plugin-react-compiler/rollup.config.js @@ -24,6 +24,7 @@ const DEV_ROLLUP_CONFIG = { format: 'cjs', sourcemap: false, exports: 'named', + inlineDynamicImports: true, }, plugins: [ typescript({ diff --git a/compiler/packages/eslint-plugin-react-compiler/rollup.config.js b/compiler/packages/eslint-plugin-react-compiler/rollup.config.js index 4d813564098a4..743e4cc844102 100644 --- a/compiler/packages/eslint-plugin-react-compiler/rollup.config.js +++ b/compiler/packages/eslint-plugin-react-compiler/rollup.config.js @@ -29,6 +29,7 @@ const DEV_ROLLUP_CONFIG = { file: 'dist/index.js', format: 'cjs', sourcemap: false, + inlineDynamicImports: true, }, treeshake: { moduleSideEffects: false, diff --git a/compiler/packages/react-compiler-healthcheck/rollup.config.js b/compiler/packages/react-compiler-healthcheck/rollup.config.js index 117974ad6b776..0c2492d14069d 100644 --- a/compiler/packages/react-compiler-healthcheck/rollup.config.js +++ b/compiler/packages/react-compiler-healthcheck/rollup.config.js @@ -33,6 +33,7 @@ const DEV_ROLLUP_CONFIG = { format: 'cjs', sourcemap: false, exports: 'named', + inlineDynamicImports: true, }, plugins: [ typescript({ diff --git a/compiler/packages/react-compiler-runtime/rollup.config.js b/compiler/packages/react-compiler-runtime/rollup.config.js index 260359dd0cd81..2399f2160c223 100644 --- a/compiler/packages/react-compiler-runtime/rollup.config.js +++ b/compiler/packages/react-compiler-runtime/rollup.config.js @@ -21,6 +21,7 @@ const PROD_ROLLUP_CONFIG = { file: 'dist/index.js', format: 'cjs', sourcemap: true, + inlineDynamicImports: true, }, plugins: [ typescript({ diff --git a/compiler/yarn.lock b/compiler/yarn.lock index b4c72ff3c5ede..21f771c93fd68 100644 --- a/compiler/yarn.lock +++ b/compiler/yarn.lock @@ -2512,17 +2512,18 @@ resolved "https://registry.yarnpkg.com/@pkgjs/parseargs/-/parseargs-0.11.0.tgz#a77ea742fab25775145434eb1d2328cf5013ac33" integrity sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg== -"@rollup/plugin-commonjs@^25.0.7": - version "25.0.7" - resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-25.0.7.tgz#145cec7589ad952171aeb6a585bbeabd0fd3b4cf" - integrity sha512-nEvcR+LRjEjsaSsc4x3XZfCCvZIaSMenZu/OiwOKGN2UhQpAYI7ru7czFvyWbErlpoGjnSX3D5Ch5FcMA3kRWQ== +"@rollup/plugin-commonjs@^28.0.2": + version "28.0.2" + resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-28.0.2.tgz#193d7a86470f112b56927c1d821ee45951a819ea" + integrity sha512-BEFI2EDqzl+vA1rl97IDRZ61AIwGH093d9nz8+dThxJNH8oSoB7MjWvPCX3dkaK1/RCJ/1v/R1XB15FuSs0fQw== dependencies: "@rollup/pluginutils" "^5.0.1" commondir "^1.0.1" estree-walker "^2.0.2" - glob "^8.0.3" + fdir "^6.2.0" is-reference "1.2.1" magic-string "^0.30.3" + picomatch "^4.0.2" "@rollup/plugin-json@^6.1.0": version "6.1.0" @@ -2531,15 +2532,14 @@ dependencies: "@rollup/pluginutils" "^5.1.0" -"@rollup/plugin-node-resolve@^15.2.3": - version "15.2.3" - resolved "https://registry.yarnpkg.com/@rollup/plugin-node-resolve/-/plugin-node-resolve-15.2.3.tgz#e5e0b059bd85ca57489492f295ce88c2d4b0daf9" - integrity sha512-j/lym8nf5E21LwBT4Df1VD6hRO2L2iwUeUmP7litikRsVp1H6NWx20NEp0Y7su+7XGc476GnXXc4kFeZNGmaSQ== +"@rollup/plugin-node-resolve@^16.0.0": + version "16.0.0" + resolved "https://registry.yarnpkg.com/@rollup/plugin-node-resolve/-/plugin-node-resolve-16.0.0.tgz#b1a0594661f40d7b061d82136e847354ff85f211" + integrity sha512-0FPvAeVUT/zdWoO0jnb/V5BlBsUSNfkIOtFHzMO4H9MOklrmQFY6FduVHKucNb/aTFxvnGhj4MNj/T1oNdDfNg== dependencies: "@rollup/pluginutils" "^5.0.1" "@types/resolve" "1.20.2" deepmerge "^4.2.2" - is-builtin-module "^3.2.1" is-module "^1.0.0" resolve "^1.22.1" @@ -2552,10 +2552,10 @@ smob "^1.0.0" terser "^5.17.4" -"@rollup/plugin-typescript@^11.1.6": - version "11.1.6" - resolved "https://registry.yarnpkg.com/@rollup/plugin-typescript/-/plugin-typescript-11.1.6.tgz#724237d5ec12609ec01429f619d2a3e7d4d1b22b" - integrity sha512-R92yOmIACgYdJ7dJ97p4K69I8gg6IEHt8M7dUBxN3W6nrO8uUxX5ixl0yU/N3aZTi8WhPuICvOHXQvF6FaykAA== +"@rollup/plugin-typescript@^12.1.2": + version "12.1.2" + resolved "https://registry.yarnpkg.com/@rollup/plugin-typescript/-/plugin-typescript-12.1.2.tgz#ebaeec2e7376faa889030ccd7cb485a649e63118" + integrity sha512-cdtSp154H5sv637uMr1a8OTWB0L1SWDSm1rDGiyfcGcvQ6cuTs4MDk2BVEBGysUWago4OJN4EQZqOTl/QY3Jgg== dependencies: "@rollup/pluginutils" "^5.1.0" resolve "^1.22.1" @@ -3621,11 +3621,6 @@ buffer@^5.5.0: base64-js "^1.3.1" ieee754 "^1.1.13" -builtin-modules@^3.3.0: - version "3.3.0" - resolved "https://registry.yarnpkg.com/builtin-modules/-/builtin-modules-3.3.0.tgz#cae62812b89801e9656336e46223e030386be7b6" - integrity sha512-zhaCDicdLuWN5UbN5IMnFqNMhNfo919sH85y2/ea+5Yg9TsTkeZxpL+JLbp6cgYFS4sRLp3YV4S6yDuqVWHYOw== - call-bind@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/call-bind/-/call-bind-1.0.2.tgz#b1d4e89e688119c3c9a903ad30abb2f6a919be3c" @@ -4408,6 +4403,11 @@ fbt@^1.0.2: dependencies: invariant "^2.2.4" +fdir@^6.2.0: + version "6.4.2" + resolved "https://registry.yarnpkg.com/fdir/-/fdir-6.4.2.tgz#ddaa7ce1831b161bc3657bb99cb36e1622702689" + integrity sha512-KnhMXsKSPZlAhp7+IjUkRZKPb4fUyccpDrdFXbi4QL1qkmFh9kVY09Yox+n4MaOb3lHZ1Tv829C3oaaXoMYPDQ== + file-entry-cache@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/file-entry-cache/-/file-entry-cache-6.0.1.tgz#211b2dd9659cb0394b073e7323ac3c933d522027" @@ -4592,17 +4592,6 @@ glob@^7.1.3, glob@^7.1.4, glob@^7.1.6: once "^1.3.0" path-is-absolute "^1.0.0" -glob@^8.0.3: - version "8.1.0" - resolved "https://registry.yarnpkg.com/glob/-/glob-8.1.0.tgz#d388f656593ef708ee3e34640fdfb99a9fd1c33e" - integrity sha512-r8hpEjiQEYlF2QU0df3dS+nxxSIreXQS1qRhMJM0Q5NDdR386C7jb7Hwwod8Fgiuex+k0GFjgft18yvxm5XoCQ== - dependencies: - fs.realpath "^1.0.0" - inflight "^1.0.4" - inherits "2" - minimatch "^5.0.1" - once "^1.3.0" - globals@^11.1.0: version "11.12.0" resolved "https://registry.yarnpkg.com/globals/-/globals-11.12.0.tgz#ab8795338868a0babd8525758018c2a7eb95c42e" @@ -4804,13 +4793,6 @@ is-arrayish@^0.2.1: resolved "https://registry.yarnpkg.com/is-arrayish/-/is-arrayish-0.2.1.tgz#77c99840527aa8ecb1a8ba697b80645a7a926a9d" integrity sha512-zz06S8t0ozoDXMG+ube26zeCTNXcKIPJZJi8hBrF4idCLms4CG9QtK7qBl1boi5ODzFpjswb5JPmHCbMpjaYzg== -is-builtin-module@^3.2.1: - version "3.2.1" - resolved "https://registry.yarnpkg.com/is-builtin-module/-/is-builtin-module-3.2.1.tgz#f03271717d8654cfcaf07ab0463faa3571581169" - integrity sha512-BSLE3HnV2syZ0FK0iMA/yUGplUeMmNz4AW5fnTunbCIqZi4vG3WjJT9FHMy5D69xmAYBHXQhJdALdpwVxV501A== - dependencies: - builtin-modules "^3.3.0" - is-core-module@^2.11.0: version "2.12.1" resolved "https://registry.yarnpkg.com/is-core-module/-/is-core-module-2.12.1.tgz#0c0b6885b6f80011c71541ce15c8d66cf5a4f9fd" @@ -6429,13 +6411,6 @@ minimatch@^3.0.4, minimatch@^3.0.5, minimatch@^3.1.1, minimatch@^3.1.2: dependencies: brace-expansion "^1.1.7" -minimatch@^5.0.1, minimatch@~5.1.2: - version "5.1.6" - resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-5.1.6.tgz#1cfcb8cf5522ea69952cd2af95ae09477f122a96" - integrity sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g== - dependencies: - brace-expansion "^2.0.1" - minimatch@^9.0.4: version "9.0.5" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-9.0.5.tgz#d74f9dd6b57d83d8e98cfb82133b03978bc929e5" @@ -6443,6 +6418,13 @@ minimatch@^9.0.4: dependencies: brace-expansion "^2.0.1" +minimatch@~5.1.2: + version "5.1.6" + resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-5.1.6.tgz#1cfcb8cf5522ea69952cd2af95ae09477f122a96" + integrity sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g== + dependencies: + brace-expansion "^2.0.1" + minimist@^1.2.8: version "1.2.8" resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.8.tgz#c1a464e7693302e082a075cee0c057741ac4772c" @@ -6724,6 +6706,11 @@ picomatch@^2.0.4, picomatch@^2.2.3, picomatch@^2.3.1: resolved "https://registry.yarnpkg.com/picomatch/-/picomatch-2.3.1.tgz#3ba3833733646d9d3e4995946c1365a67fb07a42" integrity sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA== +picomatch@^4.0.2: + version "4.0.2" + resolved "https://registry.yarnpkg.com/picomatch/-/picomatch-4.0.2.tgz#77c742931e8f3b8820946c76cd0c1f13730d1dab" + integrity sha512-M7BAV6Rlcy5u+m6oPhAPFgJTzAioX/6B0DxyvDlo9l8+T3nLKbrczg2WLUyzd45L8RqfUMyGPzekbMvX2Ldkwg== + pify@^4.0.1: version "4.0.1" resolved "https://registry.yarnpkg.com/pify/-/pify-4.0.1.tgz#4b2cd25c50d598735c50292224fd8c6df41e3231" From fe21c947c82b173ae538aa1d215559ec3dccd103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Jan 2025 13:02:22 -0500 Subject: [PATCH 3/5] [Fiber] Yield every other frame for Transition/Retry work (#31828) This flag first moves the `shouldYield()` logic into React itself. We need this for `postTask` compatibility anyway since this logic is no longer a concern of the scheduler. This means that there can also be no global `requestPaint()` that asks for painting earlier. So this is best rolled out with `enableAlwaysYieldScheduler` (and ideally `enableYieldingBeforePassive`) instead of `enableRequestPaint`. Once in React we can change the yield timing heuristics. This uses the previous 5ms for Idle work to keep everything responsive while doing background work. However, for Transitions and Retries we have seen that same thread animations (like loading states animating, or constant animations like cool Three.js stuff) can take CPU time away from the Transition that causes moving into new content to slow down. Therefore we only yield every 25ms. The purpose of this yield is not to avoid the overhead of yielding, which is very low, but rather to intentionally block any frequently occurring other main thread work like animations from starving our work. If we could we could just tell everyone else to throttle their stuff for ideal scheduling but that's not quite realistic. In other words, the purpose of this is to reduce the frame rate of animations to 30 fps and we achieve this by not yielding. We still do yield to allow the animations to not just stall. This seems like a good balance. The 5ms of Idle is because we don't really need to yield less often since the overhead is low. We keep it low to allow 120 fps animations to run if necessary and our work may not be the only work within a frame so we need to yield early enough to leave enough time left. Similarly we choose 25ms rather than say 35ms to ensure that we push long enough to guarantee to half the frame rate but low enough that there's plenty of time left for a rAF to power each animation every other frame. It's also low enough that if something else interrupts the work like a new interaction, we can still be responsive to that within 50ms or so. We also need to yield in case there's I/O work that needs to get bounced through the main thread. This flag is currently off everywhere since we have so many other scheduling flags but that means there's some urgency to roll those out fully so we can test this one. There's also some tests to update since this doesn't go through the Mock scheduler anymore for yields. --- .../src/ReactFiberWorkLoop.js | 26 ++++++++++++++++--- packages/shared/ReactFeatureFlags.js | 3 +++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 2 ++ .../forks/ReactFeatureFlags.test-renderer.js | 2 ++ ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 2 ++ .../shared/forks/ReactFeatureFlags.www.js | 2 ++ 8 files changed, 36 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 2e8b443d22caa..8e75135ff6e08 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -41,6 +41,7 @@ import { enableSiblingPrerendering, enableComponentPerformanceTrack, enableYieldingBeforePassive, + enableThrottledScheduling, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; @@ -2610,8 +2611,10 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // can't trust the result of `shouldYield`, because the host I/O is // likely mocked. workLoopSync(); + } else if (enableThrottledScheduling) { + workLoopConcurrent(includesNonIdleWork(lanes)); } else { - workLoopConcurrent(); + workLoopConcurrentByScheduler(); } break; } catch (thrownValue) { @@ -2650,10 +2653,27 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { } /** @noinline */ -function workLoopConcurrent() { +function workLoopConcurrent(nonIdle: boolean) { + // We yield every other "frame" when rendering Transition or Retries. Those are blocking + // revealing new content. The purpose of this yield is not to avoid the overhead of yielding, + // which is very low, but rather to intentionally block any frequently occuring other main + // thread work like animations from starving our work. In other words, the purpose of this + // is to reduce the framerate of animations to 30 frames per second. + // For Idle work we yield every 5ms to keep animations going smooth. + if (workInProgress !== null) { + const yieldAfter = now() + (nonIdle ? 25 : 5); + do { + // $FlowFixMe[incompatible-call] flow doesn't know that now() is side-effect free + performUnitOfWork(workInProgress); + } while (workInProgress !== null && now() < yieldAfter); + } +} + +/** @noinline */ +function workLoopConcurrentByScheduler() { // Perform work until Scheduler asks us to yield while (workInProgress !== null && !shouldYield()) { - // $FlowFixMe[incompatible-call] found when upgrading Flow + // $FlowFixMe[incompatible-call] flow doesn't know that shouldYield() is side-effect free performUnitOfWork(workInProgress); } } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 0720ab2a88b78..f1332a2bfd1ec 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -81,6 +81,9 @@ export const enableLegacyFBSupport = false; // Fix gated tests that fail with this flag enabled before turning it back on. export const enableYieldingBeforePassive = false; +// Experiment to intentionally yield less to block high framerate animations. +export const enableThrottledScheduling = false; + export const enableLegacyCache = __EXPERIMENTAL__; export const enableAsyncIterableChildren = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 4ea9499e6ca80..ed06661eedc39 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -81,6 +81,7 @@ export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const enableHydrationLaneScheduling = true; export const enableYieldingBeforePassive = false; +export const enableThrottledScheduling = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index dddd80aeea82a..2220891b7b426 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -73,6 +73,8 @@ export const enableHydrationLaneScheduling = true; export const enableYieldingBeforePassive = false; +export const enableThrottledScheduling = false; + // Profiling Only export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 95826007dc9cd..2c5bc8f297ba5 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -72,6 +72,8 @@ export const enableUseResourceEffectHook = false; export const enableYieldingBeforePassive = true; +export const enableThrottledScheduling = false; + // TODO: This must be in sync with the main ReactFeatureFlags file because // the Test Renderer's value must be the same as the one used by the // react package. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 81060cfafb1b1..1c2fbb4847960 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -69,6 +69,7 @@ export const enableSiblingPrerendering = true; export const enableUseResourceEffectHook = true; export const enableHydrationLaneScheduling = true; export const enableYieldingBeforePassive = false; +export const enableThrottledScheduling = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e0e9906d52b8f..8e96038f44246 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -84,5 +84,7 @@ export const enableHydrationLaneScheduling = true; export const enableYieldingBeforePassive = false; +export const enableThrottledScheduling = false; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index cf514f93d43ac..658443aea13e9 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -58,6 +58,8 @@ export const enableLegacyFBSupport = true; export const enableYieldingBeforePassive = false; +export const enableThrottledScheduling = false; + export const enableHydrationLaneScheduling = true; export const enableComponentPerformanceTrack = false; From 1e9eb95db5b3a2064ecc26915a4e640b3a9bdaf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Jan 2025 13:04:09 -0500 Subject: [PATCH 4/5] [Fiber] Mark cascading updates (#31866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A common source of performance problems is due to cascading renders from calling `setState` in `useLayoutEffect` or `useEffect`. This marks the entry from the update to when we start the render as red and `"Cascade"` to highlight this. Screenshot 2024-12-19 at 10 54 59 PM In addition to this case, there's another case where you call `setState` multiple times in the same event causing multiple renders. This might be due to multiple `flushSync`, or spawned a microtasks from a `useLayoutEffect`. In theory it could also be from a microtask scheduled after the first `setState`. This one we can only detect if it's from an event that has a `window.event` since otherwise it's hard to know if we're still in the same event. Screenshot 2024-12-19 at 11 38 44 PM I decided against making a ping in a microtask considered a cascade. Because that should ideally be using the Suspense Optimization and so wouldn't be considered multi-pass. Screenshot 2024-12-19 at 11 07 30 PM We might consider making the whole render phase and maybe commit phase red but that should maybe reserved for actual errors. The "Blocked" phase really represents the `setState` and so will have the stack trace of the first update. --- .../src/ReactFiberPerformanceTrack.js | 19 +++++++++----- .../src/ReactFiberRootScheduler.js | 26 +++++++++++++------ .../src/ReactFiberWorkLoop.js | 9 +++---- .../src/ReactProfilerTimer.js | 12 +++++++++ 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 61bfd5cf7f844..6e0ebc4e980af 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -276,11 +276,15 @@ export function logBlockingStart( eventTime: number, eventType: null | string, eventIsRepeat: boolean, + isSpawnedUpdate: boolean, renderStartTime: number, lanes: Lanes, ): void { if (supportsUserTiming) { reusableLaneDevToolDetails.track = 'Blocking'; + // If a blocking update was spawned within render or an effect, that's considered a cascading render. + // If you have a second blocking update within the same event, that suggests multiple flushSync or + // setState in a microtask which is also considered a cascade. if (eventTime > 0 && eventType !== null) { // Log the time from the event timeStamp until we called setState. reusableLaneDevToolDetails.color = eventIsRepeat @@ -295,14 +299,17 @@ export function logBlockingStart( } if (updateTime > 0) { // Log the time from when we called setState until we started rendering. - reusableLaneDevToolDetails.color = includesOnlyHydrationOrOffscreenLanes( - lanes, - ) - ? 'tertiary-light' - : 'primary-light'; + reusableLaneDevToolDetails.color = isSpawnedUpdate + ? 'error' + : includesOnlyHydrationOrOffscreenLanes(lanes) + ? 'tertiary-light' + : 'primary-light'; reusableLaneOptions.start = updateTime; reusableLaneOptions.end = renderStartTime; - performance.measure('Blocked', reusableLaneOptions); + performance.measure( + isSpawnedUpdate ? 'Cascade' : 'Blocked', + reusableLaneOptions, + ); } } } diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index dcaadc5a6ef18..e3791f4c8dcff 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -128,12 +128,12 @@ export function ensureRootIsScheduled(root: FiberRoot): void { // We're inside an `act` scope. if (!didScheduleMicrotask_act) { didScheduleMicrotask_act = true; - scheduleImmediateTask(processRootScheduleInMicrotask); + scheduleImmediateRootScheduleTask(); } } else { if (!didScheduleMicrotask) { didScheduleMicrotask = true; - scheduleImmediateTask(processRootScheduleInMicrotask); + scheduleImmediateRootScheduleTask(); } } @@ -229,13 +229,17 @@ function flushSyncWorkAcrossRoots_impl( isFlushingWork = false; } -function processRootScheduleInMicrotask() { +function processRootScheduleInImmediateTask() { if (enableProfilerTimer && enableComponentPerformanceTrack) { // Track the currently executing event if there is one so we can ignore this // event when logging events. trackSchedulerEvent(); } + processRootScheduleInMicrotask(); +} + +function processRootScheduleInMicrotask() { // This function is always called inside a microtask. It should never be // called synchronously. didScheduleMicrotask = false; @@ -558,7 +562,7 @@ function cancelCallback(callbackNode: mixed) { } } -function scheduleImmediateTask(cb: () => mixed) { +function scheduleImmediateRootScheduleTask() { if (__DEV__ && ReactSharedInternals.actQueue !== null) { // Special case: Inside an `act` scope, we push microtasks to the fake `act` // callback queue. This is because we currently support calling `act` @@ -566,7 +570,7 @@ function scheduleImmediateTask(cb: () => mixed) { // that you always await the result so that the microtasks have a chance to // run. But it hasn't happened yet. ReactSharedInternals.actQueue.push(() => { - cb(); + processRootScheduleInMicrotask(); return null; }); } @@ -588,14 +592,20 @@ function scheduleImmediateTask(cb: () => mixed) { // wrong semantically but it prevents an infinite loop. The bug is // Safari's, not ours, so we just do our best to not crash even though // the behavior isn't completely correct. - Scheduler_scheduleCallback(ImmediateSchedulerPriority, cb); + Scheduler_scheduleCallback( + ImmediateSchedulerPriority, + processRootScheduleInImmediateTask, + ); return; } - cb(); + processRootScheduleInMicrotask(); }); } else { // If microtasks are not supported, use Scheduler. - Scheduler_scheduleCallback(ImmediateSchedulerPriority, cb); + Scheduler_scheduleCallback( + ImmediateSchedulerPriority, + processRootScheduleInImmediateTask, + ); } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 8e75135ff6e08..133c91f518dfc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -236,6 +236,7 @@ import { blockingEventTime, blockingEventType, blockingEventIsRepeat, + blockingSpawnedUpdate, blockingSuspendedTime, transitionClampTime, transitionStartTime, @@ -1664,11 +1665,8 @@ export function flushSyncWork(): boolean { export function isAlreadyRendering(): boolean { // Used by the renderer to print a warning if certain APIs are called from - // the wrong context. - return ( - __DEV__ && - (executionContext & (RenderContext | CommitContext)) !== NoContext - ); + // the wrong context, and for profiling warnings. + return (executionContext & (RenderContext | CommitContext)) !== NoContext; } export function isInvalidExecutionContextForEventFunction(): boolean { @@ -1797,6 +1795,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { clampedEventTime, blockingEventType, blockingEventIsRepeat, + blockingSpawnedUpdate, renderStartTime, lanes, ); diff --git a/packages/react-reconciler/src/ReactProfilerTimer.js b/packages/react-reconciler/src/ReactProfilerTimer.js index d408ba0ff7bd0..d3bdf6f6a9c7f 100644 --- a/packages/react-reconciler/src/ReactProfilerTimer.js +++ b/packages/react-reconciler/src/ReactProfilerTimer.js @@ -30,6 +30,8 @@ import { enableComponentPerformanceTrack, } from 'shared/ReactFeatureFlags'; +import {isAlreadyRendering} from './ReactFiberWorkLoop'; + // Intentionally not named imports because Rollup would use dynamic dispatch for // CommonJS interop named imports. import * as Scheduler from 'scheduler'; @@ -50,6 +52,7 @@ export let blockingUpdateTime: number = -1.1; // First sync setState scheduled. export let blockingEventTime: number = -1.1; // Event timeStamp of the first setState. export let blockingEventType: null | string = null; // Event type of the first setState. export let blockingEventIsRepeat: boolean = false; +export let blockingSpawnedUpdate: boolean = false; export let blockingSuspendedTime: number = -1.1; // TODO: This should really be one per Transition lane. export let transitionClampTime: number = -0; @@ -78,6 +81,9 @@ export function startUpdateTimerByLane(lane: Lane): void { if (isSyncLane(lane) || isBlockingLane(lane)) { if (blockingUpdateTime < 0) { blockingUpdateTime = now(); + if (isAlreadyRendering()) { + blockingSpawnedUpdate = true; + } const newEventTime = resolveEventTimeStamp(); const newEventType = resolveEventType(); if ( @@ -85,6 +91,11 @@ export function startUpdateTimerByLane(lane: Lane): void { newEventType !== blockingEventType ) { blockingEventIsRepeat = false; + } else if (newEventType !== null) { + // If this is a second update in the same event, we treat it as a spawned update. + // This might be a microtask spawned from useEffect, multiple flushSync or + // a setState in a microtask spawned after the first setState. Regardless it's bad. + blockingSpawnedUpdate = true; } blockingEventTime = newEventTime; blockingEventType = newEventType; @@ -141,6 +152,7 @@ export function clearBlockingTimers(): void { blockingUpdateTime = -1.1; blockingSuspendedTime = -1.1; blockingEventIsRepeat = true; + blockingSpawnedUpdate = false; } export function startAsyncTransitionTimer(): void { From 0de1233fd180969f7ffdfc98151922f2466ceb1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Jan 2025 13:28:24 -0500 Subject: [PATCH 5/5] [Fiber] Mark error boundaries and commit phases when an error is thrown (#31876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This tracks commit phase errors and marks the component that errored as red. These also get the errors attached to the entry. Screenshot 2024-12-20 at 2 40 14 PM In the render phase I just mark the Error Boundary that caught the error. We don't have access to the actual error since it's locked behind closures in the update queue. We could probably expose that someway. Screenshot 2024-12-20 at 1 49 05 PM Follow ups: Since the Error Boundary doesn't commit its attempted render, we don't log those. If we did then maybe we should just mark the errored component like I do for the commit phase. We could potentially walk the list of errors and log the captured fibers and just log their entries as children. We could also potentially walk the uncommitted Fiber tree by stashing it somewhere or even getting it from the alternate. This could be done on Suspense boundaries too to track failed hydrations. --------- Co-authored-by: Ricky --- .../src/ReactFiberCommitWork.js | 117 ++++++++++++----- .../src/ReactFiberPerformanceTrack.js | 118 +++++++++++++++++- .../src/ReactFiberWorkLoop.js | 18 ++- .../src/ReactProfilerTimer.js | 43 +++++++ 4 files changed, 262 insertions(+), 34 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 43b726e981007..a244c65e3e886 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -98,6 +98,7 @@ import { Cloned, PerformedWork, ForceClientRender, + DidCapture, } from './ReactFiberFlags'; import { commitStartTime, @@ -107,14 +108,17 @@ import { resetComponentEffectTimers, pushComponentEffectStart, popComponentEffectStart, + pushComponentEffectErrors, + popComponentEffectErrors, componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, } from './ReactProfilerTimer'; import { logComponentRender, + logComponentErrored, logComponentEffect, - logSuspenseBoundaryClientRendered, } from './ReactFiberPerformanceTrack'; import {ConcurrentMode, NoMode, ProfileMode} from './ReactTypeOfMode'; import {deferHiddenCallbacks} from './ReactFiberClassUpdateQueue'; @@ -395,7 +399,7 @@ function commitLayoutEffectOnFiber( committedLanes: Lanes, ): void { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); // When updating this function, also update reappearLayoutEffects, which does // most of the same things when an offscreen tree goes from hidden -> visible. const flags = finishedWork.flags; @@ -631,10 +635,12 @@ function commitLayoutEffectOnFiber( componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function abortRootTransitions( @@ -1627,7 +1633,7 @@ function commitMutationEffectsOnFiber( lanes: Lanes, ) { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); const current = finishedWork.alternate; const flags = finishedWork.flags; @@ -2136,10 +2142,12 @@ function commitMutationEffectsOnFiber( componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function commitReconciliationEffects(finishedWork: Fiber) { @@ -2212,7 +2220,7 @@ function recursivelyTraverseLayoutEffects( export function disappearLayoutEffects(finishedWork: Fiber) { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: @@ -2285,10 +2293,12 @@ export function disappearLayoutEffects(finishedWork: Fiber) { componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) { @@ -2310,7 +2320,7 @@ export function reappearLayoutEffects( includeWorkInProgressEffects: boolean, ) { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); // Turn on layout effects in a tree that previously disappeared. const flags = finishedWork.flags; switch (finishedWork.tag) { @@ -2461,10 +2471,12 @@ export function reappearLayoutEffects( componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function recursivelyTraverseReappearLayoutEffects( @@ -2701,26 +2713,7 @@ function commitPassiveMountOnFiber( endTime: number, // Profiling-only. The start time of the next Fiber or root completion. ): void { const prevEffectStart = pushComponentEffectStart(); - - // If this component rendered in Profiling mode (DEV or in Profiler component) then log its - // render time. We do this after the fact in the passive effect to avoid the overhead of this - // getting in the way of the render characteristics and avoid the overhead of unwinding - // uncommitted renders. - if ( - enableProfilerTimer && - enableComponentPerformanceTrack && - (finishedWork.mode & ProfileMode) !== NoMode && - ((finishedWork.actualStartTime: any): number) > 0 && - (finishedWork.flags & PerformedWork) !== NoFlags - ) { - logComponentRender( - finishedWork, - ((finishedWork.actualStartTime: any): number), - endTime, - inHydratedSubtree, - ); - } - + const prevEffectErrors = pushComponentEffectErrors(); // When updating this function, also update reconnectPassiveEffects, which does // most of the same things when an offscreen tree goes from hidden -> visible, // or when toggling effects inside a hidden tree. @@ -2729,6 +2722,25 @@ function commitPassiveMountOnFiber( case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { + // If this component rendered in Profiling mode (DEV or in Profiler component) then log its + // render time. We do this after the fact in the passive effect to avoid the overhead of this + // getting in the way of the render characteristics and avoid the overhead of unwinding + // uncommitted renders. + if ( + enableProfilerTimer && + enableComponentPerformanceTrack && + (finishedWork.mode & ProfileMode) !== NoMode && + ((finishedWork.actualStartTime: any): number) > 0 && + (finishedWork.flags & PerformedWork) !== NoFlags + ) { + logComponentRender( + finishedWork, + ((finishedWork.actualStartTime: any): number), + endTime, + inHydratedSubtree, + ); + } + recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, @@ -2744,6 +2756,45 @@ function commitPassiveMountOnFiber( } break; } + case ClassComponent: { + // If this component rendered in Profiling mode (DEV or in Profiler component) then log its + // render time. We do this after the fact in the passive effect to avoid the overhead of this + // getting in the way of the render characteristics and avoid the overhead of unwinding + // uncommitted renders. + if ( + enableProfilerTimer && + enableComponentPerformanceTrack && + (finishedWork.mode & ProfileMode) !== NoMode && + ((finishedWork.actualStartTime: any): number) > 0 + ) { + if ((finishedWork.flags & DidCapture) !== NoFlags) { + logComponentErrored( + finishedWork, + ((finishedWork.actualStartTime: any): number), + endTime, + // TODO: The captured values are all hidden inside the updater/callback closures so + // we can't get to the errors but they're there so we should be able to log them. + [], + ); + } else if ((finishedWork.flags & PerformedWork) !== NoFlags) { + logComponentRender( + finishedWork, + ((finishedWork.actualStartTime: any): number), + endTime, + inHydratedSubtree, + ); + } + } + + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + endTime, + ); + break; + } case HostRoot: { const prevEffectDuration = pushNestedEffectDurations(); @@ -2891,7 +2942,7 @@ function commitPassiveMountOnFiber( // rendered boundary. Such as postpone. if (hydrationErrors !== null) { const startTime: number = (finishedWork.actualStartTime: any); - logSuspenseBoundaryClientRendered( + logComponentErrored( finishedWork, startTime, endTime, @@ -3074,10 +3125,12 @@ function commitPassiveMountOnFiber( componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function recursivelyTraverseReconnectPassiveEffects( @@ -3137,7 +3190,7 @@ export function reconnectPassiveEffects( endTime: number, // Profiling-only. The start time of the next Fiber or root completion. ) { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); // If this component rendered in Profiling mode (DEV or in Profiler component) then log its // render time. We do this after the fact in the passive effect to avoid the overhead of this // getting in the way of the render characteristics and avoid the overhead of unwinding @@ -3331,10 +3384,12 @@ export function reconnectPassiveEffects( componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function recursivelyTraverseAtomicPassiveEffects( @@ -3611,7 +3666,7 @@ function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void { function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: @@ -3696,10 +3751,12 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } function recursivelyTraverseDisconnectPassiveEffects(parentFiber: Fiber): void { @@ -3819,7 +3876,7 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( nearestMountedAncestor: Fiber | null, ): void { const prevEffectStart = pushComponentEffectStart(); - + const prevEffectErrors = pushComponentEffectErrors(); switch (current.tag) { case FunctionComponent: case ForwardRef: @@ -3946,10 +4003,12 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( componentEffectStartTime, componentEffectEndTime, componentEffectDuration, + componentEffectErrors, ); } popComponentEffectStart(prevEffectStart); + popComponentEffectErrors(prevEffectErrors); } export function invokeLayoutEffectMountInDEV(fiber: Fiber): void { diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 6e0ebc4e980af..ae48ed383413a 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -13,6 +13,8 @@ import type {Lanes} from './ReactFiberLane'; import type {CapturedValue} from './ReactCapturedValue'; +import {SuspenseComponent} from './ReactWorkTags'; + import getComponentNameFromFiber from './getComponentNameFromFiber'; import { @@ -159,13 +161,64 @@ export function logComponentRender( } } -export function logSuspenseBoundaryClientRendered( +export function logComponentErrored( + fiber: Fiber, + startTime: number, + endTime: number, + errors: Array>, +): void { + if (supportsUserTiming) { + const name = getComponentNameFromFiber(fiber); + if (name === null) { + // Skip + return; + } + const properties = []; + if (__DEV__) { + for (let i = 0; i < errors.length; i++) { + const capturedValue = errors[i]; + const error = capturedValue.value; + const message = + typeof error === 'object' && + error !== null && + typeof error.message === 'string' + ? // eslint-disable-next-line react-internal/safe-string-coercion + String(error.message) + : // eslint-disable-next-line react-internal/safe-string-coercion + String(error); + properties.push(['Error', message]); + } + } + performance.measure(name, { + start: startTime, + end: endTime, + detail: { + devtools: { + color: 'error', + track: COMPONENTS_TRACK, + tooltipText: + fiber.tag === SuspenseComponent + ? 'Hydration failed' + : 'Error boundary caught an error', + properties, + }, + }, + }); + } +} + +function logComponentEffectErrored( fiber: Fiber, startTime: number, endTime: number, errors: Array>, ): void { if (supportsUserTiming) { + const name = getComponentNameFromFiber(fiber); + if (name === null) { + // Skip + return; + } const properties = []; if (__DEV__) { for (let i = 0; i < errors.length; i++) { @@ -182,14 +235,14 @@ export function logSuspenseBoundaryClientRendered( properties.push(['Error', message]); } } - performance.measure('Suspense', { + performance.measure(name, { start: startTime, end: endTime, detail: { devtools: { color: 'error', track: COMPONENTS_TRACK, - tooltipText: 'Hydration failed', + tooltipText: 'A lifecycle or effect errored', properties, }, }, @@ -202,7 +255,12 @@ export function logComponentEffect( startTime: number, endTime: number, selfTime: number, + errors: null | Array>, ): void { + if (errors !== null) { + logComponentEffectErrored(fiber, startTime, endTime, errors); + return; + } const name = getComponentNameFromFiber(fiber); if (name === null) { // Skip @@ -534,7 +592,54 @@ export function logSuspendedCommitPhase( } } -export function logCommitPhase(startTime: number, endTime: number): void { +export function logCommitErrored( + startTime: number, + endTime: number, + errors: Array>, + passive: boolean, +): void { + if (supportsUserTiming) { + const properties = []; + if (__DEV__) { + for (let i = 0; i < errors.length; i++) { + const capturedValue = errors[i]; + const error = capturedValue.value; + const message = + typeof error === 'object' && + error !== null && + typeof error.message === 'string' + ? // eslint-disable-next-line react-internal/safe-string-coercion + String(error.message) + : // eslint-disable-next-line react-internal/safe-string-coercion + String(error); + properties.push(['Error', message]); + } + } + performance.measure('Errored', { + start: startTime, + end: endTime, + detail: { + devtools: { + color: 'error', + track: reusableLaneDevToolDetails.track, + trackGroup: LANES_TRACK_GROUP, + tooltipText: passive ? 'Remaining Effects Errored' : 'Commit Errored', + properties, + }, + }, + }); + } +} + +export function logCommitPhase( + startTime: number, + endTime: number, + errors: null | Array>, +): void { + if (errors !== null) { + logCommitErrored(startTime, endTime, errors, false); + return; + } if (supportsUserTiming) { reusableLaneDevToolDetails.color = 'secondary-dark'; reusableLaneOptions.start = startTime; @@ -562,7 +667,12 @@ export function logPaintYieldPhase( export function logPassiveCommitPhase( startTime: number, endTime: number, + errors: null | Array>, ): void { + if (errors !== null) { + logCommitErrored(startTime, endTime, errors, true); + return; + } if (supportsUserTiming) { reusableLaneDevToolDetails.color = 'secondary-dark'; reusableLaneOptions.start = startTime; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 133c91f518dfc..ded26ed6e2bfb 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -253,6 +253,7 @@ import { renderStartTime, commitStartTime, commitEndTime, + commitErrors, recordRenderTime, recordCommitTime, recordCommitEndTime, @@ -264,6 +265,8 @@ import { yieldStartTime, yieldReason, startPingTimerByLanes, + recordEffectError, + resetCommitErrors, } from './ReactProfilerTimer'; // DEV stuff @@ -3340,6 +3343,7 @@ function commitRootImpl( if (enableProfilerTimer) { // Mark the current commit time to be shared by all Profilers in this // batch. This enables them to be grouped later. + resetCommitErrors(); recordCommitTime(); if (enableComponentPerformanceTrack) { if (suspendedCommitReason === SUSPENDED_COMMIT) { @@ -3433,6 +3437,7 @@ function commitRootImpl( ? completedRenderEndTime : commitStartTime, commitEndTime, + commitErrors, ); } @@ -3722,6 +3727,7 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { let passiveEffectStartTime = 0; if (enableProfilerTimer && enableComponentPerformanceTrack) { + resetCommitErrors(); passiveEffectStartTime = now(); logPaintYieldPhase( commitEndTime, @@ -3758,7 +3764,11 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { if (enableProfilerTimer && enableComponentPerformanceTrack) { const passiveEffectsEndTime = now(); - logPassiveCommitPhase(passiveEffectStartTime, passiveEffectsEndTime); + logPassiveCommitPhase( + passiveEffectStartTime, + passiveEffectsEndTime, + commitErrors, + ); finalizeRender(lanes, passiveEffectsEndTime); } @@ -3842,6 +3852,9 @@ function captureCommitPhaseErrorOnRoot( error: mixed, ) { const errorInfo = createCapturedValueAtFiber(error, sourceFiber); + if (enableProfilerTimer && enableComponentPerformanceTrack) { + recordEffectError(errorInfo); + } const update = createRootErrorUpdate( rootFiber.stateNode, errorInfo, @@ -3883,6 +3896,9 @@ export function captureCommitPhaseError( !isAlreadyFailedLegacyErrorBoundary(instance)) ) { const errorInfo = createCapturedValueAtFiber(error, sourceFiber); + if (enableProfilerTimer && enableComponentPerformanceTrack) { + recordEffectError(errorInfo); + } const update = createClassErrorUpdate((SyncLane: Lane)); const root = enqueueUpdate(fiber, update, (SyncLane: Lane)); if (root !== null) { diff --git a/packages/react-reconciler/src/ReactProfilerTimer.js b/packages/react-reconciler/src/ReactProfilerTimer.js index d3bdf6f6a9c7f..bc289bcab6321 100644 --- a/packages/react-reconciler/src/ReactProfilerTimer.js +++ b/packages/react-reconciler/src/ReactProfilerTimer.js @@ -12,6 +12,9 @@ import type {Fiber} from './ReactInternalTypes'; import type {SuspendedReason} from './ReactFiberWorkLoop'; import type {Lane, Lanes} from './ReactFiberLane'; + +import type {CapturedValue} from './ReactCapturedValue'; + import { isTransitionLane, isBlockingLane, @@ -41,11 +44,13 @@ const {unstable_now: now} = Scheduler; export let renderStartTime: number = -0; export let commitStartTime: number = -0; export let commitEndTime: number = -0; +export let commitErrors: null | Array> = null; export let profilerStartTime: number = -1.1; export let profilerEffectDuration: number = -0; export let componentEffectDuration: number = -0; export let componentEffectStartTime: number = -1.1; export let componentEffectEndTime: number = -1.1; +export let componentEffectErrors: null | Array> = null; export let blockingClampTime: number = -0; export let blockingUpdateTime: number = -1.1; // First sync setState scheduled. @@ -282,6 +287,26 @@ export function popComponentEffectStart(prevEffectStart: number): void { } } +export function pushComponentEffectErrors(): null | Array< + CapturedValue, +> { + if (!enableProfilerTimer || !enableProfilerCommitHooks) { + return null; + } + const prevErrors = componentEffectErrors; + componentEffectErrors = null; + return prevErrors; +} + +export function popComponentEffectErrors( + prevErrors: null | Array>, +): void { + if (!enableProfilerTimer || !enableProfilerCommitHooks) { + return; + } + componentEffectErrors = prevErrors; +} + /** * Tracks whether the current update was a nested/cascading update (scheduled from a layout effect). * @@ -416,6 +441,24 @@ export function recordEffectDuration(fiber: Fiber): void { } } +export function recordEffectError(errorInfo: CapturedValue): void { + if (!enableProfilerTimer || !enableProfilerCommitHooks) { + return; + } + if (componentEffectErrors === null) { + componentEffectErrors = []; + } + componentEffectErrors.push(errorInfo); + if (commitErrors === null) { + commitErrors = []; + } + commitErrors.push(errorInfo); +} + +export function resetCommitErrors(): void { + commitErrors = null; +} + export function startEffectTimer(): void { if (!enableProfilerTimer || !enableProfilerCommitHooks) { return;