Skip to content

Commit 4fd039f

Browse files
acdliteacdlite
committed
Enqueue and update childLanes in same function
During a setState, the childLanes are updated immediately, even if a render is already in progress. This can lead to subtle concurrency bugs, so the plan is to wait until the in-progress render has finished before updating the childLanes, to prevent subtle concurrency bugs. As a step toward that change, when scheduling an update, we should not update the childLanes directly, but instead defer to the ReactConcurrentUpdates module to do it at the appropriate time. This makes markUpdateLaneFromFiberToRoot a private function that is only called from the ReactConcurrentUpdates module.
1 parent 3a267b4 commit 4fd039f

16 files changed

+416
-294
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,13 @@ import {
228228
} from './ReactFiber.new';
229229
import {
230230
retryDehydratedSuspenseBoundary,
231-
markUpdateLaneFromFiberToRoot,
232231
scheduleUpdateOnFiber,
233232
renderDidSuspendDelayIfPossible,
234233
markSkippedUpdateLanes,
235234
getWorkInProgressRoot,
236235
pushRenderLanes,
237236
} from './ReactFiberWorkLoop.new';
237+
import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates.new';
238238
import {setWorkInProgressVersion} from './ReactMutableSource.new';
239239
import {pushCacheProvider, CacheContext} from './ReactFiberCacheComponent.new';
240240
import {createCapturedValue} from './ReactCapturedValue';
@@ -2627,7 +2627,7 @@ function updateDehydratedSuspenseComponent(
26272627
suspenseState.retryLane = attemptHydrationAtLane;
26282628
// TODO: Ideally this would inherit the event time of the current render
26292629
const eventTime = NoTimestamp;
2630-
markUpdateLaneFromFiberToRoot(current, attemptHydrationAtLane);
2630+
enqueueConcurrentRenderForLane(current, attemptHydrationAtLane);
26312631
scheduleUpdateOnFiber(
26322632
root,
26332633
current,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,13 @@ import {
228228
} from './ReactFiber.old';
229229
import {
230230
retryDehydratedSuspenseBoundary,
231-
markUpdateLaneFromFiberToRoot,
232231
scheduleUpdateOnFiber,
233232
renderDidSuspendDelayIfPossible,
234233
markSkippedUpdateLanes,
235234
getWorkInProgressRoot,
236235
pushRenderLanes,
237236
} from './ReactFiberWorkLoop.old';
237+
import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates.old';
238238
import {setWorkInProgressVersion} from './ReactMutableSource.old';
239239
import {pushCacheProvider, CacheContext} from './ReactFiberCacheComponent.old';
240240
import {createCapturedValue} from './ReactCapturedValue';
@@ -2627,7 +2627,7 @@ function updateDehydratedSuspenseComponent(
26272627
suspenseState.retryLane = attemptHydrationAtLane;
26282628
// TODO: Ideally this would inherit the event time of the current render
26292629
const eventTime = NoTimestamp;
2630-
markUpdateLaneFromFiberToRoot(current, attemptHydrationAtLane);
2630+
enqueueConcurrentRenderForLane(current, attemptHydrationAtLane);
26312631
scheduleUpdateOnFiber(
26322632
root,
26332633
current,

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ import {
7272
requestEventTime,
7373
requestUpdateLane,
7474
scheduleUpdateOnFiber,
75-
markUpdateLaneFromFiberToRoot,
7675
} from './ReactFiberWorkLoop.new';
7776
import {logForceUpdateScheduled, logStateUpdateScheduled} from './DebugTracing';
7877
import {
@@ -216,8 +215,7 @@ const classComponentUpdater = {
216215
update.callback = callback;
217216
}
218217

219-
enqueueUpdate(fiber, update, lane);
220-
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
218+
const root = enqueueUpdate(fiber, update, lane);
221219
if (root !== null) {
222220
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
223221
entangleTransitions(root, fiber, lane);
@@ -252,8 +250,7 @@ const classComponentUpdater = {
252250
update.callback = callback;
253251
}
254252

255-
enqueueUpdate(fiber, update, lane);
256-
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
253+
const root = enqueueUpdate(fiber, update, lane);
257254
if (root !== null) {
258255
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
259256
entangleTransitions(root, fiber, lane);
@@ -287,8 +284,7 @@ const classComponentUpdater = {
287284
update.callback = callback;
288285
}
289286

290-
enqueueUpdate(fiber, update, lane);
291-
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
287+
const root = enqueueUpdate(fiber, update, lane);
292288
if (root !== null) {
293289
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
294290
entangleTransitions(root, fiber, lane);

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ import {
7272
requestEventTime,
7373
requestUpdateLane,
7474
scheduleUpdateOnFiber,
75-
markUpdateLaneFromFiberToRoot,
7675
} from './ReactFiberWorkLoop.old';
7776
import {logForceUpdateScheduled, logStateUpdateScheduled} from './DebugTracing';
7877
import {
@@ -216,8 +215,7 @@ const classComponentUpdater = {
216215
update.callback = callback;
217216
}
218217

219-
enqueueUpdate(fiber, update, lane);
220-
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
218+
const root = enqueueUpdate(fiber, update, lane);
221219
if (root !== null) {
222220
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
223221
entangleTransitions(root, fiber, lane);
@@ -252,8 +250,7 @@ const classComponentUpdater = {
252250
update.callback = callback;
253251
}
254252

255-
enqueueUpdate(fiber, update, lane);
256-
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
253+
const root = enqueueUpdate(fiber, update, lane);
257254
if (root !== null) {
258255
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
259256
entangleTransitions(root, fiber, lane);
@@ -287,8 +284,7 @@ const classComponentUpdater = {
287284
update.callback = callback;
288285
}
289286

290-
enqueueUpdate(fiber, update, lane);
291-
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
287+
const root = enqueueUpdate(fiber, update, lane);
292288
if (root !== null) {
293289
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
294290
entangleTransitions(root, fiber, lane);

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

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ import {
109109
markSkippedUpdateLanes,
110110
isUnsafeClassRenderPhaseUpdate,
111111
} from './ReactFiberWorkLoop.new';
112-
import {pushConcurrentUpdateQueue} from './ReactFiberConcurrentUpdates.new';
112+
import {
113+
enqueueConcurrentClassUpdate,
114+
unsafe_markUpdateLaneFromFiberToRoot,
115+
} from './ReactFiberConcurrentUpdates.new';
113116
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new';
114117

115118
import assign from 'shared/assign';
@@ -214,42 +217,15 @@ export function enqueueUpdate<State>(
214217
fiber: Fiber,
215218
update: Update<State>,
216219
lane: Lane,
217-
) {
220+
): FiberRoot | null {
218221
const updateQueue = fiber.updateQueue;
219222
if (updateQueue === null) {
220223
// Only occurs if the fiber has been unmounted.
221-
return;
224+
return null;
222225
}
223226

224227
const sharedQueue: SharedQueue<State> = (updateQueue: any).shared;
225228

226-
if (isUnsafeClassRenderPhaseUpdate(fiber)) {
227-
// This is an unsafe render phase update. Add directly to the update
228-
// queue so we can process it immediately during the current render.
229-
const pending = sharedQueue.pending;
230-
if (pending === null) {
231-
// This is the first update. Create a circular list.
232-
update.next = update;
233-
} else {
234-
update.next = pending.next;
235-
pending.next = update;
236-
}
237-
sharedQueue.pending = update;
238-
} else {
239-
const interleaved = sharedQueue.interleaved;
240-
if (interleaved === null) {
241-
// This is the first update. Create a circular list.
242-
update.next = update;
243-
// At the end of the current render, this queue's interleaved updates will
244-
// be transferred to the pending queue.
245-
pushConcurrentUpdateQueue(sharedQueue);
246-
} else {
247-
update.next = interleaved.next;
248-
interleaved.next = update;
249-
}
250-
sharedQueue.interleaved = update;
251-
}
252-
253229
if (__DEV__) {
254230
if (
255231
currentlyProcessingQueue === sharedQueue &&
@@ -264,6 +240,28 @@ export function enqueueUpdate<State>(
264240
didWarnUpdateInsideUpdate = true;
265241
}
266242
}
243+
244+
if (isUnsafeClassRenderPhaseUpdate(fiber)) {
245+
// This is an unsafe render phase update. Add directly to the update
246+
// queue so we can process it immediately during the current render.
247+
const pending = sharedQueue.pending;
248+
if (pending === null) {
249+
// This is the first update. Create a circular list.
250+
update.next = update;
251+
} else {
252+
update.next = pending.next;
253+
pending.next = update;
254+
}
255+
sharedQueue.pending = update;
256+
257+
// Update the childLanes even though we're most likely already rendering
258+
// this fiber. This is for backwards compatibility in the case where you
259+
// update a different component during render phase than the one that is
260+
// currently renderings (a pattern that is accompanied by a warning).
261+
return unsafe_markUpdateLaneFromFiberToRoot(fiber, lane);
262+
} else {
263+
return enqueueConcurrentClassUpdate(fiber, sharedQueue, update, lane);
264+
}
267265
}
268266

269267
export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) {

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

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ import {
109109
markSkippedUpdateLanes,
110110
isUnsafeClassRenderPhaseUpdate,
111111
} from './ReactFiberWorkLoop.old';
112-
import {pushConcurrentUpdateQueue} from './ReactFiberConcurrentUpdates.old';
112+
import {
113+
enqueueConcurrentClassUpdate,
114+
unsafe_markUpdateLaneFromFiberToRoot,
115+
} from './ReactFiberConcurrentUpdates.old';
113116
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.old';
114117

115118
import assign from 'shared/assign';
@@ -214,42 +217,15 @@ export function enqueueUpdate<State>(
214217
fiber: Fiber,
215218
update: Update<State>,
216219
lane: Lane,
217-
) {
220+
): FiberRoot | null {
218221
const updateQueue = fiber.updateQueue;
219222
if (updateQueue === null) {
220223
// Only occurs if the fiber has been unmounted.
221-
return;
224+
return null;
222225
}
223226

224227
const sharedQueue: SharedQueue<State> = (updateQueue: any).shared;
225228

226-
if (isUnsafeClassRenderPhaseUpdate(fiber)) {
227-
// This is an unsafe render phase update. Add directly to the update
228-
// queue so we can process it immediately during the current render.
229-
const pending = sharedQueue.pending;
230-
if (pending === null) {
231-
// This is the first update. Create a circular list.
232-
update.next = update;
233-
} else {
234-
update.next = pending.next;
235-
pending.next = update;
236-
}
237-
sharedQueue.pending = update;
238-
} else {
239-
const interleaved = sharedQueue.interleaved;
240-
if (interleaved === null) {
241-
// This is the first update. Create a circular list.
242-
update.next = update;
243-
// At the end of the current render, this queue's interleaved updates will
244-
// be transferred to the pending queue.
245-
pushConcurrentUpdateQueue(sharedQueue);
246-
} else {
247-
update.next = interleaved.next;
248-
interleaved.next = update;
249-
}
250-
sharedQueue.interleaved = update;
251-
}
252-
253229
if (__DEV__) {
254230
if (
255231
currentlyProcessingQueue === sharedQueue &&
@@ -264,6 +240,28 @@ export function enqueueUpdate<State>(
264240
didWarnUpdateInsideUpdate = true;
265241
}
266242
}
243+
244+
if (isUnsafeClassRenderPhaseUpdate(fiber)) {
245+
// This is an unsafe render phase update. Add directly to the update
246+
// queue so we can process it immediately during the current render.
247+
const pending = sharedQueue.pending;
248+
if (pending === null) {
249+
// This is the first update. Create a circular list.
250+
update.next = update;
251+
} else {
252+
update.next = pending.next;
253+
pending.next = update;
254+
}
255+
sharedQueue.pending = update;
256+
257+
// Update the childLanes even though we're most likely already rendering
258+
// this fiber. This is for backwards compatibility in the case where you
259+
// update a different component during render phase than the one that is
260+
// currently renderings (a pattern that is accompanied by a warning).
261+
return unsafe_markUpdateLaneFromFiberToRoot(fiber, lane);
262+
} else {
263+
return enqueueConcurrentClassUpdate(fiber, sharedQueue, update, lane);
264+
}
267265
}
268266

269267
export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) {

0 commit comments

Comments
 (0)