Skip to content

Commit c519bf6

Browse files
acdliteacdlite
committed
Fix infinite loop if unmemoized val passed to uDV
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.
1 parent ebd7ff6 commit c519bf6

File tree

5 files changed

+370
-64
lines changed

5 files changed

+370
-64
lines changed

‎packages/react-reconciler/src/ReactFiberHooks.new.js

Lines changed: 62 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
NoLanes,
4848
isSubsetOfLanes,
4949
includesBlockingLane,
50+
includesOnlyNonUrgentLanes,
5051
mergeLanes,
5152
removeLanes,
5253
intersectLanes,
@@ -86,6 +87,7 @@ import {
8687
requestEventTime,
8788
markSkippedUpdateLanes,
8889
isInterleavedUpdate,
90+
getSpawnedLane,
8991
} from './ReactFiberWorkLoop.new';
9092

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

19311933
function mountDeferredValue<T>(value: T): T {
1932-
const [prevValue, setValue] = mountState(value);
1933-
mountEffect(() => {
1934-
const prevTransition = ReactCurrentBatchConfig.transition;
1935-
ReactCurrentBatchConfig.transition = {};
1936-
try {
1937-
setValue(value);
1938-
} finally {
1939-
ReactCurrentBatchConfig.transition = prevTransition;
1940-
}
1941-
}, [value]);
1942-
return prevValue;
1934+
const hook = mountWorkInProgressHook();
1935+
hook.memoizedState = value;
1936+
return value;
19431937
}
19441938

19451939
function updateDeferredValue<T>(value: T): T {
1946-
const [prevValue, setValue] = updateState(value);
1947-
updateEffect(() => {
1948-
const prevTransition = ReactCurrentBatchConfig.transition;
1949-
ReactCurrentBatchConfig.transition = {};
1950-
try {
1951-
setValue(value);
1952-
} finally {
1953-
ReactCurrentBatchConfig.transition = prevTransition;
1954-
}
1955-
}, [value]);
1956-
return prevValue;
1940+
const hook = updateWorkInProgressHook();
1941+
const resolvedCurrentHook: Hook = (currentHook: any);
1942+
const prevValue: T = resolvedCurrentHook.memoizedState;
1943+
return updateDeferredValueImpl(hook, prevValue, value);
19571944
}
19581945

19591946
function rerenderDeferredValue<T>(value: T): T {
1960-
const [prevValue, setValue] = rerenderState(value);
1961-
updateEffect(() => {
1962-
const prevTransition = ReactCurrentBatchConfig.transition;
1963-
ReactCurrentBatchConfig.transition = {};
1964-
try {
1965-
setValue(value);
1966-
} finally {
1967-
ReactCurrentBatchConfig.transition = prevTransition;
1947+
const hook = updateWorkInProgressHook();
1948+
if (currentHook === null) {
1949+
// This is a rerender during a mount.
1950+
hook.memoizedState = value;
1951+
return value;
1952+
} else {
1953+
// This is a rerender during an update.
1954+
const prevValue: T = currentHook.memoizedState;
1955+
return updateDeferredValueImpl(hook, prevValue, value);
1956+
}
1957+
}
1958+
1959+
function updateDeferredValueImpl<T>(hook: Hook, prevValue: T, value: T): T {
1960+
const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes);
1961+
if (shouldDeferValue) {
1962+
// This is an urgent update. If the value has changed, keep using the
1963+
// previous value and spawn a deferred render to update it later.
1964+
1965+
if (!is(value, prevValue)) {
1966+
// Schedule a deferred render
1967+
const deferredLane = getSpawnedLane();
1968+
currentlyRenderingFiber.lanes = mergeLanes(
1969+
currentlyRenderingFiber.lanes,
1970+
deferredLane,
1971+
);
1972+
markSkippedUpdateLanes(deferredLane);
1973+
1974+
// Set this to true to indicate that the rendered value is inconsistent
1975+
// from the latest value. The name "baseState" doesn't really match how we
1976+
// use it because we're reusing a state hook field instead of creating a
1977+
// new one.
1978+
hook.baseState = true;
19681979
}
1969-
}, [value]);
1970-
return prevValue;
1980+
1981+
// Reuse the previous value
1982+
return prevValue;
1983+
} else {
1984+
// This is not an urgent update, so we can use the latest value regardless
1985+
// of what it is. No need to defer it.
1986+
1987+
// However, if we're currently inside a spawned render, then we need to mark
1988+
// this as an update to prevent the fiber from bailing out.
1989+
//
1990+
// `baseState` is true when the current value is different from the rendered
1991+
// value. The name doesn't really match how we use it because we're reusing
1992+
// a state hook field instead of creating a new one.
1993+
if (hook.baseState) {
1994+
// Flip this back to false.
1995+
hook.baseState = false;
1996+
markWorkInProgressReceivedUpdate();
1997+
}
1998+
1999+
return value;
2000+
}
19712001
}
19722002

