From ccef29af2d71aba566d13e3c89c86210bd2f1247 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 13 Feb 2020 14:15:29 -0800 Subject: [PATCH] Refactored hook to reflect changes in RFC --- .../react-debug-tools/src/ReactDebugHooks.js | 19 +- .../src/server/ReactPartialRendererHooks.js | 10 +- .../src/ReactFiberBeginWork.js | 33 ++ .../src/ReactFiberCommitWork.js | 4 +- .../react-reconciler/src/ReactFiberHooks.js | 209 ++++++------ .../src/ReactFiberWorkLoop.js | 63 ++-- ...eactHooksWithNoopRenderer-test.internal.js | 307 ++++++++++++++---- .../src/ReactShallowRenderer.js | 10 +- packages/react/src/React.js | 2 + packages/react/src/ReactHooks.js | 8 +- packages/shared/ReactMutableSource.js | 141 +++++--- scripts/error-codes/codes.json | 4 +- 12 files changed, 545 insertions(+), 265 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 1003bd1727d0d..24d5d6d89fe0b 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -72,9 +72,13 @@ function getPrimitiveStackCache(): Map> { Dispatcher.useCallback(() => {}); Dispatcher.useMemo(() => null); Dispatcher.useMutableSource( - {}, { - getVersion: () => null, + _source: {}, + _getVersion: () => 1, + _workInProgressVersionPrimary: null, + _workInProgressVersionSecondary: null, + }, + { getSnapshot: () => null, subscribe: () => () => {}, }, @@ -236,13 +240,14 @@ function useMemo( return value; } -function useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, -): S { +export function useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, +): Snapshot { const hook = nextHook(); const getSnapshot = config.getSnapshot; - const value = hook !== null ? hook.memoizedState.snapshot : getSnapshot(); + const value = + hook !== null ? hook.memoizedState.snapshot : getSnapshot(source._source); hookLog.push({primitive: 'MutableSource', stackError: new Error(), value}); return value; } diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js index 059ffea16c1a4..d9b3c9b159630 100644 --- a/packages/react-dom/src/server/ReactPartialRendererHooks.js +++ b/packages/react-dom/src/server/ReactPartialRendererHooks.js @@ -463,13 +463,13 @@ function useResponder(responder, props): ReactEventResponderListener { }; } -function useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, -): S { +function useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, +): Snapshot { resolveCurrentlyRenderingComponent(); const getSnapshot = config.getSnapshot; - return getSnapshot(); + return getSnapshot(source._source); } function useDeferredValue(value: T, config: TimeoutConfig | null | void): T { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 9db5886a6c2b4..c931b73c6d5bc 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -277,6 +277,7 @@ function forceUnmountCurrentAndReconcile( } function updateForwardRef( + root: FiberRoot, current: Fiber | null, workInProgress: Fiber, Component: any, @@ -316,6 +317,7 @@ function updateForwardRef( nextChildren = renderWithHooks( current, workInProgress, + root, render, nextProps, ref, @@ -330,6 +332,7 @@ function updateForwardRef( nextChildren = renderWithHooks( current, workInProgress, + root, render, nextProps, ref, @@ -342,6 +345,7 @@ function updateForwardRef( nextChildren = renderWithHooks( current, workInProgress, + root, render, nextProps, ref, @@ -370,6 +374,7 @@ function updateForwardRef( } function updateMemoComponent( + root: FiberRoot, current: Fiber | null, workInProgress: Fiber, Component: any, @@ -398,6 +403,7 @@ function updateMemoComponent( validateFunctionComponentInDev(workInProgress, type); } return updateSimpleMemoComponent( + root, current, workInProgress, resolvedType, @@ -478,6 +484,7 @@ function updateMemoComponent( } function updateSimpleMemoComponent( + root: FiberRoot, current: Fiber | null, workInProgress: Fiber, Component: any, @@ -532,6 +539,7 @@ function updateSimpleMemoComponent( } } return updateFunctionComponent( + root, current, workInProgress, Component, @@ -601,6 +609,7 @@ function markRef(current: Fiber | null, workInProgress: Fiber) { } function updateFunctionComponent( + root, current, workInProgress, Component, @@ -638,6 +647,7 @@ function updateFunctionComponent( nextChildren = renderWithHooks( current, workInProgress, + root, Component, nextProps, context, @@ -652,6 +662,7 @@ function updateFunctionComponent( nextChildren = renderWithHooks( current, workInProgress, + root, Component, nextProps, context, @@ -664,6 +675,7 @@ function updateFunctionComponent( nextChildren = renderWithHooks( current, workInProgress, + root, Component, nextProps, context, @@ -692,6 +704,7 @@ function updateFunctionComponent( } function updateChunk( + root: FiberRoot, current: Fiber | null, workInProgress: Fiber, chunk: any, @@ -714,6 +727,7 @@ function updateChunk( nextChildren = renderWithHooks( current, workInProgress, + root, render, nextProps, data, @@ -728,6 +742,7 @@ function updateChunk( nextChildren = renderWithHooks( current, workInProgress, + root, render, nextProps, data, @@ -740,6 +755,7 @@ function updateChunk( nextChildren = renderWithHooks( current, workInProgress, + root, render, nextProps, data, @@ -1110,6 +1126,7 @@ function updateHostText(current, workInProgress) { } function mountLazyComponent( + root, _current, workInProgress, elementType, @@ -1147,6 +1164,7 @@ function mountLazyComponent( ); } child = updateFunctionComponent( + root, null, workInProgress, Component, @@ -1177,6 +1195,7 @@ function mountLazyComponent( ); } child = updateForwardRef( + root, null, workInProgress, Component, @@ -1201,6 +1220,7 @@ function mountLazyComponent( } } child = updateMemoComponent( + root, null, workInProgress, Component, @@ -1214,6 +1234,7 @@ function mountLazyComponent( if (enableChunksAPI) { // TODO: Resolve for Hot Reloading. child = updateChunk( + root, null, workInProgress, Component, @@ -1306,6 +1327,7 @@ function mountIncompleteClassComponent( } function mountIndeterminateComponent( + root, _current, workInProgress, Component, @@ -1362,6 +1384,7 @@ function mountIndeterminateComponent( value = renderWithHooks( null, workInProgress, + root, Component, props, context, @@ -1371,6 +1394,7 @@ function mountIndeterminateComponent( value = renderWithHooks( null, workInProgress, + root, Component, props, context, @@ -1467,6 +1491,7 @@ function mountIndeterminateComponent( value = renderWithHooks( null, workInProgress, + root, Component, props, context, @@ -2886,6 +2911,7 @@ function remountFiber( } function beginWork( + root: FiberRoot, current: Fiber | null, workInProgress: Fiber, renderExpirationTime: ExpirationTime, @@ -3109,6 +3135,7 @@ function beginWork( switch (workInProgress.tag) { case IndeterminateComponent: { return mountIndeterminateComponent( + root, current, workInProgress, workInProgress.type, @@ -3118,6 +3145,7 @@ function beginWork( case LazyComponent: { const elementType = workInProgress.elementType; return mountLazyComponent( + root, current, workInProgress, elementType, @@ -3133,6 +3161,7 @@ function beginWork( ? unresolvedProps : resolveDefaultProps(Component, unresolvedProps); return updateFunctionComponent( + root, current, workInProgress, Component, @@ -3181,6 +3210,7 @@ function beginWork( ? unresolvedProps : resolveDefaultProps(type, unresolvedProps); return updateForwardRef( + root, current, workInProgress, type, @@ -3227,6 +3257,7 @@ function beginWork( } resolvedProps = resolveDefaultProps(type.type, resolvedProps); return updateMemoComponent( + root, current, workInProgress, type, @@ -3237,6 +3268,7 @@ function beginWork( } case SimpleMemoComponent: { return updateSimpleMemoComponent( + root, current, workInProgress, workInProgress.type, @@ -3292,6 +3324,7 @@ function beginWork( const chunk = workInProgress.type; const props = workInProgress.pendingProps; return updateChunk( + root, current, workInProgress, chunk, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 9d9d220f33d52..34c7ae3deac2e 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -356,7 +356,9 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) { if ((effect.tag & tag) === tag) { // Mount const create = effect.create; - effect.destroy = create(); + if (typeof create === 'function') { + effect.destroy = create(); + } if (__DEV__) { const destroy = effect.destroy; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 10faead460176..f5a0fd44b5374 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -13,6 +13,7 @@ import type { ReactEventResponderListener, } from 'shared/ReactTypes'; import type {Fiber} from './ReactFiber'; +import type {FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {HookEffectTag} from './ReactHookEffectTags'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; @@ -22,8 +23,8 @@ import type { MutableSourceHookConfig, } from 'shared/ReactMutableSource'; +import {isPrimaryRenderer} from './ReactFiberHostConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {HostRoot} from 'shared/ReactWorkTags'; import {NoWork, Sync} from './ReactFiberExpirationTime'; import {readContext} from './ReactFiberNewContext'; @@ -46,8 +47,6 @@ import { warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, markUnprocessedUpdateTime, - getMutableSourcePendingExpirationTime, - warnAboutUpdateOnUnmountedFiberInDEV, } from './ReactFiberWorkLoop'; import invariant from 'shared/invariant'; @@ -62,6 +61,7 @@ import { getCurrentPriorityLevel, } from './SchedulerWithReactIntegration'; import { + getPendingExpirationTime, getWorkInProgressVersion, setWorkInProgressVersion, } from 'shared/ReactMutableSource'; @@ -108,10 +108,10 @@ export type Dispatcher = {| useTransition( config: SuspenseConfig | void | null, ): [(() => void) => void, boolean], - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S, + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot, |}; type Update = {| @@ -162,7 +162,7 @@ export type Hook = {| export type Effect = {| tag: HookEffectTag, - create: () => (() => void) | void, + create: (() => (() => void) | void) | void, destroy: (() => void) | void, deps: Array | null, next: Effect, @@ -184,6 +184,10 @@ let renderExpirationTime: ExpirationTime = NoWork; // the work-in-progress hook. let currentlyRenderingFiber: Fiber = (null: any); +// The work-in-progress root fiber. +// Used by mutable source hook to detect tears from pending updates. +let currentlyRenderingRoot: FiberRoot = (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 // work-in-progress hook list is a new list that will be added to the @@ -362,6 +366,7 @@ function areHookInputsEqual( export function renderWithHooks( current: Fiber | null, workInProgress: Fiber, + root: FiberRoot, Component: any, props: any, secondArg: any, @@ -369,6 +374,7 @@ export function renderWithHooks( ): any { renderExpirationTime = nextRenderExpirationTime; currentlyRenderingFiber = workInProgress; + currentlyRenderingRoot = root; if (__DEV__) { hookTypesDev = @@ -475,6 +481,7 @@ export function renderWithHooks( renderExpirationTime = NoWork; currentlyRenderingFiber = (null: any); + currentlyRenderingRoot = (null: any); currentHook = null; workInProgressHook = null; @@ -853,42 +860,31 @@ function rerenderReducer( return [newState, dispatch]; } -type MutableSourceState = {| - source: MutableSource, - config: MutableSourceHookConfig, - snapshot: S, +type MutableSourceState = {| + config: MutableSourceHookConfig, destroy?: () => void, + snapshot: Snapshot, + source: MutableSource, |}; -function useMutableSourceImpl( +function useMutableSourceImpl( hook: Hook, - source: MutableSource, - config: MutableSourceHookConfig, -): S { - const {getSnapshot, getVersion, subscribe} = config; - const version = getVersion(); - const pendingExpirationTime = getMutableSourcePendingExpirationTime(source); - - let isSafeToReadFromSource = false; + source: MutableSource, + config: MutableSourceHookConfig, +): Snapshot { + const {getSnapshot, subscribe} = config; + const version = source._getVersion(); + const pendingExpirationTime = getPendingExpirationTime( + currentlyRenderingRoot, + source, + ); // Is it safe to read from this source during the current render? - // If the source has pending updates, we can use the current render's expiration - // time to determine if it's safe to read again from the source. - // If there are no pending updates, we can use the work-in-progress version. - if (pendingExpirationTime === null) { - const lastReadVersion = getWorkInProgressVersion(source); - if (lastReadVersion === null) { - // This is the only case where we need to actually update the version number. - // A changed version number for a new source will throw and restart the render, - // at which point the work-in-progress version Map will be reinitialized. - // Once a source has been subscribed to, we use expiration time to determine safety. - setWorkInProgressVersion(source, version); + let isSafeToReadFromSource = false; - isSafeToReadFromSource = true; - } else { - isSafeToReadFromSource = lastReadVersion === version; - } - } else { + if (pendingExpirationTime !== null) { + // If the source has pending updates, we can use the current render's expiration + // time to determine if it's safe to read again from the source. const currentTime = requestCurrentTimeForUpdate(); const suspenseConfig = requestCurrentSuspenseConfig(); const expirationTime = computeExpirationForFiber( @@ -906,13 +902,31 @@ function useMutableSourceImpl( currentlyRenderingFiber.expirationTime = pendingExpirationTime; markUnprocessedUpdateTime(pendingExpirationTime); } + } else { + // If there are no pending updates, we should still check the work-in-progress version. + // It's possible this source (or the part we are reading from) has just not yet been subscribed to. + const lastReadVersion = getWorkInProgressVersion(source, isPrimaryRenderer); + if (lastReadVersion === null) { + // This is the only case where we need to actually update the version number. + // A changed version number for a new source will throw and restart the render, + // at which point the work-in-progress version Map will be reinitialized. + // Once a source has been subscribed to, we use expiration time to determine safety. + setWorkInProgressVersion(source, version, isPrimaryRenderer); + + isSafeToReadFromSource = true; + } else { + isSafeToReadFromSource = lastReadVersion === version; + } } - let prevMemoizedState = ((hook.memoizedState: any): ?MutableSourceState); - let snapshot = ((null: any): S); + let prevMemoizedState = ((hook.memoizedState: any): ?MutableSourceState< + Source, + Snapshot, + >); + let snapshot = ((null: any): Snapshot); if (isSafeToReadFromSource) { - snapshot = getSnapshot(); + snapshot = getSnapshot(source._source); } else { // Since we can't read safely, can we reuse a previous snapshot? if ( @@ -941,7 +955,7 @@ function useMutableSourceImpl( const destroy = prevMemoizedState != null ? prevMemoizedState.destroy : undefined; - const memoizedState: MutableSourceState = { + const memoizedState: MutableSourceState = { config, destroy, snapshot, @@ -952,24 +966,10 @@ function useMutableSourceImpl( if (prevSource !== source) { const fiber = currentlyRenderingFiber; + const root = currentlyRenderingRoot; const create = () => { const scheduleUpdate = () => { - let node = fiber; - let root = null; - while (node !== null) { - if (node.tag === HostRoot) { - root = node.stateNode; - break; - } - node = node.return; - } - - if (root === null) { - warnAboutUpdateOnUnmountedFiberInDEV(fiber); - return; - } - const alreadyScheduledExpirationTime = root.mutableSourcePendingUpdateMap.get( source, ); @@ -996,13 +996,13 @@ function useMutableSourceImpl( // Was the source mutated between when we rendered and when we're subscribing? // If so, we also need to schedule an update. - const maybeNewSnapshot = getSnapshot(); + const maybeNewSnapshot = getSnapshot(source._source); if (snapshot !== maybeNewSnapshot) { scheduleUpdate(); } // Unsubscribe on destroy. - memoizedState.destroy = subscribe(scheduleUpdate); + memoizedState.destroy = subscribe(source._source, scheduleUpdate); return memoizedState.destroy; }; @@ -1011,25 +1011,28 @@ function useMutableSourceImpl( // and remove them from the previous source currentlyRenderingFiber.effectTag |= UpdateEffect | PassiveEffect; pushEffect(HookHasEffect | HookPassive, create, destroy, null); + } else { + // Carry forward the Passive effect flag so we remember to unsubscribe on future unmount. + pushEffect(HookPassive, undefined, destroy, null); } return snapshot; } -function mountMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, -): S { +function mountMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, +): Snapshot { const hook = mountWorkInProgressHook(); - return useMutableSourceImpl(hook, source, config); + return useMutableSourceImpl(hook, source, config); } -function updateMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, -): S { +function updateMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, +): Snapshot { const hook = updateWorkInProgressHook(); - return useMutableSourceImpl(hook, source, config); + return useMutableSourceImpl(hook, source, config); } function mountState( @@ -1787,13 +1790,13 @@ if (__DEV__) { mountHookTypesDev(); return mountTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; mountHookTypesDev(); - return mountMutableSource(source, config); + return mountMutableSource(source, config); }, }; @@ -1912,13 +1915,13 @@ if (__DEV__) { updateHookTypesDev(); return mountTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; updateHookTypesDev(); - return updateMutableSource(source, config); + return updateMutableSource(source, config); }, }; @@ -2037,13 +2040,13 @@ if (__DEV__) { updateHookTypesDev(); return updateTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; updateHookTypesDev(); - return updateMutableSource(source, config); + return updateMutableSource(source, config); }, }; @@ -2162,13 +2165,13 @@ if (__DEV__) { updateHookTypesDev(); return rerenderTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; updateHookTypesDev(); - return updateMutableSource(source, config); + return updateMutableSource(source, config); }, }; @@ -2301,14 +2304,14 @@ if (__DEV__) { mountHookTypesDev(); return mountTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; warnInvalidHookAccess(); mountHookTypesDev(); - return mountMutableSource(source, config); + return mountMutableSource(source, config); }, }; @@ -2441,14 +2444,14 @@ if (__DEV__) { updateHookTypesDev(); return updateTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; warnInvalidHookAccess(); updateHookTypesDev(); - return updateMutableSource(source, config); + return updateMutableSource(source, config); }, }; @@ -2581,14 +2584,14 @@ if (__DEV__) { updateHookTypesDev(); return rerenderTransition(config); }, - useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, - ): S { + useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot { currentHookNameInDev = 'useMutableSource'; warnInvalidHookAccess(); updateHookTypesDev(); - return updateMutableSource(source, config); + return updateMutableSource(source, config); }, }; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 67fc9d90c4f42..312db6abcbc7c 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -15,16 +15,11 @@ import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; import type {Effect as HookEffect} from './ReactFiberHooks'; -import type { - MutableSource, - MutableSourceMetadata, -} from 'shared/ReactMutableSource'; import { - initializeWorkInProgressVersionMap as initializeMutableSourceWorkInProgressVersionMap, - resetWorkInProgressVersionMap as resetMutableSourceWorkInProgressVersionMap, + clearPendingUpdates as clearPendingMutableSourceUpdates, + resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions, } from 'shared/ReactMutableSource'; - import { warnAboutDeprecatedLifecycles, deferPassiveEffectCleanupDuringUnmount, @@ -65,6 +60,7 @@ import * as Scheduler from 'scheduler'; import {__interactionsRef, __subscriberRef} from 'scheduler/tracing'; import { + isPrimaryRenderer, prepareForCommit, resetAfterCommit, scheduleTimeout, @@ -301,16 +297,6 @@ let spawnedWorkDuringRender: null | Array = null; // receive the same expiration time. Otherwise we get tearing. let currentEventTime: ExpirationTime = NoWork; -export function getMutableSourcePendingExpirationTime( - source: MutableSource, -): ExpirationTime | null { - invariant(workInProgressRoot !== null, 'Expected a work-in-progress root.'); - const expirationTime = workInProgressRoot.mutableSourcePendingUpdateMap.get( - source, - ); - return expirationTime !== undefined ? expirationTime : null; -} - export function requestCurrentTimeForUpdate() { if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { // We're inside React, so it's fine to read the actual time. @@ -1277,7 +1263,7 @@ function prepareFreshStack(root, expirationTime) { workInProgressRootNextUnprocessedUpdateTime = NoWork; workInProgressRootHasPendingPing = false; - initializeMutableSourceWorkInProgressVersionMap(); + resetMutableSourceWorkInProgressVersions(isPrimaryRenderer); if (enableSchedulerTracing) { spawnedWorkDuringRender = null; @@ -1492,10 +1478,20 @@ function performUnitOfWork(unitOfWork: Fiber): Fiber | null { let next; if (enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode) { startProfilerTimer(unitOfWork); - next = beginWork(current, unitOfWork, renderExpirationTime); + next = beginWork( + ((workInProgressRoot: any): FiberRoot), + current, + unitOfWork, + renderExpirationTime, + ); stopProfilerTimerIfRunningAndRecordDelta(unitOfWork, true); } else { - next = beginWork(current, unitOfWork, renderExpirationTime); + next = beginWork( + ((workInProgressRoot: any): FiberRoot), + current, + unitOfWork, + renderExpirationTime, + ); } resetCurrentDebugFiberInDEV(); @@ -2016,16 +2012,8 @@ function commitRootImpl(root, renderPriorityLevel) { nestedUpdateCount = 0; } - // Remove pending mutable source entries that we've completed processing. - root.mutableSourcePendingUpdateMap.forEach( - (pendingExpirationTime, source) => { - if (pendingExpirationTime <= expirationTime) { - root.mutableSourcePendingUpdateMap.delete(source); - } - }, - ); - - resetMutableSourceWorkInProgressVersionMap(); + // Clear any pending updates that were just processed. + clearPendingMutableSourceUpdates(root, expirationTime); onCommitRoot(finishedWork.stateNode, expirationTime); @@ -2233,7 +2221,9 @@ export function enqueuePendingPassiveHookEffectUnmount( function invokePassiveEffectCreate(effect: HookEffect): void { const create = effect.create; - effect.destroy = create(); + if (typeof create === 'function') { + effect.destroy = create(); + } } function flushPassiveEffectsImpl() { @@ -2308,7 +2298,9 @@ function flushPassiveEffectsImpl() { } else { try { const create = effect.create; - effect.destroy = create(); + if (typeof create === 'function') { + effect.destroy = create(); + } } catch (error) { invariant(fiber !== null, 'Should be working on an effect.'); captureCommitPhaseError(fiber, error); @@ -2693,7 +2685,7 @@ function checkForInterruption( } let didWarnStateUpdateForUnmountedComponent: Set | null = null; -export function warnAboutUpdateOnUnmountedFiberInDEV(fiber: Fiber) { +function warnAboutUpdateOnUnmountedFiberInDEV(fiber: Fiber) { if (__DEV__) { const tag = fiber.tag; if ( @@ -2734,7 +2726,7 @@ export function warnAboutUpdateOnUnmountedFiberInDEV(fiber: Fiber) { let beginWork; if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { let dummyFiber = null; - beginWork = (current, unitOfWork, expirationTime) => { + beginWork = (root, current, unitOfWork, expirationTime) => { // If a component throws an error, we replay it again in a synchronously // dispatched event, so that the debugger will treat it as an uncaught // error See ReactErrorUtils for more information. @@ -2746,7 +2738,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { unitOfWork, ); try { - return originalBeginWork(current, unitOfWork, expirationTime); + return originalBeginWork(root, current, unitOfWork, expirationTime); } catch (originalError) { if ( originalError !== null && @@ -2780,6 +2772,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { null, originalBeginWork, null, + root, current, unitOfWork, expirationTime, diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index c2e6771035b5b..bc49f3feaabf0 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -1903,15 +1903,72 @@ describe('ReactHooksWithNoopRenderer', () => { }); describe('useMutableSource', () => { - function createConfig(source) { + function createConfig() { return { - getVersion: () => source.version, - getSnapshot: () => source.value, - subscribe: callback => source.subscribe(callback), + getSnapshot: source => source.value, + subscribe: (source, callback) => source.subscribe(callback), + }; + } + + function createComplexSource(initialValueA, initialValueB) { + const callbacksA = []; + const callbacksB = []; + let revision = 0; + let valueA = 'a:one'; + let valueB = 'b:one'; + + const subscribeHelper = (callbacks, callback) => { + if (callbacks.indexOf(callback) < 0) { + callbacks.push(callback); + } + return () => { + const index = callbacks.indexOf(callback); + if (index >= 0) { + callbacks.splice(index, 1); + } + }; + }; + + return { + subscribeA(callback) { + subscribeHelper(callbacksA, callback); + }, + subscribeB(callback) { + subscribeHelper(callbacksB, callback); + }, + + get listenerCountA() { + return callbacksA.length; + }, + get listenerCountB() { + return callbacksB.length; + }, + + set valueA(newValue) { + revision++; + valueA = newValue; + callbacksA.forEach(callback => callback()); + }, + get valueA() { + return valueA; + }, + + set valueB(newValue) { + revision++; + valueB = newValue; + callbacksB.forEach(callback => callback()); + }, + get valueB() { + return valueB; + }, + + get version() { + return revision; + }, }; } - function createMutableSource(initialValue) { + function createSource(initialValue) { const callbacks = []; let revision = 0; let value = initialValue; @@ -1944,21 +2001,29 @@ describe('ReactHooksWithNoopRenderer', () => { }; } - function Component({config, label, source}) { - const snapshot = React.useMutableSource(source, config); + function createMutableSource(source) { + return React.createMutableSource(source, { + getVersion: () => source.version, + }); + } + + function Component({config, label, mutableSource}) { + const snapshot = React.useMutableSource(mutableSource, config); Scheduler.unstable_yieldValue(`${label}:${snapshot}`); return
{`${label}:${snapshot}`}
; } it('should subscribe to a source and schedule updates when it changes', () => { - const source = createMutableSource('one'); - const config = createConfig(source); + const source = createSource('one'); + const mutableSource = createMutableSource(source); + const config = createConfig(); - ReactNoop.render( + ReactNoop.renderToRootWithID( - - + + , + 'root', () => Scheduler.unstable_yieldValue('Sync effect'), ); expect(Scheduler).toFlushAndYieldThrough([ @@ -1976,23 +2041,38 @@ describe('ReactHooksWithNoopRenderer', () => { source.value = 'two'; expect(Scheduler).toFlushAndYieldThrough(['a:two', 'b:two']); - // Umounting should remove event listeners - ReactNoop.render(null); + // Umounting a component should remove its subscriptino. + ReactNoop.renderToRootWithID( + + + , + 'root', + () => Scheduler.unstable_yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYield(['a:two', 'Sync effect']); + ReactNoop.flushPassiveEffects(); + expect(source.listenerCount).toBe(1); + + // Umounting a root should remove the remaining event listeners + ReactNoop.unmountRootWithID('root'); expect(Scheduler).toFlushAndYield([]); ReactNoop.flushPassiveEffects(); expect(source.listenerCount).toBe(0); + + // Changes to source should not trigger an updates or warnings. source.value = 'three'; expect(Scheduler).toFlushAndYield([]); }); it('should restart work if a new source is mutated during render', () => { - const source = createMutableSource('one'); - const config = createConfig(source); + const source = createSource('one'); + const mutableSource = createMutableSource(source); + const config = createConfig(); ReactNoop.render( - - + + , () => Scheduler.unstable_yieldValue('Sync effect'), ); @@ -2012,13 +2092,14 @@ describe('ReactHooksWithNoopRenderer', () => { }); it('should schedule an update if a new source is mutated between render and commit (subscription)', () => { - const source = createMutableSource('one'); - const config = createConfig(source); + const source = createSource('one'); + const mutableSource = createMutableSource(source); + const config = createConfig(); ReactNoop.render( - - + + , () => Scheduler.unstable_yieldValue('Sync effect'), ); @@ -2039,14 +2120,20 @@ describe('ReactHooksWithNoopRenderer', () => { }); it('should unsubscribe and resubscribe if a new source is used', () => { - const sourceA = createMutableSource('a-one'); - const configA = createConfig(sourceA); + const sourceA = createSource('a-one'); + const mutableSourceA = createMutableSource(sourceA); + const configA = createConfig(); - const sourceB = createMutableSource('b-one'); - const configB = createConfig(sourceB); + const sourceB = createSource('b-one'); + const mutableSourceB = createMutableSource(sourceB); + const configB = createConfig(); ReactNoop.render( - , + , () => Scheduler.unstable_yieldValue('Sync effect'), ); expect(Scheduler).toFlushAndYieldThrough(['only:a-one', 'Sync effect']); @@ -2059,7 +2146,11 @@ describe('ReactHooksWithNoopRenderer', () => { // If we re-render with a new source, the old one should be unsubscribed. ReactNoop.render( - , + , () => Scheduler.unstable_yieldValue('Sync effect'), ); expect(Scheduler).toFlushAndYield(['only:b-one', 'Sync effect']); @@ -2077,13 +2168,14 @@ describe('ReactHooksWithNoopRenderer', () => { }); it('should re-use previously read snapshot value when reading is unsafe', () => { - const source = createMutableSource('one'); - const config = createConfig(source); + const source = createSource('one'); + const mutableSource = createMutableSource(source); + const config = createConfig(); ReactNoop.render( - - + + , () => Scheduler.unstable_yieldValue('Sync effect'), ); @@ -2099,8 +2191,16 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.flushSync(() => { ReactNoop.render( - - + + , () => Scheduler.unstable_yieldValue('Sync effect'), ); @@ -2111,12 +2211,13 @@ describe('ReactHooksWithNoopRenderer', () => { }); it('should read from source on newly mounted subtree if no pending updates are scheduled for source', () => { - const source = createMutableSource('one'); - const config = createConfig(source); + const source = createSource('one'); + const mutableSource = createMutableSource(source); + const config = createConfig(); ReactNoop.render( - + , () => Scheduler.unstable_yieldValue('Sync effect'), ); @@ -2124,25 +2225,24 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render( - - + + , () => Scheduler.unstable_yieldValue('Sync effect'), ); expect(Scheduler).toFlushAndYield(['a:one', 'b:one', 'Sync effect']); }); - // TODO (useMutableSource) Re-enable the assertion below! - // The current failure is expected and unrelated to this hook. - xit('should through and restart render if source and snapshot are unavailable during an update', () => { - const source = createMutableSource('one'); - const configA = createConfig(source); - const configB = createConfig(source); + it('should throw and restart render if source and snapshot are unavailable during an update', () => { + const source = createSource('one'); + const mutableSource = createMutableSource(source); + const configA = createConfig(); + const configB = createConfig(); ReactNoop.render( - - + + , () => Scheduler.unstable_yieldValue('Sync effect'), ); @@ -2156,23 +2256,106 @@ describe('ReactHooksWithNoopRenderer', () => { // Force a higher priority render with a new config. // This should signal that the snapshot is not safe and trigger a full re-render. - ReactNoop.flushSync(() => { - ReactNoop.render( - - - - , - () => Scheduler.unstable_yieldValue('Sync effect'), - ); - }); - expect(Scheduler).toHaveYielded(['a:two', 'b:two', 'Sync effect']); + // + // TODO (useMutableSource) Remove toThrow() and reenable toHaveYielded() below. + // The current failure is expected and unrelated to this hook. + expect(() => + ReactNoop.flushSync(() => { + ReactNoop.render( + + + + , + () => Scheduler.unstable_yieldValue('Sync effect'), + ); + }), + ).toThrow('Cannot read from mutable source'); + // expect(Scheduler).toHaveYielded(['a:two', 'b:two', 'Sync effect']); + }); + + it('should only update components whose subscriptions fire', () => { + const source = createComplexSource('one', 'one'); + const mutableSource = createMutableSource(source); + + // Subscribe to part of the store. + const configA = { + getSnapshot: s => s.valueA, + subscribe: (s, callback) => s.subscribeA(callback), + }; + const configB = { + getSnapshot: s => s.valueB, + subscribe: (s, callback) => s.subscribeB(callback), + }; + + ReactNoop.render( + + + + , + () => Scheduler.unstable_yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYield(['a:a:one', 'b:b:one', 'Sync effect']); + + // Changes to part of the store (e.g. A) should not render other parts. + source.valueA = 'a:two'; + expect(Scheduler).toFlushAndYield(['a:a:two']); + source.valueB = 'b:two'; + expect(Scheduler).toFlushAndYield(['b:b:two']); }); - // TODO (useMutableSource) Test case for a scoped subscription, - // followed by a render that reads from a different part of the store, - // with a mutation between its first and second read, - // that isn't picked up on by the scoped subscription, - // to verify that we also use the version number to protect against this case. + it('should detect tearing in part of the store not yet subscribed to', () => { + const source = createComplexSource('one', 'one'); + const mutableSource = createMutableSource(source); + + // Subscribe to part of the store. + const configA = { + getSnapshot: s => s.valueA, + subscribe: (s, callback) => s.subscribeA(callback), + }; + const configB = { + getSnapshot: s => s.valueB, + subscribe: (s, callback) => s.subscribeB(callback), + }; + + ReactNoop.render( + + + , + () => Scheduler.unstable_yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYield(['a:a:one', 'Sync effect']); + + // Because the store has not chagned yet, there are no pending updates, + // so it is considered safe to read from when we start this render. + ReactNoop.render( + + + + + , + () => Scheduler.unstable_yieldValue('Sync effect'), + ); + expect(Scheduler).toFlushAndYieldThrough(['a:a:one', 'b:b:one']); + + // Mutating the source should trigger a tear detection on the next read, + // which should throw and re-render the entire tree. + source.valueB = 'b:two'; + + expect(Scheduler).toFlushAndYield([ + 'a:a:one', + 'b:b:two', + 'c:b:two', + 'Sync effect', + ]); + }); }); describe('useCallback', () => { diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 41eb2af9c97a9..5ea8bebbc5bc8 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -319,13 +319,13 @@ class ReactShallowRenderer { }; // TODO: implement if we decide to keep the shallow renderer - const useMutableSource = ( - source: MutableSource, - config: MutableSourceHookConfig, - ): V => { + const useMutableSource = ( + source: MutableSource, + config: MutableSourceHookConfig, + ): Snapshot => { this._validateCurrentlyRenderingComponent(); const getSnapshot = config.getSnapshot; - return getSnapshot(); + return getSnapshot(source._source); }; const useMemo = ( diff --git a/packages/react/src/React.js b/packages/react/src/React.js index 7190dd7cec4ea..81aabb01df08d 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -58,6 +58,7 @@ import ReactSharedInternals from './ReactSharedInternals'; import createFundamental from 'shared/createFundamentalComponent'; import createResponder from 'shared/createEventResponder'; import createScope from 'shared/createScope'; +import {createMutableSource} from 'shared/ReactMutableSource'; import { enableJSXTransformAPI, enableDeprecatedFlareAPI, @@ -81,6 +82,7 @@ const React = { PureComponent, createContext, + createMutableSource, forwardRef, lazy, memo, diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 7413a825136c3..98bfd8e0a8ac8 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -182,10 +182,10 @@ export function useDeferredValue(value: T, config: ?Object): T { return dispatcher.useDeferredValue(value, config); } -export function useMutableSource( - source: MutableSource, - config: MutableSourceHookConfig, -): S { +export function useMutableSource( + source: MutableSource, + config: MutableSourceHookConfig, +): Snapshot { const dispatcher = resolveDispatcher(); return dispatcher.useMutableSource(source, config); } diff --git a/packages/shared/ReactMutableSource.js b/packages/shared/ReactMutableSource.js index 21113965b40d2..d10335b5c6a0b 100644 --- a/packages/shared/ReactMutableSource.js +++ b/packages/shared/ReactMutableSource.js @@ -8,66 +8,127 @@ */ import type {ExpirationTime} from 'react-reconciler/src/ReactFiberExpirationTime'; +import type {FiberRoot} from 'react-reconciler/src/ReactFiberRoot'; -import invariant from 'shared/invariant'; - -export type MutableSource = $NonMaybeType; +// Mutable source version can be anything (e.g. number, string, immutable data structure) +// so long as it changes every time any part of the source changes. export type Version = $NonMaybeType; -export type MutableSourceHookConfig = {| - getSnapshot: () => S, - getVersion: () => $NonMaybeType, - subscribe: (callback: Function) => () => void, -|}; - // Tracks expiration time for all mutable sources with pending updates. // Used to determine if a source is safe to read during updates. // If there are no entries in this map for a given source, // or if the current render’s expiration time is ≤ this value, // it is safe to read from the source without tearing. -export type MutableSourcePendingUpdateMap = Map; +export type MutableSourcePendingUpdateMap = Map< + MutableSource, + ExpirationTime, +>; + +type MutableSourceConfig = {| + getVersion: () => Version, +|}; + +export type MutableSource> = {| + _source: Source, + + _getVersion: () => Version, + + // Tracks the version of this source at the time it was most recently read. + // Used to determine if a source is safe to read from before it has been subscribed to. + // Version number is only used during mount, + // since the mechanism for determining safety after subscription is expiration time. + // + // As a workaround to support multiple concurrent renderers, + // we categorize some renderers as primary and others as secondary. + // We only expect there to be two concurrent renderers at most: + // React Native (primary) and Fabric (secondary); + // React DOM (primary) and React ART (secondary). + // Secondary renderers store their context values on separate fields. + // We use the same approach for Context. + _workInProgressVersionPrimary: null | Version, + _workInProgressVersionSecondary: null | Version, +|}; + +export type MutableSourceHookConfig, Snapshot> = {| + getSnapshot: (source: Source) => Snapshot, + subscribe: (source: Source, callback: Function) => () => void, +|}; -// Tracks the version of each source at the time it was most recently read. -// Used to determine if a source is safe to read from before it has been subscribed to. -// Version number is only used for sources that have not yet been subscribed to, -// since the mechanism for determining safety after subscription is expiration time. -type MutableSourceWorkInProgressVersionMap = Map; +// Work in progress version numbers only apply to a single render, +// and should be reset before starting a new render. +// This tracks which mutable sources need to be reset after a render. +let workInProgressPrimarySources: Array> = []; +let workInProgressSecondarySources: Array> = []; -let workInProgressVersionMap: null | MutableSourceWorkInProgressVersionMap = null; +export function createMutableSource>( + source: Source, + config: MutableSourceConfig, +): MutableSource { + return { + _getVersion: config.getVersion, + _source: source, + _workInProgressVersionPrimary: null, + _workInProgressVersionSecondary: null, + }; +} + +export function clearPendingUpdates( + root: FiberRoot, + expirationTime: ExpirationTime, +): void { + // Remove pending mutable source entries that we've completed processing. + root.mutableSourcePendingUpdateMap.forEach( + (pendingExpirationTime, mutableSource) => { + if (pendingExpirationTime <= expirationTime) { + root.mutableSourcePendingUpdateMap.delete(mutableSource); + } + }, + ); +} -export function resetWorkInProgressVersionMap(): void { - workInProgressVersionMap = null; +export function resetWorkInProgressVersions(isPrimaryRenderer: boolean): void { + if (isPrimaryRenderer) { + workInProgressPrimarySources.forEach(mutableSource => { + mutableSource._workInProgressVersionPrimary = null; + }); + workInProgressPrimarySources.length = 0; + } else { + workInProgressSecondarySources.forEach(mutableSource => { + mutableSource._workInProgressVersionSecondary = null; + }); + workInProgressSecondarySources.length = 0; + } } -export function initializeWorkInProgressVersionMap(): void { - workInProgressVersionMap = new Map(); +export function getPendingExpirationTime( + root: FiberRoot, + source: MutableSource, +): ExpirationTime | null { + const expirationTime = root.mutableSourcePendingUpdateMap.get(source); + return expirationTime !== undefined ? expirationTime : null; } export function getWorkInProgressVersion( - source: MutableSource, + mutableSource: MutableSource, + isPrimaryRenderer: boolean, ): null | Version { - invariant( - workInProgressVersionMap !== null, - 'Expected a work-in-progress version map.', - ); - - const version = ((workInProgressVersionMap: any): MutableSourceWorkInProgressVersionMap).get( - source, - ); - return version === undefined ? null : version; + if (isPrimaryRenderer) { + return mutableSource._workInProgressVersionPrimary; + } else { + return mutableSource._workInProgressVersionSecondary; + } } export function setWorkInProgressVersion( - source: MutableSource, + mutableSource: MutableSource, version: Version, + isPrimaryRenderer: boolean, ): void { - invariant( - workInProgressVersionMap !== null, - 'Expected a work-in-progress version map.', - ); - - ((workInProgressVersionMap: any): MutableSourceWorkInProgressVersionMap).set( - source, - version, - ); + if (isPrimaryRenderer) { + mutableSource._workInProgressVersionPrimary = version; + workInProgressPrimarySources.push(mutableSource); + } else { + mutableSource._workInProgressVersionSecondary = version; + workInProgressSecondarySources.push(mutableSource); + } } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 7aa89986a7f99..654b283dc676e 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -345,7 +345,5 @@ "344": "Expected prepareToHydrateHostSuspenseInstance() to never be called. This error is likely caused by a bug in React. Please file an issue.", "345": "Root did not complete. This is a bug in React.", "346": "An event responder context was used outside of an event cycle.", - "347": "Maps are not valid as a React child (found: %s). Consider converting children to an array of keyed ReactElements instead.", - "348": "Expected a work-in-progress root.", - "349": "Expected a work-in-progress version map." + "347": "Maps are not valid as a React child (found: %s). Consider converting children to an array of keyed ReactElements instead." }