Skip to content

Commit

Permalink
Address TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
@tyao1
tyao1 committed Sep 21, 2022
1 parent 89a4ba9 commit f4f46dd
Show file tree
Hide file tree
Showing 15 changed files with 46 additions and 61 deletions.
11 changes: 5 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -915,26 +915,25 @@ describe('ReactDOMFiberAsync', () => {

window.event = undefined;
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.
window.event = 'test';
setThrowing(false);
setState(2);

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

expect(Scheduler).toFlushAndYield(['Count: 2']);
expect(counterRef.current.textContent).toBe('Count: 2');
Expand Down
13 changes: 8 additions & 5 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,20 +402,19 @@ 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,
schedulerNode: number,
rafNode: AnimationFrameID,
schedulerNode: number | null,
task: function,
|};

let currentTask: FrameAlignedTask | null = null;
function performFrameAlignedWork() {
if (currentTask != null) {
const task = currentTask.task;
localCancelAnimationFrame(currentTask.id);
localCancelAnimationFrame(currentTask.rafNode);
Scheduler.unstable_cancelCallback(currentTask.schedulerNode);
currentTask = null;
if (task != null) {
Expand All @@ -424,6 +423,10 @@ function performFrameAlignedWork() {
}
}

export function isFrameAlignedTask(task: any): boolean {
return task != null && task.rafNode != null && task.task != null;
}

export function scheduleFrameAlignedTask(task: any): any {
if (currentTask === null) {
const rafNode = localRequestAnimationFrame(performFrameAlignedWork);
Expand All @@ -449,7 +452,7 @@ 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.
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2795,7 +2795,7 @@ function updateDehydratedSuspenseComponent(
current,
attemptHydrationAtLane,
eventTime,
false, // TODO: what about isUnknownUpdate
false,
);
} else {
// We have already tried to ping at a higher priority than we're rendering with
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2795,7 +2795,7 @@ function updateDehydratedSuspenseComponent(
current,
attemptHydrationAtLane,
eventTime,
false, // TODO: what about isUnknownUpdate
false,
);
} else {
// We have already tried to ping at a higher priority than we're rendering with
Expand Down
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1642,8 +1642,15 @@ function checkIfSnapshotChanged<T>(inst: StoreInstance<T>): boolean {

function forceStoreRerender(fiber) {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
const isUnknownEventPriority = requestUpdateLane_isUnknownEventPriority();
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false); // TODO: isUnknownEvent
scheduleUpdateOnFiber(
root,
fiber,
SyncLane,
NoTimestamp,
isUnknownEventPriority,
);
}
}

Expand Down
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1642,8 +1642,15 @@ function checkIfSnapshotChanged<T>(inst: StoreInstance<T>): boolean {

function forceStoreRerender(fiber) {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
const isUnknownEventPriority = requestUpdateLane_isUnknownEventPriority();
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false); // TODO: isUnknownEvent
scheduleUpdateOnFiber(
root,
fiber,
SyncLane,
NoTimestamp,
isUnknownEventPriority,
);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberHotReloading.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ function scheduleFibersWithFamiliesRecursively(
}
if (needsRemount || needsRender) {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);

if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberHotReloading.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ function scheduleFibersWithFamiliesRecursively(
}
if (needsRemount || needsRender) {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);

if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand Down
11 changes: 0 additions & 11 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,6 @@ export function attemptSynchronousHydration(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, eventTime, false);
}
});
Expand Down Expand Up @@ -481,7 +480,6 @@ export function attemptDiscreteHydration(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, lane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, lane, eventTime, false);
}
markRetryLaneIfNotHydrated(fiber, lane);
Expand All @@ -499,7 +497,6 @@ export function attemptContinuousHydration(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, lane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, lane, eventTime, false);
}
markRetryLaneIfNotHydrated(fiber, lane);
Expand All @@ -515,7 +512,6 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, lane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, lane, eventTime, false);
}
markRetryLaneIfNotHydrated(fiber, lane);
Expand Down Expand Up @@ -695,7 +691,6 @@ if (__DEV__) {

const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand All @@ -720,7 +715,6 @@ if (__DEV__) {

const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand All @@ -746,7 +740,6 @@ if (__DEV__) {

const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand All @@ -760,7 +753,6 @@ if (__DEV__) {
}
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};
Expand All @@ -771,7 +763,6 @@ if (__DEV__) {
}
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};
Expand All @@ -786,15 +777,13 @@ if (__DEV__) {
}
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};

scheduleUpdate = (fiber: Fiber) => {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};
Expand Down
11 changes: 0 additions & 11 deletions packages/react-reconciler/src/ReactFiberReconciler.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,6 @@ export function attemptSynchronousHydration(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, eventTime, false);
}
});
Expand Down Expand Up @@ -481,7 +480,6 @@ export function attemptDiscreteHydration(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, lane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, lane, eventTime, false);
}
markRetryLaneIfNotHydrated(fiber, lane);
Expand All @@ -499,7 +497,6 @@ export function attemptContinuousHydration(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, lane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, lane, eventTime, false);
}
markRetryLaneIfNotHydrated(fiber, lane);
Expand All @@ -515,7 +512,6 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {
const root = enqueueConcurrentRenderForLane(fiber, lane);
if (root !== null) {
const eventTime = requestEventTime();
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, lane, eventTime, false);
}
markRetryLaneIfNotHydrated(fiber, lane);
Expand Down Expand Up @@ -695,7 +691,6 @@ if (__DEV__) {

const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand All @@ -720,7 +715,6 @@ if (__DEV__) {

const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand All @@ -746,7 +740,6 @@ if (__DEV__) {

const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
}
Expand All @@ -760,7 +753,6 @@ if (__DEV__) {
}
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};
Expand All @@ -771,7 +763,6 @@ if (__DEV__) {
}
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};
Expand All @@ -786,15 +777,13 @@ if (__DEV__) {
}
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};

scheduleUpdate = (fiber: Fiber) => {
const root = enqueueConcurrentRenderForLane(fiber, SyncLane);
if (root !== null) {
// TODO: isUnknownEvent
scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp, false);
}
};
Expand Down
14 changes: 4 additions & 10 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import {
supportsMicrotasks,
errorHydratingContainer,
scheduleMicrotask,
isFrameAlignedTask,
cancelFrameAlignedTask,
scheduleFrameAlignedTask,
supportsFrameAlignedTask,
Expand Down Expand Up @@ -703,7 +704,6 @@ export function scheduleUpdateOnFiber(
didScheduleUpdateDuringPassiveEffects = true;
}
}

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

Expand Down Expand Up @@ -903,11 +903,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
root.hasUnknownUpdates &&
!isFrameAlignedTask(existingCallbackNode)
) {
// Do nothing, we need to schedule a new rAF.
} else {
Expand All @@ -921,10 +918,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
isFrameAlignedTask(existingCallbackNode)
) {
cancelFrameAlignedTask(existingCallbackNode);
} else {
Expand Down
Loading

0 comments on commit f4f46dd

Please sign in to comment.