diff --git a/packages/react-devtools-shared/src/__tests__/preprocessData-test.js b/packages/react-devtools-shared/src/__tests__/preprocessData-test.js index 880607ff1d76..72086a684cd4 100644 --- a/packages/react-devtools-shared/src/__tests__/preprocessData-test.js +++ b/packages/react-devtools-shared/src/__tests__/preprocessData-test.js @@ -1463,9 +1463,7 @@ describe('Timeline profiler', () => { expect(event.warning).toBe(null); }); - // This is temporarily disabled because the warning doesn't work - // with useDeferredValue - it.skip('should warn about long nested (state) updates during layout effects', async () => { + it('should warn about long nested (state) updates during layout effects', async () => { function Component() { const [didMount, setDidMount] = React.useState(false); Scheduler.unstable_yieldValue( @@ -1525,9 +1523,7 @@ describe('Timeline profiler', () => { ); }); - // This is temporarily disabled because the warning doesn't work - // with useDeferredValue - it.skip('should warn about long nested (forced) updates during layout effects', async () => { + it('should warn about long nested (forced) updates during layout effects', async () => { class Component extends React.Component { _didMount: boolean = false; componentDidMount() { @@ -1658,9 +1654,7 @@ describe('Timeline profiler', () => { }); }); - // This is temporarily disabled because the warning doesn't work - // with useDeferredValue - it.skip('should not warn about deferred value updates scheduled during commit phase', async () => { + it('should not warn about deferred value updates scheduled during commit phase', async () => { function Component() { const [value, setValue] = React.useState(0); const deferredValue = React.useDeferredValue(value); diff --git a/packages/react-devtools-timeline/src/import-worker/preprocessData.js b/packages/react-devtools-timeline/src/import-worker/preprocessData.js index 110b4e8923fb..258eef931a38 100644 --- a/packages/react-devtools-timeline/src/import-worker/preprocessData.js +++ b/packages/react-devtools-timeline/src/import-worker/preprocessData.js @@ -1124,10 +1124,7 @@ export default async function preprocessData( lane => profilerData.laneToLabelMap.get(lane) === 'Transition', ) ) { - // FIXME: This warning doesn't account for "nested updates" that are - // spawned by useDeferredValue. Disabling temporarily until we figure - // out the right way to handle this. - // schedulingEvent.warning = WARNING_STRINGS.NESTED_UPDATE; + schedulingEvent.warning = WARNING_STRINGS.NESTED_UPDATE; } } }); diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index b7799d1dedcc..6a3250383338 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -47,8 +47,6 @@ import { NoLanes, isSubsetOfLanes, includesBlockingLane, - includesOnlyNonUrgentLanes, - claimNextTransitionLane, mergeLanes, removeLanes, intersectLanes, @@ -1928,73 +1926,45 @@ function updateMemo( } function mountDeferredValue(value: T): T { - const hook = mountWorkInProgressHook(); - hook.memoizedState = value; - return value; + const [prevValue, setValue] = mountState(value); + mountEffect(() => { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = {}; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; + } + }, [value]); + return prevValue; } function updateDeferredValue(value: T): T { - const hook = updateWorkInProgressHook(); - const resolvedCurrentHook: Hook = (currentHook: any); - const prevValue: T = resolvedCurrentHook.memoizedState; - return updateDeferredValueImpl(hook, prevValue, value); + const [prevValue, setValue] = updateState(value); + updateEffect(() => { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = {}; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; + } + }, [value]); + return prevValue; } function rerenderDeferredValue(value: T): T { - const hook = updateWorkInProgressHook(); - if (currentHook === null) { - // This is a rerender during a mount. - hook.memoizedState = value; - return value; - } else { - // This is a rerender during an update. - const prevValue: T = currentHook.memoizedState; - return updateDeferredValueImpl(hook, prevValue, value); - } -} - -function updateDeferredValueImpl(hook: Hook, prevValue: T, value: T): T { - const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes); - if (shouldDeferValue) { - // This is an urgent update. If the value has changed, keep using the - // previous value and spawn a deferred render to update it later. - - if (!is(value, prevValue)) { - // Schedule a deferred render - const deferredLane = claimNextTransitionLane(); - currentlyRenderingFiber.lanes = mergeLanes( - currentlyRenderingFiber.lanes, - deferredLane, - ); - markSkippedUpdateLanes(deferredLane); - - // Set this to true to indicate that the rendered value is inconsistent - // from the latest value. The name "baseState" doesn't really match how we - // use it because we're reusing a state hook field instead of creating a - // new one. - hook.baseState = true; + const [prevValue, setValue] = rerenderState(value); + updateEffect(() => { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = {}; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; } - - // Reuse the previous value - return prevValue; - } else { - // This is not an urgent update, so we can use the latest value regardless - // of what it is. No need to defer it. - - // However, if we're currently inside a spawned render, then we need to mark - // this as an update to prevent the fiber from bailing out. - // - // `baseState` is true when the current value is different from the rendered - // value. The name doesn't really match how we use it because we're reusing - // a state hook field instead of creating a new one. - if (hook.baseState) { - // Flip this back to false. - hook.baseState = false; - markWorkInProgressReceivedUpdate(); - } - - return value; - } + }, [value]); + return prevValue; } function startTransition(setPending, callback, options) { diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 03360bd6e96c..5f2887c76d22 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -47,8 +47,6 @@ import { NoLanes, isSubsetOfLanes, includesBlockingLane, - includesOnlyNonUrgentLanes, - claimNextTransitionLane, mergeLanes, removeLanes, intersectLanes, @@ -1928,73 +1926,45 @@ function updateMemo( } function mountDeferredValue(value: T): T { - const hook = mountWorkInProgressHook(); - hook.memoizedState = value; - return value; + const [prevValue, setValue] = mountState(value); + mountEffect(() => { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = {}; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; + } + }, [value]); + return prevValue; } function updateDeferredValue(value: T): T { - const hook = updateWorkInProgressHook(); - const resolvedCurrentHook: Hook = (currentHook: any); - const prevValue: T = resolvedCurrentHook.memoizedState; - return updateDeferredValueImpl(hook, prevValue, value); + const [prevValue, setValue] = updateState(value); + updateEffect(() => { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = {}; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; + } + }, [value]); + return prevValue; } function rerenderDeferredValue(value: T): T { - const hook = updateWorkInProgressHook(); - if (currentHook === null) { - // This is a rerender during a mount. - hook.memoizedState = value; - return value; - } else { - // This is a rerender during an update. - const prevValue: T = currentHook.memoizedState; - return updateDeferredValueImpl(hook, prevValue, value); - } -} - -function updateDeferredValueImpl(hook: Hook, prevValue: T, value: T): T { - const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes); - if (shouldDeferValue) { - // This is an urgent update. If the value has changed, keep using the - // previous value and spawn a deferred render to update it later. - - if (!is(value, prevValue)) { - // Schedule a deferred render - const deferredLane = claimNextTransitionLane(); - currentlyRenderingFiber.lanes = mergeLanes( - currentlyRenderingFiber.lanes, - deferredLane, - ); - markSkippedUpdateLanes(deferredLane); - - // Set this to true to indicate that the rendered value is inconsistent - // from the latest value. The name "baseState" doesn't really match how we - // use it because we're reusing a state hook field instead of creating a - // new one. - hook.baseState = true; + const [prevValue, setValue] = rerenderState(value); + updateEffect(() => { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = {}; + try { + setValue(value); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; } - - // Reuse the previous value - return prevValue; - } else { - // This is not an urgent update, so we can use the latest value regardless - // of what it is. No need to defer it. - - // However, if we're currently inside a spawned render, then we need to mark - // this as an update to prevent the fiber from bailing out. - // - // `baseState` is true when the current value is different from the rendered - // value. The name doesn't really match how we use it because we're reusing - // a state hook field instead of creating a new one. - if (hook.baseState) { - // Flip this back to false. - hook.baseState = false; - markWorkInProgressReceivedUpdate(); - } - - return value; - } + }, [value]); + return prevValue; } function startTransition(setPending, callback, options) { diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js deleted file mode 100644 index 9cd07c066e4c..000000000000 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ /dev/null @@ -1,224 +0,0 @@ -/** - * Copyright (c) Facebook, 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. - */ - -'use strict'; - -let React; -let ReactNoop; -let Scheduler; -let act; -let startTransition; -let useDeferredValue; -let useMemo; -let useState; - -describe('ReactDeferredValue', () => { - beforeEach(() => { - jest.resetModules(); - - React = require('react'); - ReactNoop = require('react-noop-renderer'); - Scheduler = require('scheduler'); - act = require('jest-react').act; - startTransition = React.startTransition; - useDeferredValue = React.useDeferredValue; - useMemo = React.useMemo; - useState = React.useState; - }); - - function Text({text}) { - Scheduler.unstable_yieldValue(text); - return text; - } - - it('does not cause an infinite defer loop if the original value isn\t memoized', async () => { - function App({value}) { - // The object passed to useDeferredValue is never the same as the previous - // render. A naive implementation would endlessly spawn deferred renders. - const {value: deferredValue} = useDeferredValue({value}); - - const child = useMemo(() => , [ - value, - ]); - - const deferredChild = useMemo( - () => , - [deferredValue], - ); - - return ( -
-
{child}
-
{deferredChild}
-
- ); - } - - const root = ReactNoop.createRoot(); - - // Initial render - await act(async () => { - root.render(); - }); - expect(Scheduler).toHaveYielded(['Original: 1', 'Deferred: 1']); - - // If it's an urgent update, the value is deferred - await act(async () => { - root.render(); - - expect(Scheduler).toFlushUntilNextPaint(['Original: 2']); - // The deferred value updates in a separate render - expect(Scheduler).toFlushUntilNextPaint(['Deferred: 2']); - }); - expect(root).toMatchRenderedOutput( -
-
Original: 2
-
Deferred: 2
-
, - ); - - // But if it updates during a transition, it doesn't defer - await act(async () => { - startTransition(() => { - root.render(); - }); - // The deferred value updates in the same render as the original - expect(Scheduler).toFlushUntilNextPaint(['Original: 3', 'Deferred: 3']); - }); - expect(root).toMatchRenderedOutput( -
-
Original: 3
-
Deferred: 3
-
, - ); - }); - - it('does not defer during a transition', async () => { - function App({value}) { - const deferredValue = useDeferredValue(value); - - const child = useMemo(() => , [ - value, - ]); - - const deferredChild = useMemo( - () => , - [deferredValue], - ); - - return ( -
-
{child}
-
{deferredChild}
-
- ); - } - - const root = ReactNoop.createRoot(); - - // Initial render - await act(async () => { - root.render(); - }); - expect(Scheduler).toHaveYielded(['Original: 1', 'Deferred: 1']); - - // If it's an urgent update, the value is deferred - await act(async () => { - root.render(); - - expect(Scheduler).toFlushUntilNextPaint(['Original: 2']); - // The deferred value updates in a separate render - expect(Scheduler).toFlushUntilNextPaint(['Deferred: 2']); - }); - expect(root).toMatchRenderedOutput( -
-
Original: 2
-
Deferred: 2
-
, - ); - - // But if it updates during a transition, it doesn't defer - await act(async () => { - startTransition(() => { - root.render(); - }); - // The deferred value updates in the same render as the original - expect(Scheduler).toFlushUntilNextPaint(['Original: 3', 'Deferred: 3']); - }); - expect(root).toMatchRenderedOutput( -
-
Original: 3
-
Deferred: 3
-
, - ); - }); - - it("works if there's a render phase update", async () => { - function App({value: propValue}) { - const [value, setValue] = useState(null); - if (value !== propValue) { - setValue(propValue); - } - - const deferredValue = useDeferredValue(value); - - const child = useMemo(() => , [ - value, - ]); - - const deferredChild = useMemo( - () => , - [deferredValue], - ); - - return ( -
-
{child}
-
{deferredChild}
-
- ); - } - - const root = ReactNoop.createRoot(); - - // Initial render - await act(async () => { - root.render(); - }); - expect(Scheduler).toHaveYielded(['Original: 1', 'Deferred: 1']); - - // If it's an urgent update, the value is deferred - await act(async () => { - root.render(); - - expect(Scheduler).toFlushUntilNextPaint(['Original: 2']); - // The deferred value updates in a separate render - expect(Scheduler).toFlushUntilNextPaint(['Deferred: 2']); - }); - expect(root).toMatchRenderedOutput( -
-
Original: 2
-
Deferred: 2
-
, - ); - - // But if it updates during a transition, it doesn't defer - await act(async () => { - startTransition(() => { - root.render(); - }); - // The deferred value updates in the same render as the original - expect(Scheduler).toFlushUntilNextPaint(['Original: 3', 'Deferred: 3']); - }); - expect(root).toMatchRenderedOutput( -
-
Original: 3
-
Deferred: 3
-
, - ); - }); -});