From afc8eee36450cb76dd4d12e2aa3e7138c892c294 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 15:36:30 -0800 Subject: [PATCH 1/9] We don't need the global state for this --- packages/react-reconciler/src/ReactFiberHooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 1e80abb8ef8e..dafce4e5106f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -478,7 +478,7 @@ export function renderWithHooks( // at the beginning of the render phase and there's no re-entrancy. ReactCurrentDispatcher.current = ContextOnlyDispatcher; - const renderedWork: Fiber = (currentlyRenderingFiber: any); + const renderedWork: Fiber = workInProgress; renderedWork.memoizedState = firstWorkInProgressHook; renderedWork.expirationTime = remainingExpirationTime; From 302610c0baee6d77e7bf2b8e5a9ce7e3ddb880a5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 15:57:40 -0800 Subject: [PATCH 2/9] Move componentUpdateQueue and sideEffectTag out of global state --- .../react-reconciler/src/ReactFiberHooks.js | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index dafce4e5106f..ccf847c3258b 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -188,8 +188,6 @@ let workInProgressHook: Hook | null = null; let nextWorkInProgressHook: Hook | null = null; let remainingExpirationTime: ExpirationTime = NoWork; -let componentUpdateQueue: FunctionComponentUpdateQueue | null = null; -let sideEffectTag: SideEffectTag = 0; // Updates scheduled during render will trigger an immediate re-render at the // end of the current pass. We can't store these updates on the normal queue, @@ -399,17 +397,17 @@ export function renderWithHooks( current !== null && current.type !== workInProgress.type; } + workInProgress.updateQueue = null; + // The following should have already been reset // currentHook = null; // workInProgressHook = null; // remainingExpirationTime = NoWork; - // componentUpdateQueue = null; // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; // numberOfReRenders = 0; - // sideEffectTag = 0; // TODO Warn if no hooks are used at all during mount, then some are used during update. // Currently we will identify the update render as a mount because nextCurrentHook === null. @@ -456,7 +454,8 @@ export function renderWithHooks( currentHook = null; workInProgressHook = null; - componentUpdateQueue = null; + + workInProgress.updateQueue = null; if (__DEV__) { // Also validate hook order for cascading updates. @@ -482,8 +481,6 @@ export function renderWithHooks( renderedWork.memoizedState = firstWorkInProgressHook; renderedWork.expirationTime = remainingExpirationTime; - renderedWork.updateQueue = (componentUpdateQueue: any); - renderedWork.effectTag |= sideEffectTag; if (__DEV__) { renderedWork._debugHookTypes = hookTypesDev; @@ -510,8 +507,6 @@ export function renderWithHooks( } remainingExpirationTime = NoWork; - componentUpdateQueue = null; - sideEffectTag = 0; // These were reset above // didScheduleRenderPhaseUpdate = false; @@ -565,8 +560,6 @@ export function resetHooks(): void { } remainingExpirationTime = NoWork; - componentUpdateQueue = null; - sideEffectTag = 0; didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; @@ -864,8 +857,10 @@ function pushEffect(tag, create, destroy, deps) { // Circular next: (null: any), }; + let fiber = ((currentlyRenderingFiber: any): Fiber); + let componentUpdateQueue: null | FunctionComponentUpdateQueue = (fiber.updateQueue: any); if (componentUpdateQueue === null) { - componentUpdateQueue = createFunctionComponentUpdateQueue(); + (fiber: any).updateQueue = componentUpdateQueue = createFunctionComponentUpdateQueue(); componentUpdateQueue.lastEffect = effect.next = effect; } else { const lastEffect = componentUpdateQueue.lastEffect; @@ -899,7 +894,8 @@ function updateRef(initialValue: T): {current: T} { function mountEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; - sideEffectTag |= fiberEffectTag; + let fiber = ((currentlyRenderingFiber: any): Fiber); + fiber.effectTag |= fiberEffectTag; hook.memoizedState = pushEffect(hookEffectTag, create, undefined, nextDeps); } @@ -920,7 +916,9 @@ function updateEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { } } - sideEffectTag |= fiberEffectTag; + let fiber = ((currentlyRenderingFiber: any): Fiber); + fiber.effectTag |= fiberEffectTag; + hook.memoizedState = pushEffect(hookEffectTag, create, destroy, nextDeps); } From 0d0bc765049ede84920301d191b220f184fcb01f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 16:46:52 -0800 Subject: [PATCH 3/9] Move remainingExpirationTime off global state --- .../react-reconciler/src/ReactFiberHooks.js | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index ccf847c3258b..ab6078f172cd 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -187,8 +187,6 @@ let firstWorkInProgressHook: Hook | null = null; let workInProgressHook: Hook | null = null; let nextWorkInProgressHook: Hook | null = null; -let remainingExpirationTime: ExpirationTime = NoWork; - // Updates scheduled during render will trigger an immediate re-render at the // end of the current pass. We can't store these updates on the normal queue, // because if the work is aborted, they should be discarded. Because this is @@ -398,13 +396,12 @@ export function renderWithHooks( } workInProgress.updateQueue = null; + workInProgress.expirationTime = NoWork; // The following should have already been reset // currentHook = null; // workInProgressHook = null; - // remainingExpirationTime = NoWork; - // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; // numberOfReRenders = 0; @@ -477,13 +474,8 @@ export function renderWithHooks( // at the beginning of the render phase and there's no re-entrancy. ReactCurrentDispatcher.current = ContextOnlyDispatcher; - const renderedWork: Fiber = workInProgress; - - renderedWork.memoizedState = firstWorkInProgressHook; - renderedWork.expirationTime = remainingExpirationTime; - if (__DEV__) { - renderedWork._debugHookTypes = hookTypesDev; + workInProgress._debugHookTypes = hookTypesDev; } // This check uses currentHook so that it works the same in DEV and prod bundles. @@ -506,8 +498,6 @@ export function renderWithHooks( hookTypesUpdateIndexDev = -1; } - remainingExpirationTime = NoWork; - // These were reset above // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; @@ -559,8 +549,6 @@ export function resetHooks(): void { currentHookNameInDev = null; } - remainingExpirationTime = NoWork; - didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; numberOfReRenders = 0; @@ -763,9 +751,10 @@ function updateReducer( newBaseState = newState; } // Update the remaining priority in the queue. - if (updateExpirationTime > remainingExpirationTime) { - remainingExpirationTime = updateExpirationTime; - markUnprocessedUpdateTime(remainingExpirationTime); + let fiber = ((currentlyRenderingFiber: any): Fiber); + if (updateExpirationTime > fiber.expirationTime) { + fiber.expirationTime = updateExpirationTime; + markUnprocessedUpdateTime(updateExpirationTime); } } else { // This update does have sufficient priority. From d4b1793f0089dc3c3db570400f8dc7f476fff1ea Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 16:10:05 -0800 Subject: [PATCH 4/9] Move firstWorkInProgressHook off global state --- packages/react-reconciler/src/ReactFiberHooks.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index ab6078f172cd..5f9e71c1c61c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -12,7 +12,6 @@ import type { ReactContext, ReactEventResponderListener, } from 'shared/ReactTypes'; -import type {SideEffectTag} from 'shared/ReactSideEffectTags'; import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {HookEffectTag} from './ReactHookEffectTags'; @@ -183,7 +182,6 @@ let currentlyRenderingFiber: Fiber | null = null; // work-in-progress fiber. let currentHook: Hook | null = null; let nextCurrentHook: Hook | null = null; -let firstWorkInProgressHook: Hook | null = null; let workInProgressHook: Hook | null = null; let nextWorkInProgressHook: Hook | null = null; @@ -395,6 +393,7 @@ export function renderWithHooks( current !== null && current.type !== workInProgress.type; } + workInProgress.memoizedState = null; workInProgress.updateQueue = null; workInProgress.expirationTime = NoWork; @@ -447,7 +446,7 @@ export function renderWithHooks( // Start over from the beginning of the list nextCurrentHook = current !== null ? current.memoizedState : null; - nextWorkInProgressHook = firstWorkInProgressHook; + nextWorkInProgressHook = workInProgress.memoizedState; currentHook = null; workInProgressHook = null; @@ -488,7 +487,6 @@ export function renderWithHooks( currentHook = null; nextCurrentHook = null; - firstWorkInProgressHook = null; workInProgressHook = null; nextWorkInProgressHook = null; @@ -538,7 +536,6 @@ export function resetHooks(): void { currentHook = null; nextCurrentHook = null; - firstWorkInProgressHook = null; workInProgressHook = null; nextWorkInProgressHook = null; @@ -567,7 +564,8 @@ function mountWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list - firstWorkInProgressHook = workInProgressHook = hook; + let fiber = ((currentlyRenderingFiber: any): Fiber); + fiber.memoizedState = workInProgressHook = hook; } else { // Append to the end of the list workInProgressHook = workInProgressHook.next = hook; @@ -608,7 +606,8 @@ function updateWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list. - workInProgressHook = firstWorkInProgressHook = newHook; + let fiber = ((currentlyRenderingFiber: any): Fiber); + fiber.memoizedState = workInProgressHook = newHook; } else { // Append to the end of the list. workInProgressHook = workInProgressHook.next = newHook; From 0432096bca5270669b076f8e1c2aef762fd866cb Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 18:50:36 -0800 Subject: [PATCH 5/9] Reset fiber to its current state if it throws --- packages/react-reconciler/src/ReactFiberThrow.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 0205814d5b5e..99c556c77542 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -195,6 +195,18 @@ function throwException( // This is a thenable. const thenable: Thenable = (value: any); + if ((sourceFiber.mode & BlockingMode) === NoMode) { + // Reset the memoizedState to what it was before we attempted + // to render it. + let currentSource = sourceFiber.alternate; + if (currentSource) { + sourceFiber.memoizedState = currentSource.memoizedState; + sourceFiber.expirationTime = currentSource.expirationTime; + } else { + sourceFiber.memoizedState = null; + } + } + checkForWrongSuspensePriorityInDEV(sourceFiber); let hasInvisibleParentBoundary = hasSuspenseContext( From 3e77742d8c4e64b89f816c0b1ce0bc156f8c5f61 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 19:05:47 -0800 Subject: [PATCH 6/9] Move rerender error check to avoid global state This means that it's harder to find it since it's not in the dispatch function's stack but we can add a DEV only one for that if we really need it. Alternatively, we can check it in against the renderUpdates queue. --- .../react-reconciler/src/ReactFiberHooks.js | 86 +++++++++---------- 1 file changed, 41 insertions(+), 45 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 5f9e71c1c61c..e229ff457932 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -200,8 +200,7 @@ let renderPhaseUpdates: Map< UpdateQueue, Update, > | null = null; -// Counter to prevent infinite loops. -let numberOfReRenders: number = 0; + const RE_RENDER_LIMIT = 25; // In DEV, this is the name of the currently executing primitive hook @@ -403,7 +402,6 @@ export function renderWithHooks( // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; - // numberOfReRenders = 0; // TODO Warn if no hooks are used at all during mount, then some are used during update. // Currently we will identify the update render as a mount because nextCurrentHook === null. @@ -435,8 +433,17 @@ export function renderWithHooks( let children = Component(props, refOrContext); if (didScheduleRenderPhaseUpdate) { + // Counter to prevent infinite loops. + let numberOfReRenders: number = 0; do { didScheduleRenderPhaseUpdate = false; + + invariant( + numberOfReRenders < RE_RENDER_LIMIT, + 'Too many re-renders. React limits the number of renders to prevent ' + + 'an infinite loop.', + ); + numberOfReRenders += 1; if (__DEV__) { // Even when hot reloading, allow dependencies to stabilize @@ -466,7 +473,6 @@ export function renderWithHooks( } while (didScheduleRenderPhaseUpdate); renderPhaseUpdates = null; - numberOfReRenders = 0; } // We can assume the previous dispatcher is always this one, since we set it @@ -499,7 +505,6 @@ export function renderWithHooks( // These were reset above // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; - // numberOfReRenders = 0; invariant( !didRenderTooFewHooks, @@ -548,7 +553,6 @@ export function resetHooks(): void { didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; - numberOfReRenders = 0; } function mountWorkInProgressHook(): Hook { @@ -669,45 +673,43 @@ function updateReducer( queue.lastRenderedReducer = reducer; - if (numberOfReRenders > 0) { + if (renderPhaseUpdates !== null) { // This is a re-render. Apply the new render phase updates to the previous // work-in-progress hook. const dispatch: Dispatch = (queue.dispatch: any); - if (renderPhaseUpdates !== null) { - // Render phase updates are stored in a map of queue -> linked list - const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); - if (firstRenderPhaseUpdate !== undefined) { - renderPhaseUpdates.delete(queue); - let newState = hook.memoizedState; - let update = firstRenderPhaseUpdate; - do { - // Process this render phase update. We don't have to check the - // priority because it will always be the same as the current - // render's. - const action = update.action; - newState = reducer(newState, action); - update = update.next; - } while (update !== null); - - // Mark that the fiber performed work, but only if the new state is - // different from the current state. - if (!is(newState, hook.memoizedState)) { - markWorkInProgressReceivedUpdate(); - } + // Render phase updates are stored in a map of queue -> linked list + const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); + if (firstRenderPhaseUpdate !== undefined) { + renderPhaseUpdates.delete(queue); + let newState = hook.memoizedState; + let update = firstRenderPhaseUpdate; + do { + // Process this render phase update. We don't have to check the + // priority because it will always be the same as the current + // render's. + const action = update.action; + newState = reducer(newState, action); + update = update.next; + } while (update !== null); + + // Mark that the fiber performed work, but only if the new state is + // different from the current state. + if (!is(newState, hook.memoizedState)) { + markWorkInProgressReceivedUpdate(); + } - hook.memoizedState = newState; - // Don't persist the state accumulated from the render phase updates to - // the base state unless the queue is empty. - // TODO: Not sure if this is the desired semantics, but it's what we - // do for gDSFP. I can't remember why. - if (hook.baseUpdate === queue.last) { - hook.baseState = newState; - } + hook.memoizedState = newState; + // Don't persist the state accumulated from the render phase updates to + // the base state unless the queue is empty. + // TODO: Not sure if this is the desired semantics, but it's what we + // do for gDSFP. I can't remember why. + if (hook.baseUpdate === queue.last) { + hook.baseState = newState; + } - queue.lastRenderedState = newState; + queue.lastRenderedState = newState; - return [newState, dispatch]; - } + return [newState, dispatch]; } return [hook.memoizedState, dispatch]; } @@ -1208,12 +1210,6 @@ function dispatchAction( queue: UpdateQueue, action: A, ) { - invariant( - numberOfReRenders < RE_RENDER_LIMIT, - 'Too many re-renders. React limits the number of renders to prevent ' + - 'an infinite loop.', - ); - if (__DEV__) { warning( typeof arguments[3] !== 'function', From cc1aa9d567a4ea5aef18f79e5d6ce55e7e2279a6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 19:20:51 -0800 Subject: [PATCH 7/9] Move next___Hook out of global state --- .../react-reconciler/src/ReactFiberHooks.js | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index e229ff457932..82c0ec4d2a90 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -181,9 +181,7 @@ let currentlyRenderingFiber: Fiber | null = null; // work-in-progress hook list is a new list that will be added to the // work-in-progress fiber. let currentHook: Hook | null = null; -let nextCurrentHook: Hook | null = null; let workInProgressHook: Hook | null = null; -let nextWorkInProgressHook: Hook | null = null; // Updates scheduled during render will trigger an immediate re-render at the // end of the current pass. We can't store these updates on the normal queue, @@ -379,7 +377,6 @@ export function renderWithHooks( ): any { renderExpirationTime = nextRenderExpirationTime; currentlyRenderingFiber = workInProgress; - nextCurrentHook = current !== null ? current.memoizedState : null; if (__DEV__) { hookTypesDev = @@ -404,14 +401,14 @@ export function renderWithHooks( // renderPhaseUpdates = null; // TODO Warn if no hooks are used at all during mount, then some are used during update. - // Currently we will identify the update render as a mount because nextCurrentHook === null. + // Currently we will identify the update render as a mount because memoizedState === null. // This is tricky because it's valid for certain types of components (e.g. React.lazy) - // Using nextCurrentHook to differentiate between mount/update only works if at least one stateful hook is used. + // Using memoizedState to differentiate between mount/update only works if at least one stateful hook is used. // Non-stateful hooks (e.g. context) don't get added to memoizedState, - // so nextCurrentHook would be null during updates and mounts. + // so memoizedState would be null during updates and mounts. if (__DEV__) { - if (nextCurrentHook !== null) { + if (current !== null && current.memoizedState !== null) { ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } else if (hookTypesDev !== null) { // This dispatcher handles an edge case where a component is updating, @@ -425,7 +422,7 @@ export function renderWithHooks( } } else { ReactCurrentDispatcher.current = - nextCurrentHook === null + current === null || current.memoizedState === null ? HooksDispatcherOnMount : HooksDispatcherOnUpdate; } @@ -452,9 +449,6 @@ export function renderWithHooks( } // Start over from the beginning of the list - nextCurrentHook = current !== null ? current.memoizedState : null; - nextWorkInProgressHook = workInProgress.memoizedState; - currentHook = null; workInProgressHook = null; @@ -492,9 +486,7 @@ export function renderWithHooks( currentlyRenderingFiber = null; currentHook = null; - nextCurrentHook = null; workInProgressHook = null; - nextWorkInProgressHook = null; if (__DEV__) { currentHookNameInDev = null; @@ -540,9 +532,7 @@ export function resetHooks(): void { currentlyRenderingFiber = null; currentHook = null; - nextCurrentHook = null; workInProgressHook = null; - nextWorkInProgressHook = null; if (__DEV__) { hookTypesDev = null; @@ -583,15 +573,36 @@ function updateWorkInProgressHook(): Hook { // clone, or a work-in-progress hook from a previous render pass that we can // use as a base. When we reach the end of the base list, we must switch to // the dispatcher used for mounts. + let nextCurrentHook: null | Hook; + if (currentHook === null) { + let fiber = ((currentlyRenderingFiber: any): Fiber); + let current = fiber.alternate; + if (current !== null) { + nextCurrentHook = current.memoizedState; + } else { + nextCurrentHook = null; + } + } else { + nextCurrentHook = currentHook.next; + } + + let nextWorkInProgressHook: null | Hook; + if (workInProgressHook === null) { + let fiber = ((currentlyRenderingFiber: any): Fiber); + nextWorkInProgressHook = fiber.memoizedState; + } else { + nextWorkInProgressHook = workInProgressHook.next; + } + if (nextWorkInProgressHook !== null) { // There's already a work-in-progress. Reuse it. workInProgressHook = nextWorkInProgressHook; nextWorkInProgressHook = workInProgressHook.next; currentHook = nextCurrentHook; - nextCurrentHook = currentHook !== null ? currentHook.next : null; } else { // Clone from the current hook. + invariant( nextCurrentHook !== null, 'Rendered more hooks than during the previous render.', @@ -616,7 +627,6 @@ function updateWorkInProgressHook(): Hook { // Append to the end of the list. workInProgressHook = workInProgressHook.next = newHook; } - nextCurrentHook = currentHook.next; } return workInProgressHook; } From 25aa2e5e04d3286d21181009dc3fcabcc9ac9f61 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 19:54:12 -0800 Subject: [PATCH 8/9] Assert that currentlyRenderingFiber is always set When accessed, this should always be set. This could enforced by storing this on the dispatcher for example. --- .../react-reconciler/src/ReactFiberHooks.js | 53 +++++++------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 82c0ec4d2a90..bed33089415f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -174,7 +174,7 @@ type Dispatch = A => void; let renderExpirationTime: ExpirationTime = NoWork; // The work-in-progress fiber. I've named it differently to distinguish it from // the work-in-progress hook. -let currentlyRenderingFiber: Fiber | null = null; +let currentlyRenderingFiber: Fiber = (null: any); // Hooks are stored as a linked list on the fiber's memoizedState field. The // current hook list is the list that belongs to the current fiber. The @@ -258,9 +258,7 @@ function checkDepsAreArrayDev(deps: mixed) { function warnOnHookMismatchInDev(currentHookName: HookType) { if (__DEV__) { - const componentName = getComponentName( - ((currentlyRenderingFiber: any): Fiber).type, - ); + const componentName = getComponentName(currentlyRenderingFiber.type); if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) { didWarnAboutMismatchedHooksForComponent.add(componentName); @@ -483,7 +481,7 @@ export function renderWithHooks( currentHook !== null && currentHook.next !== null; renderExpirationTime = NoWork; - currentlyRenderingFiber = null; + currentlyRenderingFiber = (null: any); currentHook = null; workInProgressHook = null; @@ -529,7 +527,7 @@ export function resetHooks(): void { // component is a module-style component. renderExpirationTime = NoWork; - currentlyRenderingFiber = null; + currentlyRenderingFiber = (null: any); currentHook = null; workInProgressHook = null; @@ -558,8 +556,7 @@ function mountWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list - let fiber = ((currentlyRenderingFiber: any): Fiber); - fiber.memoizedState = workInProgressHook = hook; + currentlyRenderingFiber.memoizedState = workInProgressHook = hook; } else { // Append to the end of the list workInProgressHook = workInProgressHook.next = hook; @@ -575,8 +572,7 @@ function updateWorkInProgressHook(): Hook { // the dispatcher used for mounts. let nextCurrentHook: null | Hook; if (currentHook === null) { - let fiber = ((currentlyRenderingFiber: any): Fiber); - let current = fiber.alternate; + let current = currentlyRenderingFiber.alternate; if (current !== null) { nextCurrentHook = current.memoizedState; } else { @@ -588,8 +584,7 @@ function updateWorkInProgressHook(): Hook { let nextWorkInProgressHook: null | Hook; if (workInProgressHook === null) { - let fiber = ((currentlyRenderingFiber: any): Fiber); - nextWorkInProgressHook = fiber.memoizedState; + nextWorkInProgressHook = currentlyRenderingFiber.memoizedState; } else { nextWorkInProgressHook = workInProgressHook.next; } @@ -621,8 +616,7 @@ function updateWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list. - let fiber = ((currentlyRenderingFiber: any): Fiber); - fiber.memoizedState = workInProgressHook = newHook; + currentlyRenderingFiber.memoizedState = workInProgressHook = newHook; } else { // Append to the end of the list. workInProgressHook = workInProgressHook.next = newHook; @@ -662,8 +656,7 @@ function mountReducer( }); const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( null, - // Flow doesn't know this is non-null, but we do. - ((currentlyRenderingFiber: any): Fiber), + currentlyRenderingFiber, queue, ): any)); return [hook.memoizedState, dispatch]; @@ -762,9 +755,8 @@ function updateReducer( newBaseState = newState; } // Update the remaining priority in the queue. - let fiber = ((currentlyRenderingFiber: any): Fiber); - if (updateExpirationTime > fiber.expirationTime) { - fiber.expirationTime = updateExpirationTime; + if (updateExpirationTime > currentlyRenderingFiber.expirationTime) { + currentlyRenderingFiber.expirationTime = updateExpirationTime; markUnprocessedUpdateTime(updateExpirationTime); } } else { @@ -835,8 +827,7 @@ function mountState( BasicStateAction, > = (queue.dispatch = (dispatchAction.bind( null, - // Flow doesn't know this is non-null, but we do. - ((currentlyRenderingFiber: any): Fiber), + currentlyRenderingFiber, queue, ): any)); return [hook.memoizedState, dispatch]; @@ -857,10 +848,10 @@ function pushEffect(tag, create, destroy, deps) { // Circular next: (null: any), }; - let fiber = ((currentlyRenderingFiber: any): Fiber); - let componentUpdateQueue: null | FunctionComponentUpdateQueue = (fiber.updateQueue: any); + let componentUpdateQueue: null | FunctionComponentUpdateQueue = (currentlyRenderingFiber.updateQueue: any); if (componentUpdateQueue === null) { - (fiber: any).updateQueue = componentUpdateQueue = createFunctionComponentUpdateQueue(); + componentUpdateQueue = createFunctionComponentUpdateQueue(); + currentlyRenderingFiber.updateQueue = (componentUpdateQueue: any); componentUpdateQueue.lastEffect = effect.next = effect; } else { const lastEffect = componentUpdateQueue.lastEffect; @@ -894,8 +885,7 @@ function updateRef(initialValue: T): {current: T} { function mountEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; - let fiber = ((currentlyRenderingFiber: any): Fiber); - fiber.effectTag |= fiberEffectTag; + currentlyRenderingFiber.effectTag |= fiberEffectTag; hook.memoizedState = pushEffect(hookEffectTag, create, undefined, nextDeps); } @@ -916,8 +906,7 @@ function updateEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { } } - let fiber = ((currentlyRenderingFiber: any): Fiber); - fiber.effectTag |= fiberEffectTag; + currentlyRenderingFiber.effectTag |= fiberEffectTag; hook.memoizedState = pushEffect(hookEffectTag, create, destroy, nextDeps); } @@ -929,9 +918,7 @@ function mountEffect( if (__DEV__) { // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests if ('undefined' !== typeof jest) { - warnIfNotCurrentlyActingEffectsInDEV( - ((currentlyRenderingFiber: any): Fiber), - ); + warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); } } return mountEffectImpl( @@ -949,9 +936,7 @@ function updateEffect( if (__DEV__) { // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests if ('undefined' !== typeof jest) { - warnIfNotCurrentlyActingEffectsInDEV( - ((currentlyRenderingFiber: any): Fiber), - ); + warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); } } return updateEffectImpl( From 1ea22f8f69db18aebef4e0a2c3512fabf47c85fe Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 3 Dec 2019 12:40:20 -0800 Subject: [PATCH 9/9] Add another test just to be safe --- ...tSuspenseWithNoopRenderer-test.internal.js | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 15c7b9e5f19a..86fb78244188 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -1512,6 +1512,165 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }); + it('does not call lifecycles of a suspended component (hooks)', async () => { + function TextWithLifecycle(props) { + React.useLayoutEffect( + () => { + Scheduler.unstable_yieldValue(`Layout Effect [${props.text}]`); + return () => { + Scheduler.unstable_yieldValue( + `Destroy Layout Effect [${props.text}]`, + ); + }; + }, + [props.text], + ); + React.useEffect( + () => { + Scheduler.unstable_yieldValue(`Effect [${props.text}]`); + return () => { + Scheduler.unstable_yieldValue(`Destroy Effect [${props.text}]`); + }; + }, + [props.text], + ); + return ; + } + + function AsyncTextWithLifecycle(props) { + React.useLayoutEffect( + () => { + Scheduler.unstable_yieldValue(`Layout Effect [${props.text}]`); + return () => { + Scheduler.unstable_yieldValue( + `Destroy Layout Effect [${props.text}]`, + ); + }; + }, + [props.text], + ); + React.useEffect( + () => { + Scheduler.unstable_yieldValue(`Effect [${props.text}]`); + return () => { + Scheduler.unstable_yieldValue(`Destroy Effect [${props.text}]`); + }; + }, + [props.text], + ); + const text = props.text; + const ms = props.ms; + try { + TextResource.read([text, ms]); + Scheduler.unstable_yieldValue(text); + return ; + } catch (promise) { + if (typeof promise.then === 'function') { + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + } else { + Scheduler.unstable_yieldValue(`Error! [${text}]`); + } + throw promise; + } + } + + function App({text}) { + return ( + }> + + + + + ); + } + + ReactNoop.renderLegacySyncRoot(, () => + Scheduler.unstable_yieldValue('Commit root'), + ); + expect(Scheduler).toHaveYielded([ + 'A', + 'Suspend! [B]', + 'C', + 'Loading...', + + 'Layout Effect [A]', + // B's effect should not fire because it suspended + // 'Layout Effect [B]', + 'Layout Effect [C]', + 'Layout Effect [Loading...]', + 'Commit root', + ]); + + // Flush passive effects. + expect(Scheduler).toFlushAndYield([ + 'Effect [A]', + // B's effect should not fire because it suspended + // 'Effect [B]', + 'Effect [C]', + 'Effect [Loading...]', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> +