From 6f0dc94df99a4ec0f703b304765e4aab684bb640 Mon Sep 17 00:00:00 2001 From: Noah Lemen Date: Wed, 6 Mar 2024 15:29:22 -0500 Subject: [PATCH] cleanup enableProfilerNestedUpdateScheduledHook --- .../src/ReactFiberWorkLoop.js | 36 -- .../__tests__/ReactProfiler-test.internal.js | 456 ------------------ packages/shared/ReactFeatureFlags.js | 4 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.native.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../forks/ReactFeatureFlags.www-dynamic.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 2 - 10 files changed, 504 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 98e164e0b221f..74b276bfffd69 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -30,7 +30,6 @@ import { enableProfilerTimer, enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, - enableProfilerNestedUpdateScheduledHook, enableDebugTracing, enableSchedulingProfiler, disableSchedulerTimeoutInWorkLoop, @@ -107,7 +106,6 @@ import { ForwardRef, MemoComponent, SimpleMemoComponent, - Profiler, HostComponent, HostHoistable, HostSingleton, @@ -581,10 +579,6 @@ let hasUncaughtError = false; let firstUncaughtError = null; let legacyErrorBoundariesThatAlreadyFailed: Set | null = null; -// Only used when enableProfilerNestedUpdateScheduledHook is true; -// to track which root is currently committing layout effects. -let rootCommittingMutationOrLayoutEffects: FiberRoot | null = null; - let rootDoesHavePassiveEffects: boolean = false; let rootWithPendingPassiveEffects: FiberRoot | null = null; let pendingPassiveEffectsLanes: Lanes = NoLanes; @@ -807,26 +801,6 @@ export function scheduleUpdateOnFiber( warnIfUpdatesNotWrappedWithActDEV(fiber); - if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) { - if ( - (executionContext & CommitContext) !== NoContext && - root === rootCommittingMutationOrLayoutEffects - ) { - if (fiber.mode & ProfileMode) { - let current: null | Fiber = fiber; - while (current !== null) { - if (current.tag === Profiler) { - const {id, onNestedUpdateScheduled} = current.memoizedProps; - if (typeof onNestedUpdateScheduled === 'function') { - onNestedUpdateScheduled(id); - } - } - current = current.return; - } - } - } - } - if (enableTransitionTracing) { const transition = ReactCurrentBatchConfig.transition; if (transition !== null && transition.name != null) { @@ -2938,12 +2912,6 @@ function commitRootImpl( recordCommitTime(); } - if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) { - // Track the root here, rather than in commitLayoutEffects(), because of ref setters. - // Updates scheduled during ref detachment should also be flagged. - rootCommittingMutationOrLayoutEffects = root; - } - // The next phase is the mutation phase, where we mutate the host tree. commitMutationEffects(root, finishedWork, lanes); @@ -2982,10 +2950,6 @@ function commitRootImpl( markLayoutEffectsStopped(); } - if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) { - rootCommittingMutationOrLayoutEffects = null; - } - // Tell Scheduler to yield at the end of the frame, so the browser has an // opportunity to paint. requestPaint(); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index fd42561393101..0f4155a43209f 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -26,7 +26,6 @@ function loadModules({ enableProfilerTimer = true, enableProfilerCommitHooks = true, enableProfilerNestedUpdatePhase = true, - enableProfilerNestedUpdateScheduledHook = false, replayFailedUnitOfWorkWithInvokeGuardedCallback = false, useNoopRenderer = false, } = {}) { @@ -36,8 +35,6 @@ function loadModules({ ReactFeatureFlags.enableProfilerCommitHooks = enableProfilerCommitHooks; ReactFeatureFlags.enableProfilerNestedUpdatePhase = enableProfilerNestedUpdatePhase; - ReactFeatureFlags.enableProfilerNestedUpdateScheduledHook = - enableProfilerNestedUpdateScheduledHook; ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = replayFailedUnitOfWorkWithInvokeGuardedCallback; @@ -2319,456 +2316,3 @@ describe(`onPostCommit`, () => { expect(call[3]).toBe(11221221); // commit start time (before mutations or effects) }); }); - -describe(`onNestedUpdateScheduled`, () => { - beforeEach(() => { - jest.resetModules(); - - loadModules({ - enableProfilerNestedUpdateScheduledHook: true, - useNoopRenderer: true, - }); - }); - - it('is not called when the legacy render API is used to schedule an update', () => { - const onNestedUpdateScheduled = jest.fn(); - - ReactNoop.renderLegacySyncRoot( - -
initial
- , - ); - - ReactNoop.renderLegacySyncRoot( - -
update
- , - ); - - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - it('is not called when the root API is used to schedule an update', () => { - const onNestedUpdateScheduled = jest.fn(); - - ReactNoop.render( - -
initial
- , - ); - - ReactNoop.render( - -
update
- , - ); - - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - it('is called when a function component schedules an update during a layout effect', async () => { - function Component() { - const [didMount, setDidMount] = React.useState(false); - React.useLayoutEffect(() => { - setDidMount(true); - }, []); - Scheduler.log(`Component:${didMount}`); - return didMount; - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:false', 'Component:true']); - expect(onNestedUpdateScheduled).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduled.mock.calls[0][0]).toBe('test'); - }); - - it('is called when a function component schedules a batched update during a layout effect', async () => { - function Component() { - const [didMount, setDidMount] = React.useState(false); - React.useLayoutEffect(() => { - ReactNoop.batchedUpdates(() => { - setDidMount(true); - }); - }, []); - Scheduler.log(`Component:${didMount}`); - return didMount; - } - - const onNestedUpdateScheduled = jest.fn(); - const onRender = jest.fn(); - - ReactNoop.render( - - - , - ); - await waitForAll(['Component:false', 'Component:true']); - - expect(onRender).toHaveBeenCalledTimes(2); - expect(onRender.mock.calls[0][1]).toBe('mount'); - expect(onRender.mock.calls[1][1]).toBe('nested-update'); - - expect(onNestedUpdateScheduled).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduled.mock.calls[0][0]).toBe('root'); - }); - - it('bubbles up and calls all ancestor Profilers', async () => { - function Component() { - const [didMount, setDidMount] = React.useState(false); - React.useLayoutEffect(() => { - setDidMount(true); - }, []); - Scheduler.log(`Component:${didMount}`); - return didMount; - } - const onNestedUpdateScheduledOne = jest.fn(); - const onNestedUpdateScheduledTwo = jest.fn(); - const onNestedUpdateScheduledThree = jest.fn(); - - await act(() => { - ReactNoop.render( - - - <> - - - - - , - ); - }); - - assertLog(['Component:false', 'Component:true']); - expect(onNestedUpdateScheduledOne).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduledOne.mock.calls[0][0]).toBe('one'); - expect(onNestedUpdateScheduledTwo).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduledTwo.mock.calls[0][0]).toBe('two'); - expect(onNestedUpdateScheduledThree).not.toHaveBeenCalled(); - }); - - it('is not called when an update is scheduled for another doort during a layout effect', async () => { - const setStateRef = React.createRef(null); - - function ComponentRootOne() { - const [state, setState] = React.useState(false); - setStateRef.current = setState; - Scheduler.log(`ComponentRootOne:${state}`); - return state; - } - - function ComponentRootTwo() { - React.useLayoutEffect(() => { - setStateRef.current(true); - }, []); - Scheduler.log('ComponentRootTwo'); - return null; - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.renderToRootWithID( - - - , - 1, - ); - - ReactNoop.renderToRootWithID( - - - , - 2, - ); - }); - - assertLog([ - 'ComponentRootOne:false', - 'ComponentRootTwo', - 'ComponentRootOne:true', - ]); - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - it('is not called when a function component schedules an update during a passive effect', async () => { - function Component() { - const [didMount, setDidMount] = React.useState(false); - React.useEffect(() => { - setDidMount(true); - }, []); - Scheduler.log(`Component:${didMount}`); - return didMount; - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:false', 'Component:true']); - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - it('is not called when a function component schedules an update outside of render', async () => { - const updateFnRef = React.createRef(null); - - function Component() { - const [state, setState] = React.useState(false); - updateFnRef.current = () => setState(true); - Scheduler.log(`Component:${state}`); - return state; - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - assertLog(['Component:false']); - - await act(() => { - updateFnRef.current(); - }); - assertLog(['Component:true']); - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - it('it is not called when a component schedules an update during render', async () => { - function Component() { - const [state, setState] = React.useState(false); - if (state === false) { - setState(true); - } - Scheduler.log(`Component:${state}`); - return state; - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:false', 'Component:true']); - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - it('it is called when a component schedules an update from a ref callback', async () => { - function Component({mountChild}) { - const [refAttached, setRefAttached] = React.useState(false); - const [refDetached, setRefDetached] = React.useState(false); - const refSetter = React.useCallback(ref => { - if (ref !== null) { - setRefAttached(true); - } else { - setRefDetached(true); - } - }, []); - Scheduler.log(`Component:${refAttached}:${refDetached}`); - return mountChild ?
: null; - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:false:false', 'Component:true:false']); - expect(onNestedUpdateScheduled).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduled.mock.calls[0][0]).toBe('test'); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:true:false', 'Component:true:true']); - expect(onNestedUpdateScheduled).toHaveBeenCalledTimes(2); - expect(onNestedUpdateScheduled.mock.calls[1][0]).toBe('test'); - }); - - it('is called when a class component schedules an update from the componentDidMount lifecycles', async () => { - class Component extends React.Component { - state = { - value: false, - }; - componentDidMount() { - this.setState({value: true}); - } - render() { - const {value} = this.state; - Scheduler.log(`Component:${value}`); - return value; - } - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:false', 'Component:true']); - expect(onNestedUpdateScheduled).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduled.mock.calls[0][0]).toBe('test'); - }); - - it('is called when a class component schedules an update from the componentDidUpdate lifecycles', async () => { - class Component extends React.Component { - state = { - nestedUpdateSheduled: false, - }; - componentDidUpdate(prevProps, prevState) { - if ( - this.props.scheduleNestedUpdate && - !this.state.nestedUpdateSheduled - ) { - this.setState({nestedUpdateSheduled: true}); - } - } - render() { - const {scheduleNestedUpdate} = this.props; - const {nestedUpdateSheduled} = this.state; - Scheduler.log( - `Component:${scheduleNestedUpdate}:${nestedUpdateSheduled}`, - ); - return nestedUpdateSheduled; - } - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - assertLog(['Component:false:false']); - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - - assertLog(['Component:true:false', 'Component:true:true']); - expect(onNestedUpdateScheduled).toHaveBeenCalledTimes(1); - expect(onNestedUpdateScheduled.mock.calls[0][0]).toBe('test'); - }); - - it('is not called when a class component schedules an update outside of render', async () => { - const updateFnRef = React.createRef(null); - - class Component extends React.Component { - state = { - value: false, - }; - render() { - const {value} = this.state; - updateFnRef.current = () => this.setState({value: true}); - Scheduler.log(`Component:${value}`); - return value; - } - } - - const onNestedUpdateScheduled = jest.fn(); - - await act(() => { - ReactNoop.render( - - - , - ); - }); - assertLog(['Component:false']); - - await act(() => { - updateFnRef.current(); - }); - assertLog(['Component:true']); - expect(onNestedUpdateScheduled).not.toHaveBeenCalled(); - }); - - // TODO Add hydration tests to ensure we don't have false positives called. -}); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 506f6746b98de..4f8fde4a262a6 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -277,10 +277,6 @@ export const enableUpdaterTracking = __PROFILE__; // Internal only. export const enableGetInspectorDataForInstanceInProduction = false; -// Profiler API accepts a function to be called when a nested update is scheduled. -// This callback accepts the component type (class instance or function) the update is scheduled for. -export const enableProfilerNestedUpdateScheduledHook = false; - export const consoleManagedByDevToolsDuringStrictMode = true; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index c78362e810e6a..150dad49f930a 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -36,7 +36,6 @@ export const enableSchedulingProfiler = __PROFILE__; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; -export const enableProfilerNestedUpdateScheduledHook = false; export const enableUpdaterTracking = __PROFILE__; export const enableCache = false; export const enableLegacyCache = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 1db47db31b857..8db2b9c4ddf57 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -18,7 +18,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; -export const enableProfilerNestedUpdateScheduledHook = false; export const enableUpdaterTracking = __PROFILE__; export const enableCache = false; export const enableLegacyCache = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 60e53b3fb3315..4d9dd958f1ca6 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -18,7 +18,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; -export const enableProfilerNestedUpdateScheduledHook = false; export const enableUpdaterTracking = false; export const enableCache = true; export const enableLegacyCache = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 5de35a8b21ee2..af2ebddbd0b10 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -18,7 +18,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; -export const enableProfilerNestedUpdateScheduledHook = false; export const enableUpdaterTracking = false; export const enableCache = true; export const enableLegacyCache = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index fbf88c8dbabac..0ec325a4997af 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -18,7 +18,6 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; -export const enableProfilerNestedUpdateScheduledHook = false; export const enableUpdaterTracking = false; export const enableCache = true; export const enableLegacyCache = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 39fc234de2ad6..5219e1ed13a75 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -17,7 +17,6 @@ export const disableInputAttributeSyncing = __VARIANT__; export const disableIEWorkarounds = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__; export const enableUseRefAccessWarning = __VARIANT__; -export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; export const enableLazyContextPropagation = __VARIANT__; export const forceConcurrentByDefaultForTesting = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 4d77c65f4017e..4d6bfa56fb8bd 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -48,8 +48,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; -export const enableProfilerNestedUpdateScheduledHook: boolean = - __PROFILE__ && dynamicFeatureFlags.enableProfilerNestedUpdateScheduledHook; export const enableUpdaterTracking = __PROFILE__; export const enableSuspenseAvoidThisFallback = true;