Skip to content

Commit

Permalink
Always push updates to interleaved queue first
Browse files Browse the repository at this point in the history 
Interleaves updates (updates that are scheduled while another render
is already is progress) go into a special queue that isn't applied until
the end of the current render. They are transferred to the "real" queue
at the beginning of the next render.

Currently we check during `setState` whether an update should go
directly onto the real queue or onto the special interleaved queue. The
logic is subtle and it can lead to bugs if you mess it up, as in facebook#24400.

Instead, this changes it to always go onto the interleaved queue. The
behavior is the same but the logic is simpler.

As a further step, we can also wait to update the `childLanes` until
the end of the current render. I'll do this in the next step.
  • Loading branch information
@acdlite
acdlite committed Jun 3, 2022
1 parent 8dc5de3 commit 0841d66
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 126 deletions.
33 changes: 10 additions & 23 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ import {
requestUpdateLane,
requestEventTime,
markSkippedUpdateLanes,
isInterleavedUpdate,
} from './ReactFiberWorkLoop.new';

import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
Expand Down Expand Up @@ -2337,30 +2336,18 @@ function enqueueUpdate<S, A>(
update: Update<S, A>,
lane: Lane,
) {
if (isInterleavedUpdate(fiber, lane)) {
const interleaved = queue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
const interleaved = queue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(queue);
} else {
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
}

function entangleTransitionUpdate<S, A>(
Expand Down
33 changes: 10 additions & 23 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ import {
requestUpdateLane,
requestEventTime,
markSkippedUpdateLanes,
isInterleavedUpdate,
} from './ReactFiberWorkLoop.old';

import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
Expand Down Expand Up @@ -2337,30 +2336,18 @@ function enqueueUpdate<S, A>(
update: Update<S, A>,
lane: Lane,
) {
if (isInterleavedUpdate(fiber, lane)) {
const interleaved = queue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
const interleaved = queue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(queue);
} else {
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
}

function entangleTransitionUpdate<S, A>(
Expand Down
4 changes: 0 additions & 4 deletions packages/react-reconciler/src/ReactFiberInterleavedUpdates.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ export function pushInterleavedQueue(
}
}

export function hasInterleavedUpdates() {
return interleavedQueues !== null;
}

export function enqueueInterleavedUpdates() {
// Transfer the interleaved updates onto the main queue. Each queue has a
// `pending` field and an `interleaved` field. When they are not null, they
Expand Down
4 changes: 0 additions & 4 deletions packages/react-reconciler/src/ReactFiberInterleavedUpdates.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ export function pushInterleavedQueue(
}
}

export function hasInterleavedUpdates() {
return interleavedQueues !== null;
}

export function enqueueInterleavedUpdates() {
// Transfer the interleaved updates onto the main queue. Each queue has a
// `pending` field and an `interleaved` field. When they are not null, they
Expand Down
33 changes: 9 additions & 24 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,7 @@ import {
pop as popFromStack,
createCursor,
} from './ReactFiberStack.new';
import {
enqueueInterleavedUpdates,
hasInterleavedUpdates,
} from './ReactFiberInterleavedUpdates.new';
import {enqueueInterleavedUpdates} from './ReactFiberInterleavedUpdates.new';

import {
markNestedUpdateScheduled,
Expand Down Expand Up @@ -606,8 +603,6 @@ export function scheduleUpdateOnFiber(
}

if (root === workInProgressRoot) {
// TODO: Consolidate with `isInterleavedUpdate` check

// Received an update to a tree that's in the middle of rendering. Mark
// that there was an interleaved update work on this root. Unless the
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
Expand Down Expand Up @@ -721,25 +716,15 @@ function markUpdateLaneFromFiberToRoot(
}
}

export function isInterleavedUpdate(fiber: Fiber, lane: Lane) {
export function isUnsafeClassRenderPhaseUpdate(fiber: Fiber) {
// Check if this is a render phase update. Only called by class components,
// which special (deprecated) behavior for UNSAFE_componentWillReceive props.
return (
// TODO: Optimize slightly by comparing to root that fiber belongs to.
// Requires some refactoring. Not a big deal though since it's rare for
// concurrent apps to have more than a single root.
(workInProgressRoot !== null ||
// If the interleaved updates queue hasn't been cleared yet, then
// we should treat this as an interleaved update, too. This is also a
// defensive coding measure in case a new update comes in between when
// rendering has finished and when the interleaved updates are transferred
// to the main queue.
hasInterleavedUpdates()) &&
(fiber.mode & ConcurrentMode) !== NoMode &&
// If this is a render phase update (i.e. UNSAFE_componentWillReceiveProps),
// then don't treat this as an interleaved update. This pattern is
// accompanied by a warning but we haven't fully deprecated it yet. We can
// remove once the deferRenderPhaseUpdateToNextBatch flag is enabled.
(deferRenderPhaseUpdateToNextBatch ||
(executionContext & RenderContext) === NoContext)
// TODO: Remove outdated deferRenderPhaseUpdateToNextBatch experiment. We
// decided not to enable it.
(!deferRenderPhaseUpdateToNextBatch ||
(fiber.mode & ConcurrentMode) === NoMode) &&
(executionContext & RenderContext) !== NoContext
);
}

Expand Down
33 changes: 9 additions & 24 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,7 @@ import {
pop as popFromStack,
createCursor,
} from './ReactFiberStack.old';
import {
enqueueInterleavedUpdates,
hasInterleavedUpdates,
} from './ReactFiberInterleavedUpdates.old';
import {enqueueInterleavedUpdates} from './ReactFiberInterleavedUpdates.old';

import {
markNestedUpdateScheduled,
Expand Down Expand Up @@ -606,8 +603,6 @@ export function scheduleUpdateOnFiber(
}

if (root === workInProgressRoot) {
// TODO: Consolidate with `isInterleavedUpdate` check

// Received an update to a tree that's in the middle of rendering. Mark
// that there was an interleaved update work on this root. Unless the
// `deferRenderPhaseUpdateToNextBatch` flag is off and this is a render
Expand Down Expand Up @@ -721,25 +716,15 @@ function markUpdateLaneFromFiberToRoot(
}
}

export function isInterleavedUpdate(fiber: Fiber, lane: Lane) {
export function isUnsafeClassRenderPhaseUpdate(fiber: Fiber) {
// Check if this is a render phase update. Only called by class components,
// which special (deprecated) behavior for UNSAFE_componentWillReceive props.
return (
// TODO: Optimize slightly by comparing to root that fiber belongs to.
// Requires some refactoring. Not a big deal though since it's rare for
// concurrent apps to have more than a single root.
(workInProgressRoot !== null ||
// If the interleaved updates queue hasn't been cleared yet, then
// we should treat this as an interleaved update, too. This is also a
// defensive coding measure in case a new update comes in between when
// rendering has finished and when the interleaved updates are transferred
// to the main queue.
hasInterleavedUpdates()) &&
(fiber.mode & ConcurrentMode) !== NoMode &&
// If this is a render phase update (i.e. UNSAFE_componentWillReceiveProps),
// then don't treat this as an interleaved update. This pattern is
// accompanied by a warning but we haven't fully deprecated it yet. We can
// remove once the deferRenderPhaseUpdateToNextBatch flag is enabled.
(deferRenderPhaseUpdateToNextBatch ||
(executionContext & RenderContext) === NoContext)
// TODO: Remove outdated deferRenderPhaseUpdateToNextBatch experiment. We
// decided not to enable it.
(!deferRenderPhaseUpdateToNextBatch ||
(fiber.mode & ConcurrentMode) === NoMode) &&
(executionContext & RenderContext) !== NoContext
);
}

Expand Down
26 changes: 14 additions & 12 deletions packages/react-reconciler/src/ReactUpdateQueue.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags
import {StrictLegacyMode} from './ReactTypeOfMode';
import {
markSkippedUpdateLanes,
isInterleavedUpdate,
isUnsafeClassRenderPhaseUpdate,
} from './ReactFiberWorkLoop.new';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new';
Expand Down Expand Up @@ -223,7 +223,19 @@ export function enqueueUpdate<State>(

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

if (isInterleavedUpdate(fiber, lane)) {
if (isUnsafeClassRenderPhaseUpdate(fiber)) {
// This is an unsafe render phase update. Add directly to the update
// queue so we can process it immediately during the current render.
const pending = sharedQueue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
sharedQueue.pending = update;
} else {
const interleaved = sharedQueue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
Expand All @@ -236,16 +248,6 @@ export function enqueueUpdate<State>(
interleaved.next = update;
}
sharedQueue.interleaved = update;
} else {
const pending = sharedQueue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
sharedQueue.pending = update;
}

if (__DEV__) {
Expand Down
26 changes: 14 additions & 12 deletions packages/react-reconciler/src/ReactUpdateQueue.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags
import {StrictLegacyMode} from './ReactTypeOfMode';
import {
markSkippedUpdateLanes,
isInterleavedUpdate,
isUnsafeClassRenderPhaseUpdate,
} from './ReactFiberWorkLoop.old';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.old';
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.old';
Expand Down Expand Up @@ -223,7 +223,19 @@ export function enqueueUpdate<State>(

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

if (isInterleavedUpdate(fiber, lane)) {
if (isUnsafeClassRenderPhaseUpdate(fiber)) {
// This is an unsafe render phase update. Add directly to the update
// queue so we can process it immediately during the current render.
const pending = sharedQueue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
sharedQueue.pending = update;
} else {
const interleaved = sharedQueue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
Expand All @@ -236,16 +248,6 @@ export function enqueueUpdate<State>(
interleaved.next = update;
}
sharedQueue.interleaved = update;
} else {
const pending = sharedQueue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
sharedQueue.pending = update;
}

if (__DEV__) {
Expand Down

0 comments on commit 0841d66

Please sign in to comment.