Skip to content

Commit

Permalink
Fix infinite loop if unmemoized val passed to uDV
Browse files Browse the repository at this point in the history
The current implementation of useDeferredValue will spawn a new
render any time the input value is different from the previous one. So
if you pass an unmemoized value (like an inline object), it will never
stop spawning new renders.

The fix is to only defer during an urgent render. If we're already
inside a transition, retry, offscreen, or other non-urgen render, then
we can use the latest value.
  • Loading branch information
acdlite committed Apr 1, 2022
1 parent ebd7ff6 commit dc73506
Show file tree
Hide file tree
Showing 5 changed files with 364 additions and 64 deletions.
94 changes: 62 additions & 32 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Expand Up @@ -47,6 +47,7 @@ import {
NoLanes,
isSubsetOfLanes,
includesBlockingLane,
includesOnlyNonUrgentLanes,
mergeLanes,
removeLanes,
intersectLanes,
Expand Down Expand Up @@ -86,6 +87,7 @@ import {
requestEventTime,
markSkippedUpdateLanes,
isInterleavedUpdate,
getSpawnedLane,
} from './ReactFiberWorkLoop.new';

import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
Expand Down Expand Up @@ -1929,45 +1931,73 @@ function updateMemo<T>(
}

function mountDeferredValue<T>(value: T): T {
const [prevValue, setValue] = mountState(value);
mountEffect(() => {
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = {};
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
}
}, [value]);
return prevValue;
const hook = mountWorkInProgressHook();
hook.memoizedState = value;
return value;
}

function updateDeferredValue<T>(value: T): T {
const [prevValue, setValue] = updateState(value);
updateEffect(() => {
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = {};
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
}
}, [value]);
return prevValue;
const hook = updateWorkInProgressHook();
const resolvedCurrentHook: Hook = (currentHook: any);
const prevValue: T = resolvedCurrentHook.memoizedState;
return updateDeferredValueImpl(hook, prevValue, value);
}

function rerenderDeferredValue<T>(value: T): T {
const [prevValue, setValue] = rerenderState(value);
updateEffect(() => {
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = {};
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
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 = getSpawnedLane();
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;
}
}, [value]);
return prevValue;

// 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;
}
}

function startTransition(setPending, callback, options) {
Expand Down
94 changes: 62 additions & 32 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Expand Up @@ -47,6 +47,7 @@ import {
NoLanes,
isSubsetOfLanes,
includesBlockingLane,
includesOnlyNonUrgentLanes,
mergeLanes,
removeLanes,
intersectLanes,
Expand Down Expand Up @@ -86,6 +87,7 @@ import {
requestEventTime,
markSkippedUpdateLanes,
isInterleavedUpdate,
getSpawnedLane,
} from './ReactFiberWorkLoop.old';

import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
Expand Down Expand Up @@ -1929,45 +1931,73 @@ function updateMemo<T>(
}

function mountDeferredValue<T>(value: T): T {
const [prevValue, setValue] = mountState(value);
mountEffect(() => {
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = {};
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
}
}, [value]);
return prevValue;
const hook = mountWorkInProgressHook();
hook.memoizedState = value;
return value;
}

function updateDeferredValue<T>(value: T): T {
const [prevValue, setValue] = updateState(value);
updateEffect(() => {
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = {};
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
}
}, [value]);
return prevValue;
const hook = updateWorkInProgressHook();
const resolvedCurrentHook: Hook = (currentHook: any);
const prevValue: T = resolvedCurrentHook.memoizedState;
return updateDeferredValueImpl(hook, prevValue, value);
}

function rerenderDeferredValue<T>(value: T): T {
const [prevValue, setValue] = rerenderState(value);
updateEffect(() => {
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = {};
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
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 = getSpawnedLane();
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;
}
}, [value]);
return prevValue;

// 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;
}
}

function startTransition(setPending, callback, options) {
Expand Down
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Expand Up @@ -312,6 +312,9 @@ let workInProgressRootConcurrentErrors: Array<mixed> | null = null;
// These are errors that we recovered from without surfacing them to the UI.
// We will log them once the tree commits.
let workInProgressRootRecoverableErrors: Array<mixed> | null = null;
// The lane to use if this render spawns additional work. This used by
// useDeferredValue; it should maybe be used for retries, too.
let workInProgressRootSpawnedLane: Lane = NoLane;

// The most recent time we committed a fallback. This lets us ensure a train
// model where we don't commit new loading states in too quick succession.
Expand Down Expand Up @@ -510,6 +513,10 @@ function requestRetryLane(fiber: Fiber) {
return claimNextRetryLane();
}

export function getSpawnedLane(): Lane {
return workInProgressRootSpawnedLane;
}

export function scheduleUpdateOnFiber(
fiber: Fiber,
lane: Lane,
Expand Down Expand Up @@ -1479,6 +1486,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
workInProgressRootPingedLanes = NoLanes;
workInProgressRootConcurrentErrors = null;
workInProgressRootRecoverableErrors = null;
workInProgressRootSpawnedLane = claimNextRetryLane();

enqueueInterleavedUpdates();

Expand Down
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Expand Up @@ -312,6 +312,9 @@ let workInProgressRootConcurrentErrors: Array<mixed> | null = null;
// These are errors that we recovered from without surfacing them to the UI.
// We will log them once the tree commits.
let workInProgressRootRecoverableErrors: Array<mixed> | null = null;
// The lane to use if this render spawns additional work. This used by
// useDeferredValue; it should maybe be used for retries, too.
let workInProgressRootSpawnedLane: Lane = NoLane;

// The most recent time we committed a fallback. This lets us ensure a train
// model where we don't commit new loading states in too quick succession.
Expand Down Expand Up @@ -510,6 +513,10 @@ function requestRetryLane(fiber: Fiber) {
return claimNextRetryLane();
}

export function getSpawnedLane(): Lane {
return workInProgressRootSpawnedLane;
}

export function scheduleUpdateOnFiber(
fiber: Fiber,
lane: Lane,
Expand Down Expand Up @@ -1479,6 +1486,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
workInProgressRootPingedLanes = NoLanes;
workInProgressRootConcurrentErrors = null;
workInProgressRootRecoverableErrors = null;
workInProgressRootSpawnedLane = claimNextRetryLane();

enqueueInterleavedUpdates();

Expand Down

0 comments on commit dc73506

Please sign in to comment.