Skip to content

Commit

Permalink
Revert "Fix infinite update loop that happens when an unmemoized valu…
Browse files Browse the repository at this point in the history 
…e is passed to useDeferredValue (#24247)"

This reverts commit f993ffc.
  • Loading branch information
@rickhanlonii
rickhanlonii committed Apr 20, 2022
1 parent 9ae80d6 commit f47e471
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 361 deletions.
12 changes: 3 additions & 9 deletions packages/react-devtools-shared/src/__tests__/preprocessData-test.js
View file
Expand Up @@ -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(
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions packages/react-devtools-timeline/src/import-worker/preprocessData.js
View file
Expand Up @@ -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;
}
}
});
Expand Down
94 changes: 32 additions & 62 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Expand Up @@ -47,8 +47,6 @@ import {
NoLanes,
isSubsetOfLanes,
includesBlockingLane,
includesOnlyNonUrgentLanes,
claimNextTransitionLane,
mergeLanes,
removeLanes,
intersectLanes,
Expand Down Expand Up @@ -1928,73 +1926,45 @@ function updateMemo<T>(
}

function mountDeferredValue<T>(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<T>(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<T>(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<T>(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) {
Expand Down
94 changes: 32 additions & 62 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
View file
Expand Up @@ -47,8 +47,6 @@ import {
NoLanes,
isSubsetOfLanes,
includesBlockingLane,
includesOnlyNonUrgentLanes,
claimNextTransitionLane,
mergeLanes,
removeLanes,
intersectLanes,
Expand Down Expand Up @@ -1928,73 +1926,45 @@ function updateMemo<T>(
}

function mountDeferredValue<T>(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<T>(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<T>(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<T>(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) {
Expand Down

0 comments on commit f47e471

Please sign in to comment.