19732003
function startTransition(setPending, callback, options) {

‎packages/react-reconciler/src/ReactFiberHooks.old.js

Lines changed: 62 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
NoLanes,
4848
isSubsetOfLanes,
4949
includesBlockingLane,
50+
includesOnlyNonUrgentLanes,
5051
mergeLanes,
5152
removeLanes,
5253
intersectLanes,
@@ -86,6 +87,7 @@ import {
8687
requestEventTime,
8788
markSkippedUpdateLanes,
8889
isInterleavedUpdate,
90+
getSpawnedLane,
8991
} from './ReactFiberWorkLoop.old';
9092

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

19311933
function mountDeferredValue<T>(value: T): T {
1932-
const [prevValue, setValue] = mountState(value);
1933-
mountEffect(() => {
1934-
const prevTransition = ReactCurrentBatchConfig.transition;
1935-
ReactCurrentBatchConfig.transition = {};
1936-
try {
1937-
setValue(value);
1938-
} finally {
1939-
ReactCurrentBatchConfig.transition = prevTransition;
1940-
}
1941-
}, [value]);
1942-
return prevValue;
1934+
const hook = mountWorkInProgressHook();
1935+
hook.memoizedState = value;
1936+
return value;
19431937
}
19441938

19451939
function updateDeferredValue<T>(value: T): T {
1946-
const [prevValue, setValue] = updateState(value);
1947-
updateEffect(() => {
1948-
const prevTransition = ReactCurrentBatchConfig.transition;
1949-
ReactCurrentBatchConfig.transition = {};
1950-
try {
1951-
setValue(value);
1952-
} finally {
1953-
ReactCurrentBatchConfig.transition = prevTransition;
1954-
}
1955-
}, [value]);
1956-
return prevValue;
1940+
const hook = updateWorkInProgressHook();
1941+
const resolvedCurrentHook: Hook = (currentHook: any);
1942+
const prevValue: T = resolvedCurrentHook.memoizedState;
1943+
return updateDeferredValueImpl(hook, prevValue, value);
19571944
}
19581945

19591946
function rerenderDeferredValue<T>(value: T): T {
1960-
const [prevValue, setValue] = rerenderState(value);
1961-
updateEffect(() => {
1962-
const prevTransition = ReactCurrentBatchConfig.transition;
1963-
ReactCurrentBatchConfig.transition = {};
1964-
try {
1965-
setValue(value);
1966-
} finally {
1967-
ReactCurrentBatchConfig.transition = prevTransition;
1947+
const hook = updateWorkInProgressHook();
1948+
if (currentHook === null) {
1949+
// This is a rerender during a mount.
1950+
hook.memoizedState = value;
1951+
return value;
1952+
} else {
1953+
// This is a rerender during an update.
1954+
const prevValue: T = currentHook.memoizedState;
1955+
return updateDeferredValueImpl(hook, prevValue, value);
1956+
}
1957+
}
1958+
1959+
function updateDeferredValueImpl<T>(hook: Hook, prevValue: T, value: T): T {
1960+
const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes);
1961+
if (shouldDeferValue) {
1962+
// This is an urgent update. If the value has changed, keep using the
1963+
// previous value and spawn a deferred render to update it later.
1964+
1965+
if (!is(value, prevValue)) {
1966+
// Schedule a deferred render
1967+
const deferredLane = getSpawnedLane();
1968+
currentlyRenderingFiber.lanes = mergeLanes(
1969+
currentlyRenderingFiber.lanes,
1970+
deferredLane,
1971+
);
1972+
markSkippedUpdateLanes(deferredLane);
1973+
1974+
// Set this to true to indicate that the rendered value is inconsistent
1975+
// from the latest value. The name "baseState" doesn't really match how we
1976+
// use it because we're reusing a state hook field instead of creating a
1977+
// new one.
1978+
hook.baseState = true;
19681979
}
1969-
}, [value]);
1970-
return prevValue;
1980+
1981+
// Reuse the previous value
1982+
return prevValue;
1983+
} else {
1984+
// This is not an urgent update, so we can use the latest value regardless
1985+
// of what it is. No need to defer it.
1986+
1987+
// However, if we're currently inside a spawned render, then we need to mark
1988+
// this as an update to prevent the fiber from bailing out.
1989+
//
1990+
// `baseState` is true when the current value is different from the rendered
1991+
// value. The name doesn't really match how we use it because we're reusing
1992+
// a state hook field instead of creating a new one.
1993+
if (hook.baseState) {
1994+
// Flip this back to false.
1995+
hook.baseState = false;
1996+
markWorkInProgressReceivedUpdate();
1997+
}
1998+
1999+
return value;
2000+
}
19712001
}
19722002

19732003
function startTransition(setPending, callback, options) {

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,9 @@ let workInProgressRootConcurrentErrors: Array<mixed> | null = null;
312312
// These are errors that we recovered from without surfacing them to the UI.
313313
// We will log them once the tree commits.
314314
let workInProgressRootRecoverableErrors: Array<mixed> | null = null;
315+
// The lane to use if this render spawns additional work. Used
316+
// by useDeferredValue.
317+
let workInProgressRootSpawnedLane: Lane = NoLane;
315318

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

516+
export function getSpawnedLane(): Lane {
517+
if (workInProgressRootSpawnedLane === NoLane) {
518+
workInProgressRootSpawnedLane = claimNextTransitionLane();
519+
}
520+
return workInProgressRootSpawnedLane;
521+
}
522+
513523
export function scheduleUpdateOnFiber(
514524
fiber: Fiber,
515525
lane: Lane,
@@ -1479,6 +1489,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
14791489
workInProgressRootPingedLanes = NoLanes;
14801490
workInProgressRootConcurrentErrors = null;
14811491
workInProgressRootRecoverableErrors = null;
1492+
workInProgressRootSpawnedLane = NoLane;
14821493

14831494
enqueueInterleavedUpdates();
14841495

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,9 @@ let workInProgressRootConcurrentErrors: Array<mixed> | null = null;
312312
// These are errors that we recovered from without surfacing them to the UI.
313313
// We will log them once the tree commits.
314314
let workInProgressRootRecoverableErrors: Array<mixed> | null = null;
315+
// The lane to use if this render spawns additional work. Used
316+
// by useDeferredValue.
317+
let workInProgressRootSpawnedLane: Lane = NoLane;
315318

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

516+
export function getSpawnedLane(): Lane {
517+
if (workInProgressRootSpawnedLane === NoLane) {
518+
workInProgressRootSpawnedLane = claimNextTransitionLane();
519+
}
520+
return workInProgressRootSpawnedLane;
521+
}
522+
513523
export function scheduleUpdateOnFiber(
514524
fiber: Fiber,
515525
lane: Lane,
@@ -1479,6 +1489,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
14791489
workInProgressRootPingedLanes = NoLanes;
14801490
workInProgressRootConcurrentErrors = null;
14811491
workInProgressRootRecoverableErrors = null;
1492+
workInProgressRootSpawnedLane = NoLane;
14821493

14831494
enqueueInterleavedUpdates();
14841495

0 commit comments

Comments
 (0)