Stop resetting outside of render

Author: eps1lon

packages/internal-test-utils/internalAct.js

@@ -249,8 +249,7 @@ export async function serverAct<T>(scope: () => Thenable<T>): Thenable<T> {
 
       // $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
       const j = jest;
-      // We have always one pending timer running that resets the Owner Stack limit.
-      if (j.getTimerCount() > 1) {
+      if (j.getTimerCount() > 0) {
         // There's a pending timer. Flush it now. We only do this in order to
         // force Suspense fallbacks to display; the fact that it's a timer
         // is an implementation detail. If there are other timers scheduled,

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -49,6 +49,10 @@ import {
   enableViewTransition,
   enableSwipeTransition,
 } from 'shared/ReactFeatureFlags';
+import {
+  startResettingOwnerStackLimit,
+  stopResettingOwnerStackLimit,
+} from 'shared/ReactOwnerStackReset';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import is from 'shared/objectIs';
 
@@ -1320,6 +1324,10 @@ function finishConcurrentRender(
     }
   }
 
+  if (__DEV__) {
+    stopResettingOwnerStackLimit();
+  }
+
   if (shouldForceFlushFallbacksInDEV()) {
     // We're inside an `act` scope. Commit immediately.
     commitRoot(
@@ -1984,6 +1992,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
   finishQueueingConcurrentUpdates();
 
   if (__DEV__) {
+    startResettingOwnerStackLimit();
+
     ReactStrictModeWarnings.discardPendingWarnings();
   }
 

packages/react-reconciler/src/__tests__/ReactOwnerStacks-test.js

@@ -104,10 +104,10 @@ describe('ReactOwnerStacks', () => {
         let i = 0;
         i <
         siblingsBeforeStackOne -
-          // One built-in JSX callsite for the unknown Owner Stack
-          1 -
-          // <App /> callsite
-          1 -
+          // JSX callsites before this are irrelevant since we always reset the limit
+          // when we start a render. Both <App /> and <UnknownOwner /> were created
+          // before we actually start rendering.
+          0 -
           // Stop so that OwnerStackOne will be right before cutoff
           1;
         i++
@@ -149,8 +149,7 @@ describe('ReactOwnerStacks', () => {
       stackOne: normalizeCodeLocInfo(stackOne),
       stackTwo: normalizeCodeLocInfo(stackTwo),
     }).toEqual({
-      // We continue resetting periodically.
-      pendingTimers: 1,
+      pendingTimers: 0,
       stackOne: '\n    in App (at **)',
       stackTwo: __VARIANT__
         ? // captured right after cutoff
@@ -176,8 +175,7 @@ describe('ReactOwnerStacks', () => {
       stackOne: normalizeCodeLocInfo(stackOne),
       stackTwo: normalizeCodeLocInfo(stackTwo),
     }).toEqual({
-      // We continue resetting periodically.
-      pendingTimers: 1,
+      pendingTimers: 0,
       // rendered everything before cutoff
       stackOne: '\n    in App (at **)',
       stackTwo: '\n    in App (at **)',
@@ -192,10 +190,10 @@ describe('ReactOwnerStacks', () => {
         let i = 0;
         i <
         siblingsBeforeStackOne -
-          // One built-in JSX callsite for the unknown Owner Stack
-          1 -
-          // <App /> callsite
-          1 -
+          // JSX callsites before this are irrelevant since we always reset the limit
+          // when we start a render. Both <App /> and <UnknownOwner /> were created
+          // before we actually start rendering.
+          0 -
           // Stop so that OwnerStackOne will be right before cutoff
           1;
         i++
@@ -273,8 +271,7 @@ describe('ReactOwnerStacks', () => {
       stackOne: normalizeCodeLocInfo(stackOne),
       stackTwo: normalizeCodeLocInfo(stackTwo),
     }).toEqual({
-      // 1 for periodically resetting the Owner Stack limit
-      pendingTimers: 1,
+      pendingTimers: 0,
       stackOne: '\n    in App (at **)',
       stackTwo:
         // captured after we reset the limit
@@ -295,10 +292,10 @@ describe('ReactOwnerStacks', () => {
         let i = 0;
         i <
         siblingsBeforeStackOne -
-          // One built-in JSX callsite for the unknown Owner Stack
-          1 -
-          // <App /> callsite
-          1 -
+          // JSX callsites before this are irrelevant since we always reset the limit
+          // when we start a render. Both <App /> and <UnknownOwner /> were created
+          // before we actually start rendering.
+          0 -
           // Stop so that OwnerStackOne will be right before cutoff
           1;
         i++
@@ -357,8 +354,7 @@ describe('ReactOwnerStacks', () => {
       stackTwo: normalizeCodeLocInfo(stackTwo),
     }).toEqual({
       // 1 for the timeout
-      // 1 for periodically resetting the Owner Stack limit
-      pendingTimers: 2,
+      pendingTimers: 1,
       stackOne: '\n    in App (at **)',
       stackTwo: undefined,
     });
@@ -372,8 +368,7 @@ describe('ReactOwnerStacks', () => {
       stackOne: normalizeCodeLocInfo(stackOne),
       stackTwo: normalizeCodeLocInfo(stackTwo),
     }).toEqual({
-      // 1 for periodically resetting the Owner Stack limit
-      pendingTimers: 1,
+      pendingTimers: 0,
       stackOne: '\n    in App (at **)',
       stackTwo:
         // captured after we reset the limit

packages/react-server/src/ReactFizzServer.js

@@ -133,6 +133,10 @@ import {
   callRenderInDEV,
 } from './ReactFizzCallUserSpace';
 
+import {
+  startResettingOwnerStackLimit,
+  stopResettingOwnerStackLimit,
+} from 'shared/ReactOwnerStackReset';
 import {
   getIteratorFn,
   ASYNC_ITERATOR,
@@ -4571,6 +4575,8 @@ export function performWork(request: Request): void {
   if (__DEV__) {
     prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack;
     ReactSharedInternals.getCurrentStack = getCurrentStackInDEV;
+
+    startResettingOwnerStackLimit();
   }
   const prevResumableState = currentResumableState;
   setCurrentResumableState(request.resumableState);
@@ -4591,10 +4597,13 @@ export function performWork(request: Request): void {
     fatalError(request, error, errorInfo, null);
   } finally {
     setCurrentResumableState(prevResumableState);
+
     ReactSharedInternals.H = prevDispatcher;
     ReactSharedInternals.A = prevAsyncDispatcher;
 
     if (__DEV__) {
+      stopResettingOwnerStackLimit();
+
       ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl;
     }
     if (prevDispatcher === HooksDispatcher) {

packages/react-server/src/ReactFlightServer.js

@@ -103,6 +103,10 @@ import {DefaultAsyncDispatcher} from './flight/ReactFlightAsyncDispatcher';
 import {resolveOwner, setCurrentOwner} from './flight/ReactFlightCurrentOwner';
 
 import {getOwnerStackByComponentInfoInDev} from 'shared/ReactComponentInfoStack';
+import {
+  startResettingOwnerStackLimit,
+  stopResettingOwnerStackLimit,
+} from 'shared/ReactOwnerStackReset';
 
 import {
   callComponentInDEV,
@@ -4058,6 +4062,10 @@ function performWork(request: Request): void {
   currentRequest = request;
   prepareToUseHooksForRequest(request);
 
+  if (__DEV__) {
+    startResettingOwnerStackLimit();
+  }
+
   const hadAbortableTasks = request.abortableTasks.size > 0;
   try {
     const pingedTasks = request.pingedTasks;
@@ -4080,6 +4088,10 @@ function performWork(request: Request): void {
     logRecoverableError(request, error, null);
     fatalError(request, error);
   } finally {
+    if (__DEV__) {
+      stopResettingOwnerStackLimit();
+    }
+
     ReactSharedInternals.H = prevDispatcher;
     resetHooksForRequest();
     currentRequest = prevRequest;

packages/react-server/src/__tests__/ReactFlightServer-test.js

@@ -73,10 +73,10 @@ describe('ReactFlight', () => {
         let i = 0;
         i <
         siblingsBeforeStackOne -
-          // One built-in JSX callsite for the unknown Owner Stack
-          1 -
-          // <App /> callsite
-          1 -
+          // JSX callsites before this are irrelevant since we always reset the limit
+          // when we start a render. Both <App /> and <UnknownOwner /> were created
+          // before we actually start rendering.
+          0 -
           // Stop so that OwnerStackOne will be right before cutoff
           1;
         i++
@@ -143,8 +143,7 @@ describe('ReactFlight', () => {
       stackOne: normalizeCodeLocInfo(stackOne),
       stackTwo: normalizeCodeLocInfo(stackTwo),
     }).toEqual({
-      // 1 for periodically resetting the Owner Stack limit
-      pendingTimers: 1,
+      pendingTimers: 0,
       stackOne: '\n    in App (at **)',
       stackTwo:
         // captured after we reset the limit

packages/react/src/ReactSharedInternalsClient.js

@@ -39,6 +39,7 @@ export type SharedStateClient = {
   // ReactDebugCurrentFrame
   getCurrentStack: null | (() => string),
 
+  // ReactOwnerStackReset
   recentlyCreatedOwnerStacks: 0,
 };
 
@@ -60,14 +61,7 @@ if (__DEV__) {
   ReactSharedInternals.thrownErrors = [];
   // Stack implementation injected by the current renderer.
   ReactSharedInternals.getCurrentStack = (null: null | (() => string));
-  // ReactOwnerStack
   ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
-  if (typeof setInterval === 'function') {
-    // TODO: Stop outside of rendering.
-    setInterval(() => {
-      ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
-    }, 1000);
-  }
 }
 
 export default ReactSharedInternals;

packages/react/src/ReactSharedInternalsServer.js

@@ -40,7 +40,7 @@ export type SharedStateServer = {
   // ReactDebugCurrentFrame
   getCurrentStack: null | (() => string),
 
-  //
+  // ReactOwnerStackReset
   recentlyCreatedOwnerStacks: 0,
 };
 
@@ -62,14 +62,7 @@ if (enableTaint) {
 if (__DEV__) {
   // Stack implementation injected by the current renderer.
   ReactSharedInternals.getCurrentStack = (null: null | (() => string));
-  // ReactOwnerStack
   ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
-  if (typeof setInterval === 'function') {
-    // TODO: Stop outside of rendering.
-    setInterval(() => {
-      ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
-    }, 1000);
-  }
 }
 
 export default ReactSharedInternals;

packages/shared/ReactOwnerStackReset.js

@@ -0,0 +1,52 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and its 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 ReactSharedInternals from 'shared/ReactSharedInternals';
+
+let resetOwnerStackIntervalId: mixed = null;
+
+export function startResettingOwnerStackLimit() {
+  if (__DEV__) {
+    ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
+
+    if (typeof setInterval === 'function') {
+      // Renderers might start in render but stop in commit.
+      // So we need to be resilient against start being called multiple times.
+      if (resetOwnerStackIntervalId === null) {
+        clearInterval((resetOwnerStackIntervalId: any));
+      }
+      resetOwnerStackIntervalId = setInterval(() => {
+        ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
+      }, 1000);
+    }
+  } else {
+    // These errors should never make it into a build so we don't need to encode them in codes.json
+    // eslint-disable-next-line react-internal/prod-error-codes
+    throw new Error(
+      'startResettingOwnerStackLimit should never be called in production mode. This is a bug in React.',
+    );
+  }
+}
+
+export function stopResettingOwnerStackLimit() {
+  if (__DEV__) {
+    if (typeof setInterval === 'function') {
+      if (resetOwnerStackIntervalId !== null) {
+        clearInterval((resetOwnerStackIntervalId: any));
+        resetOwnerStackIntervalId = null;
+      }
+    }
+  } else {
+    // These errors should never make it into a build so we don't need to encode them in codes.json
+    // eslint-disable-next-line react-internal/prod-error-codes
+    throw new Error(
+      'stopResettingOwnerStackLimit should never be called in production mode. This is a bug in React.',
+    );
+  }
+}