Skip to content

Commit

Permalink
Use frameAligned for DefaultUpdate
Browse files Browse the repository at this point in the history
  • Loading branch information
tyao1 committed Oct 12, 2022
1 parent 287acc7 commit 8cf2534
Show file tree
Hide file tree
Showing 14 changed files with 41 additions and 88 deletions.
5 changes: 0 additions & 5 deletions packages/jest-react/src/internalAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,6 @@ export function act<T>(scope: () => Thenable<T> | T): Thenable<T> {
let didFlushWork;
do {
didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();

// Flush scheduled rAF.
if (global.flushRequestAnimationFrameQueue) {
global.flushRequestAnimationFrameQueue();
}
} while (didFlushWork);
return {
then(resolve, reject) {
Expand Down
19 changes: 9 additions & 10 deletions packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import {
} from 'react-reconciler/src/ReactWorkTags';
import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';

import {UnknownEventPriority} from 'react-reconciler/src/ReactEventPriorities';
import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities';

// TODO: Remove this deep import when we delete the legacy root API
import {ConcurrentMode, NoMode} from 'react-reconciler/src/ReactTypeOfMode';
Expand Down Expand Up @@ -374,7 +374,7 @@ export function createTextInstance(
export function getCurrentEventPriority(): EventPriority {
const currentEvent = window.event;
if (currentEvent === undefined) {
return UnknownEventPriority;
return DefaultEventPriority;
}
return getEventPriority(currentEvent.type);
}
Expand Down Expand Up @@ -414,21 +414,21 @@ export const scheduleMicrotask: any =
.catch(handleErrorInNextTick)
: scheduleTimeout; // TODO: Determine the best fallback here.

// TODO: Fix these types
export const supportsFrameAlignedTask = true;

type FrameAlignedTask = {|
rafNode: number,
rafNode: AnimationFrameID,
schedulerNode: number,
task: function,
|};

let currentTask: FrameAlignedTask | null = null;
function performFrameAlignedWork() {
if (currentTask != null) {
const task = currentTask.task;
localCancelAnimationFrame(currentTask.id);
Scheduler.unstable_cancelCallback(currentTask.schedulerNode);
const constCurrentTask = currentTask;
if (constCurrentTask != null) {
const task = constCurrentTask.task;
localCancelAnimationFrame(constCurrentTask.rafNode);
Scheduler.unstable_cancelCallback(constCurrentTask.schedulerNode);
currentTask = null;
if (task != null) {
task();
Expand Down Expand Up @@ -461,9 +461,8 @@ export function scheduleFrameAlignedTask(task: any): any {
return currentTask;
}

export function cancelFrameAlignedTask(task: FrameAlignedTask) {
export function cancelFrameAlignedTask(task: any) {
Scheduler.unstable_cancelCallback(task.schedulerNode);
task.schedulerNode = null;
// We don't cancel the rAF in case it gets re-used later.
// But clear the task so if it fires and shouldn't run, it won't.
task.task = null;
Expand Down
24 changes: 11 additions & 13 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,8 @@ describe('ReactDOMFiberAsync', () => {
window.event = 'test';
setState(1);

// We should not schedule a rAF for default updates only.
expect(global.requestAnimationFrameQueue).toBe(null);
// We should schedule a rAF for default updates.
expect(global.requestAnimationFrameQueue.length).toBe(1);

window.event = undefined;
setState(2);
Expand Down Expand Up @@ -715,8 +715,8 @@ describe('ReactDOMFiberAsync', () => {
window.event = 'test';
setState(1);

// We should not schedule a rAF for default updates only.
expect(global.requestAnimationFrameQueue).toBe(null);
// We should schedule a rAF for default updates.
expect(global.requestAnimationFrameQueue.length).toBe(1);

window.event = undefined;
setState(2);
Expand Down Expand Up @@ -914,29 +914,27 @@ describe('ReactDOMFiberAsync', () => {
expect(Scheduler).toHaveYielded(['Count: 0']);

window.event = undefined;
console.log('set state');
setState(1);
//expect(global.requestAnimationFrameQueue.length).toBe(1);
global.flushRequestAnimationFrameQueue();
expect(Scheduler).toHaveYielded(['Count: 1']);

setState(2);
setThrowing(true);

expect(global.requestAnimationFrameQueue.length).toBe(1);
global.flushRequestAnimationFrameQueue();
expect(Scheduler).toHaveYielded(['Count: 2', 'suspending']);
expect(counterRef.current.textContent).toBe('Count: 1');

unsuspend();
setThrowing(false);

// Should not be scheduled in a rAF.
// Default update should be scheduled in a rAF.
window.event = 'test';
setThrowing(false);
setState(2);

// TODO: This should not yield
// global.flushRequestAnimationFrameQueue();
// expect(Scheduler).toHaveYielded([]);

expect(Scheduler).toFlushAndYield(['Count: 2']);
global.flushRequestAnimationFrameQueue();
expect(Scheduler).toHaveYielded(['Count: 2']);
expect(counterRef.current.textContent).toBe('Count: 2');
});
});
Expand Down
10 changes: 6 additions & 4 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,17 +392,19 @@ describe('ReactDOMRoot', () => {

expect(container.textContent).toEqual('a');

// Set an event so this isn't flushed synchronously as an unknown update.
window.event = 'test';

await act(async () => {
root.render(<Foo value="b" />);

expect(Scheduler).toHaveYielded(['a']);
expect(container.textContent).toEqual('a');

expect(Scheduler).toFlushAndYieldThrough(['b']);
if (gate(flags => flags.allowConcurrentByDefault)) {
if (
gate(
flags =>
flags.allowConcurrentByDefault && !flags.enableFrameEndScheduling,
)
) {
expect(container.textContent).toEqual('a');
} else {
expect(container.textContent).toEqual('b');
Expand Down
5 changes: 0 additions & 5 deletions packages/react-reconciler/src/ReactEventPriorities.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import {enableNewReconciler} from 'shared/ReactFeatureFlags';

import {
UnknownEventPriority as UnknownEventPriority_old,
DiscreteEventPriority as DiscreteEventPriority_old,
ContinuousEventPriority as ContinuousEventPriority_old,
DefaultEventPriority as DefaultEventPriority_old,
Expand All @@ -22,7 +21,6 @@ import {
} from './ReactEventPriorities.old';

import {
UnknownEventPriority as UnknownEventPriority_new,
DiscreteEventPriority as DiscreteEventPriority_new,
ContinuousEventPriority as ContinuousEventPriority_new,
DefaultEventPriority as DefaultEventPriority_new,
Expand All @@ -35,9 +33,6 @@ import {

export type EventPriority = number;

export const UnknownEventPriority: EventPriority = enableNewReconciler
? (UnknownEventPriority_new: any)
: (UnknownEventPriority_old: any);
export const DiscreteEventPriority: EventPriority = enableNewReconciler
? (DiscreteEventPriority_new: any)
: (DiscreteEventPriority_old: any);
Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,6 @@ function useMutableSource<Source, Snapshot>(
setSnapshot(maybeNewSnapshot);

const lane = requestUpdateLane(fiber);
// TODO: What to do about isUnknownEventPriority
markRootMutableRead(root, lane);
}
// If the source mutated between render and now,
Expand All @@ -1334,7 +1333,6 @@ function useMutableSource<Source, Snapshot>(

// Record a pending mutable source update with the same expiration time.
const lane = requestUpdateLane(fiber);
// TODO: What to do about isUnknownEventPriority
markRootMutableRead(root, lane);
} catch (error) {
// A selector might throw after a source mutation.
Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,6 @@ function useMutableSource<Source, Snapshot>(
setSnapshot(maybeNewSnapshot);

const lane = requestUpdateLane(fiber);
// TODO: What to do about isUnknownEventPriority
markRootMutableRead(root, lane);
}
// If the source mutated between render and now,
Expand All @@ -1334,7 +1333,6 @@ function useMutableSource<Source, Snapshot>(

// Record a pending mutable source update with the same expiration time.
const lane = requestUpdateLane(fiber);
// TODO: What to do about isUnknownEventPriority
markRootMutableRead(root, lane);
} catch (error) {
// A selector might throw after a source mutation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ function scheduleFibersWithFamiliesRecursively(
}
if (needsRemount || needsRender) {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);

if (root !== null) {
scheduleUpdateOnFiber(
root,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ function scheduleFibersWithFamiliesRecursively(
}
if (needsRemount || needsRender) {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);

if (root !== null) {
scheduleUpdateOnFiber(
root,
Expand Down
11 changes: 1 addition & 10 deletions packages/react-reconciler/src/ReactFiberLane.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -483,11 +483,7 @@ export function includesBlockingLane(root: FiberRoot, lanes: Lanes): boolean {
allowConcurrentByDefault &&
(root.current.mode & ConcurrentUpdatesByDefaultMode) !== NoMode
) {
if (
enableFrameEndScheduling &&
(lanes & DefaultLane) !== NoLanes &&
root.hasUnknownUpdates
) {
if (enableFrameEndScheduling && (lanes & DefaultLane) !== NoLanes) {
// Unknown updates should flush synchronously, even in concurrent by default.
return true;
}
Expand Down Expand Up @@ -631,7 +627,6 @@ export function markRootUpdated(
export function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
root.suspendedLanes |= suspendedLanes;
root.pingedLanes &= ~suspendedLanes;
root.hasUnknownUpdates = false;

// The suspended lanes are no longer CPU-bound. Clear their expiration times.
const expirationTimes = root.expirationTimes;
Expand Down Expand Up @@ -663,10 +658,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {

root.pendingLanes = remainingLanes;

if ((root.pendingLanes & DefaultLane) === NoLane) {
root.hasUnknownUpdates = false;
}

// Let's try everything again
root.suspendedLanes = NoLanes;
root.pingedLanes = NoLanes;
Expand Down
11 changes: 1 addition & 10 deletions packages/react-reconciler/src/ReactFiberLane.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -483,11 +483,7 @@ export function includesBlockingLane(root: FiberRoot, lanes: Lanes): boolean {
allowConcurrentByDefault &&
(root.current.mode & ConcurrentUpdatesByDefaultMode) !== NoMode
) {
if (
enableFrameEndScheduling &&
(lanes & DefaultLane) !== NoLanes &&
root.hasUnknownUpdates
) {
if (enableFrameEndScheduling && (lanes & DefaultLane) !== NoLanes) {
// Unknown updates should flush synchronously, even in concurrent by default.
return true;
}
Expand Down Expand Up @@ -631,7 +627,6 @@ export function markRootUpdated(
export function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
root.suspendedLanes |= suspendedLanes;
root.pingedLanes &= ~suspendedLanes;
root.hasUnknownUpdates = false;

// The suspended lanes are no longer CPU-bound. Clear their expiration times.
const expirationTimes = root.expirationTimes;
Expand Down Expand Up @@ -663,10 +658,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {

root.pendingLanes = remainingLanes;

if ((root.pendingLanes & DefaultLane) === NoLane) {
root.hasUnknownUpdates = false;
}

// Let's try everything again
root.suspendedLanes = NoLanes;
root.pingedLanes = NoLanes;
Expand Down
17 changes: 4 additions & 13 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,6 @@ export function scheduleUpdateOnFiber(
didScheduleUpdateDuringPassiveEffects = true;
}
}

// Mark that the root has a pending update.
markRootUpdated(root, lane, eventTime, updatePriority);

Expand Down Expand Up @@ -924,12 +923,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {

if (
enableFrameEndScheduling &&
newCallbackPriority === DefaultLane &&
existingCallbackNode !== null &&
// TODO: We can't expose the rafNode here,
// but how do we know the rAF is not scheduled?
existingCallbackNode.rafNode == null &&
root.hasUnknownUpdates
supportsFrameAlignedTask &&
newCallbackPriority === DefaultEventPriority
) {
// Do nothing, we need to schedule a new rAF.
} else {
Expand All @@ -943,10 +938,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
if (
enableFrameEndScheduling &&
supportsFrameAlignedTask &&
existingCallbackNode != null &&
// TODO: we can't expose the scheduler node here,
// but how do we know we need to cancel with the host config method?
existingCallbackNode.schedulerNode != null
existingCallbackPriority === DefaultEventPriority
) {
cancelFrameAlignedTask(existingCallbackNode);
} else {
Expand Down Expand Up @@ -998,8 +990,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
} else if (
enableFrameEndScheduling &&
supportsFrameAlignedTask &&
newCallbackPriority === DefaultLane &&
root.hasUnknownUpdates
newCallbackPriority === DefaultEventPriority
) {
if (__DEV__ && ReactCurrentActQueue.current !== null) {
// Inside `act`, use our internal `act` queue so that these get flushed
Expand Down
17 changes: 4 additions & 13 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,6 @@ export function scheduleUpdateOnFiber(
didScheduleUpdateDuringPassiveEffects = true;
}
}

// Mark that the root has a pending update.
markRootUpdated(root, lane, eventTime, updatePriority);

Expand Down Expand Up @@ -924,12 +923,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {

if (
enableFrameEndScheduling &&
newCallbackPriority === DefaultLane &&
existingCallbackNode !== null &&
// TODO: We can't expose the rafNode here,
// but how do we know the rAF is not scheduled?
existingCallbackNode.rafNode == null &&
root.hasUnknownUpdates
supportsFrameAlignedTask &&
newCallbackPriority === DefaultEventPriority
) {
// Do nothing, we need to schedule a new rAF.
} else {
Expand All @@ -943,10 +938,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
if (
enableFrameEndScheduling &&
supportsFrameAlignedTask &&
existingCallbackNode != null &&
// TODO: we can't expose the scheduler node here,
// but how do we know we need to cancel with the host config method?
existingCallbackNode.schedulerNode != null
existingCallbackPriority === DefaultEventPriority
) {
cancelFrameAlignedTask(existingCallbackNode);
} else {
Expand Down Expand Up @@ -998,8 +990,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
} else if (
enableFrameEndScheduling &&
supportsFrameAlignedTask &&
newCallbackPriority === DefaultLane &&
root.hasUnknownUpdates
newCallbackPriority === DefaultEventPriority
) {
if (__DEV__ && ReactCurrentActQueue.current !== null) {
// Inside `act`, use our internal `act` queue so that these get flushed
Expand Down
4 changes: 3 additions & 1 deletion scripts/jest/setupTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
if (error) {
throw error;
}
if (global.requestAnimationFrameQueue != null) {
console.warn('requestAnimationFrameQueue has not been flushed.');
}
});
env.beforeEach(() => {
// TODO: warn if this has not flushed.
global.requestAnimationFrameQueue = null;
});

Expand Down

0 comments on commit 8cf2534

Please sign in to comment.