From acabf112454e5545205da013266d8529599a2a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sat, 11 Nov 2017 17:00:33 -0800 Subject: [PATCH] Update Flow and Fix Hydration Types (#11493) * Update Flow * Fix createElement() issue The * type was too ambiguous. It's always a string so what's the point? Suppression for missing Flow support for {is: ''} web component argument to createElement() didn't work for some reason. I don't understand what the regex is testing for anyway (a task number?) so I just removed that, and suppression got fixed. * Remove deleted $Abstract<> feature * Expand the unsound isAsync check Flow now errors earlier because it can't find .type on a portal. * Add an unsafe cast for the null State in UpdateQueue * Introduce "hydratable instance" type The Flow error here highlighted a quirk in our typing of hydration. React only really knows about a subset of all possible nodes that can exist in a hydrated tree. Currently we assume that the host renderer filters them out to be either Instance or TextInstance. We also assume that those are different things which they might not be. E.g. it could be fine for a renderer to render "text" as the same type as one of the instances, with some default props. We don't really know what it will be narrowed down to until we call canHydrateInstance or canHydrateTextInstance. That's when the type is truly refined. So to solve this I use a different type for hydratable instance that is used in that temporary stage between us reading it from the DOM and until it gets refined by canHydrate(Text)Instance. * Have the renderer refine Hydratable Instance to Instance or Text Instance Currently we assume that if canHydrateInstance or canHydrateTextInstance returns true, then the types also match up. But we don't tell that to Flow. It just happens to work because `fiber.stateNode` is still `any`. We could potentially use some kind of predicate typing but instead of that I can just return null or instance from the "can" tests. This ensures that the renderer has to do the refinement properly. --- .flowconfig | 6 ++--- package.json | 2 +- packages/react-dom/src/client/ReactDOM.js | 23 +++++++++------- .../src/client/ReactDOMFiberComponent.js | 2 +- .../src/ReactNativeComponent.js | 2 +- .../src/ReactFiberBeginWork.js | 4 +-- .../src/ReactFiberCommitWork.js | 4 +-- .../src/ReactFiberCompleteWork.js | 4 +-- .../src/ReactFiberHostContext.js | 4 +-- .../src/ReactFiberHydrationContext.js | 27 ++++++++++++------- .../src/ReactFiberReconciler.js | 24 ++++++++--------- .../src/ReactFiberScheduler.js | 4 +-- .../src/ReactFiberUpdateQueue.js | 4 +-- yarn.lock | 6 ++--- 14 files changed, 65 insertions(+), 51 deletions(-) diff --git a/.flowconfig b/.flowconfig index a294f39e67..e67a199a57 100644 --- a/.flowconfig +++ b/.flowconfig @@ -36,10 +36,10 @@ suppress_type=$FlowFixMe suppress_type=$FixMe suppress_type=$FlowExpectedError -suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(3[0-3]\\|[1-2][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*www[a-z,_]*\\)?)\\) -suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(3[0-3]\\|[1-2][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*www[a-z,_]*\\)?)\\)?:? #[0-9]+ +suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe +suppress_comment=\\(.\\|\n\\)*\\$FlowIssue suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError [version] -^0.53.1 +^0.57.3 diff --git a/package.json b/package.json index 0c306cdcdd..bfddf3efeb 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "fbjs": "^0.8.16", "fbjs-scripts": "^0.6.0", "filesize": "^3.5.6", - "flow-bin": "^0.53.1", + "flow-bin": "^0.57.3", "git-branch": "^0.3.0", "glob": "^6.0.4", "glob-stream": "^6.1.0", diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 690f5cef99..93f9786fe1 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -459,22 +459,27 @@ const DOMRenderer = ReactFiberReconciler({ instance: Instance | TextInstance, type: string, props: Props, - ): boolean { - return ( - instance.nodeType === ELEMENT_NODE && - type.toLowerCase() === instance.nodeName.toLowerCase() - ); + ): null | Instance { + if ( + instance.nodeType !== ELEMENT_NODE || + type.toLowerCase() !== instance.nodeName.toLowerCase() + ) { + return null; + } + // This has now been refined to an element node. + return ((instance: any): Instance); }, canHydrateTextInstance( instance: Instance | TextInstance, text: string, - ): boolean { - if (text === '') { + ): null | TextInstance { + if (text === '' || instance.nodeType !== TEXT_NODE) { // Empty strings are not parsed by HTML so there won't be a correct match here. - return false; + return null; } - return instance.nodeType === TEXT_NODE; + // This has now been refined to a text node. + return ((instance: any): TextInstance); }, getNextHydratableSibling( diff --git a/packages/react-dom/src/client/ReactDOMFiberComponent.js b/packages/react-dom/src/client/ReactDOMFiberComponent.js index f2e4d147e0..0eabc364af 100644 --- a/packages/react-dom/src/client/ReactDOMFiberComponent.js +++ b/packages/react-dom/src/client/ReactDOMFiberComponent.js @@ -351,7 +351,7 @@ function updateDOMProperties( } export function createElement( - type: *, + type: string, props: Object, rootContainerElement: Element | Document, parentNamespace: string, diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index b41778a2ec..c995e1062a 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -41,7 +41,7 @@ class ReactNativeComponent extends React.Component< Props, State, > { - static defaultProps: $Abstract; + static defaultProps: DefaultProps; props: Props; state: State; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 213ae7cdf8..57aad2ff12 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -64,8 +64,8 @@ if (__DEV__) { var warnedAboutStatelessRefs = {}; } -export default function( - config: HostConfig, +export default function( + config: HostConfig, hostContext: HostContext, hydrationContext: HydrationContext, scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 0480cae2ec..68b990c1d2 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -33,8 +33,8 @@ import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf'; var {invokeGuardedCallback, hasCaughtError, clearCaughtError} = ReactErrorUtils; -export default function( - config: HostConfig, +export default function( + config: HostConfig, captureError: (failedFiber: Fiber, error: mixed) => Fiber | null, ) { const {getPublicInstance, mutation, persistence} = config; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index e0c2e6b7a0..8c83bd476f 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -43,8 +43,8 @@ import { } from './ReactFiberContext'; import {Never} from './ReactFiberExpirationTime'; -export default function( - config: HostConfig, +export default function( + config: HostConfig, hostContext: HostContext, hydrationContext: HydrationContext, ) { diff --git a/packages/react-reconciler/src/ReactFiberHostContext.js b/packages/react-reconciler/src/ReactFiberHostContext.js index 6cb6586dee..4a8e857b4b 100644 --- a/packages/react-reconciler/src/ReactFiberHostContext.js +++ b/packages/react-reconciler/src/ReactFiberHostContext.js @@ -28,8 +28,8 @@ export type HostContext = { resetHostContainer(): void, }; -export default function( - config: HostConfig, +export default function( + config: HostConfig, ): HostContext { const {getChildHostContext, getRootHostContext} = config; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index a0d659ef78..bd3889580a 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -29,8 +29,8 @@ export type HydrationContext = { popHydrationState(fiber: Fiber): boolean, }; -export default function( - config: HostConfig, +export default function( + config: HostConfig, ): HydrationContext { const {shouldSetTextContent, hydration} = config; @@ -82,7 +82,7 @@ export default function( // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. let hydrationParentFiber: null | Fiber = null; - let nextHydratableInstance: null | I | TI = null; + let nextHydratableInstance: null | HI = null; let isHydrating: boolean = false; function enterHydrationState(fiber: Fiber) { @@ -188,16 +188,26 @@ export default function( } } - function canHydrate(fiber, nextInstance) { + function tryHydrate(fiber, nextInstance) { switch (fiber.tag) { case HostComponent: { const type = fiber.type; const props = fiber.pendingProps; - return canHydrateInstance(nextInstance, type, props); + const instance = canHydrateInstance(nextInstance, type, props); + if (instance !== null) { + fiber.stateNode = (instance: I); + return true; + } + return false; } case HostText: { const text = fiber.pendingProps; - return canHydrateTextInstance(nextInstance, text); + const textInstance = canHydrateTextInstance(nextInstance, text); + if (textInstance !== null) { + fiber.stateNode = (textInstance: TI); + return true; + } + return false; } default: return false; @@ -216,12 +226,12 @@ export default function( hydrationParentFiber = fiber; return; } - if (!canHydrate(fiber, nextInstance)) { + if (!tryHydrate(fiber, nextInstance)) { // If we can't hydrate this instance let's try the next one. // We use this as a heuristic. It's based on intuition and not data so it // might be flawed or unnecessary. nextInstance = getNextHydratableSibling(nextInstance); - if (!nextInstance || !canHydrate(fiber, nextInstance)) { + if (!nextInstance || !tryHydrate(fiber, nextInstance)) { // Nothing to hydrate. Make it an insertion. insertNonHydratedInstance((hydrationParentFiber: any), fiber); isHydrating = false; @@ -237,7 +247,6 @@ export default function( nextHydratableInstance, ); } - fiber.stateNode = nextInstance; hydrationParentFiber = fiber; nextHydratableInstance = getFirstHydratableChild(nextInstance); } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 61ff50a20f..eca460c702 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -45,7 +45,7 @@ export type Deadline = { type OpaqueHandle = Fiber; type OpaqueRoot = FiberRoot; -export type HostConfig = { +export type HostConfig = { getRootHostContext(rootContainerInstance: C): CX, getChildHostContext(parentHostContext: CX, type: T, instance: C): CX, getPublicInstance(instance: I | TI): PI, @@ -95,7 +95,7 @@ export type HostConfig = { useSyncScheduling?: boolean, - +hydration?: HydrationHostConfig, + +hydration?: HydrationHostConfig, +mutation?: MutableUpdatesHostConfig, +persistence?: PersistentUpdatesHostConfig, @@ -150,12 +150,12 @@ type PersistentUpdatesHostConfig = { replaceContainerChildren(container: C, newChildren: CC): void, }; -type HydrationHostConfig = { +type HydrationHostConfig = { // Optional hydration - canHydrateInstance(instance: I | TI, type: T, props: P): boolean, - canHydrateTextInstance(instance: I | TI, text: string): boolean, - getNextHydratableSibling(instance: I | TI): null | I | TI, - getFirstHydratableChild(parentInstance: I | C): null | I | TI, + canHydrateInstance(instance: HI, type: T, props: P): null | I, + canHydrateTextInstance(instance: HI, text: string): null | TI, + getNextHydratableSibling(instance: I | TI | HI): null | HI, + getFirstHydratableChild(parentInstance: I | C): null | HI, hydrateInstance( instance: I, type: T, @@ -269,8 +269,8 @@ function getContextForSubtree( : parentContext; } -export default function( - config: HostConfig, +export default function( + config: HostConfig, ): Reconciler { var {getPublicInstance} = config; @@ -324,9 +324,9 @@ export default function( if ( enableAsyncSubtreeAPI && element != null && - element.type != null && - element.type.prototype != null && - (element.type.prototype: any).unstable_isAsyncReactComponent === true + (element: any).type != null && + (element: any).type.prototype != null && + (element: any).type.prototype.unstable_isAsyncReactComponent === true ) { expirationTime = computeAsyncExpiration(); } else { diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 57e6fe63b7..66c05df6cc 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -144,8 +144,8 @@ if (__DEV__) { }; } -export default function( - config: HostConfig, +export default function( + config: HostConfig, ) { const hostContext = ReactFiberHostContext(config); const hydrationContext: HydrationContext = ReactFiberHydrationContext( diff --git a/packages/react-reconciler/src/ReactFiberUpdateQueue.js b/packages/react-reconciler/src/ReactFiberUpdateQueue.js index bc1157b042..674c3ec7b9 100644 --- a/packages/react-reconciler/src/ReactFiberUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberUpdateQueue.js @@ -114,14 +114,14 @@ export function insertUpdateIntoFiber( // It depends on which fiber is the next current. Initialize with an empty // base state, then set to the memoizedState when rendering. Not super // happy with this approach. - queue1 = fiber.updateQueue = createUpdateQueue(null); + queue1 = fiber.updateQueue = createUpdateQueue((null: any)); } let queue2; if (alternateFiber !== null) { queue2 = alternateFiber.updateQueue; if (queue2 === null) { - queue2 = alternateFiber.updateQueue = createUpdateQueue(null); + queue2 = alternateFiber.updateQueue = createUpdateQueue((null: any)); } } else { queue2 = null; diff --git a/yarn.lock b/yarn.lock index 983d7b9d6b..4f6483bc80 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1932,9 +1932,9 @@ flat-cache@^1.2.1: graceful-fs "^4.1.2" write "^0.2.1" -flow-bin@^0.53.1: - version "0.53.1" - resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.53.1.tgz#9b22b63a23c99763ae533ebbab07f88c88c97d84" +flow-bin@^0.57.3: + version "0.57.3" + resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.57.3.tgz#843fb80a821b6d0c5847f7bb3f42365ffe53b27b" for-in@^1.0.1: version "1.0.2"