Skip to content

Commit

Permalink
Update Flow and Fix Hydration Types (#11493)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
sebmarkbage authored Nov 12, 2017
1 parent 13c491a commit acabf11
Show file tree
Hide file tree
Showing 14 changed files with 65 additions and 51 deletions.
6 changes: 3 additions & 3 deletions .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
23 changes: 14 additions & 9 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ function updateDOMProperties(
}

export function createElement(
type: *,
type: string,
props: Object,
rootContainerElement: Element | Document,
parentNamespace: string,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-renderer/src/ReactNativeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ReactNativeComponent<DefaultProps, Props, State> extends React.Component<
Props,
State,
> {
static defaultProps: $Abstract<DefaultProps>;
static defaultProps: DefaultProps;
props: Props;
state: State;

Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ if (__DEV__) {
var warnedAboutStatelessRefs = {};
}

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
hostContext: HostContext<C, CX>,
hydrationContext: HydrationContext<C, CX>,
scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';

var {invokeGuardedCallback, hasCaughtError, clearCaughtError} = ReactErrorUtils;

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
captureError: (failedFiber: Fiber, error: mixed) => Fiber | null,
) {
const {getPublicInstance, mutation, persistence} = config;
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ import {
} from './ReactFiberContext';
import {Never} from './ReactFiberExpirationTime';

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
hostContext: HostContext<C, CX>,
hydrationContext: HydrationContext<C, CX>,
) {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberHostContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export type HostContext<C, CX> = {
resetHostContainer(): void,
};

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
): HostContext<C, CX> {
const {getChildHostContext, getRootHostContext} = config;

Expand Down
27 changes: 18 additions & 9 deletions packages/react-reconciler/src/ReactFiberHydrationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ export type HydrationContext<C, CX> = {
popHydrationState(fiber: Fiber): boolean,
};

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
): HydrationContext<C, CX> {
const {shouldSetTextContent, hydration} = config;

Expand Down Expand Up @@ -82,7 +82,7 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
// 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) {
Expand Down Expand Up @@ -188,16 +188,26 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
}
}

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;
Expand All @@ -216,12 +226,12 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
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;
Expand All @@ -237,7 +247,6 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
nextHydratableInstance,
);
}
fiber.stateNode = nextInstance;
hydrationParentFiber = fiber;
nextHydratableInstance = getFirstHydratableChild(nextInstance);
}
Expand Down
24 changes: 12 additions & 12 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export type Deadline = {
type OpaqueHandle = Fiber;
type OpaqueRoot = FiberRoot;

export type HostConfig<T, P, I, TI, PI, C, CC, CX, PL> = {
export type HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL> = {
getRootHostContext(rootContainerInstance: C): CX,
getChildHostContext(parentHostContext: CX, type: T, instance: C): CX,
getPublicInstance(instance: I | TI): PI,
Expand Down Expand Up @@ -95,7 +95,7 @@ export type HostConfig<T, P, I, TI, PI, C, CC, CX, PL> = {

useSyncScheduling?: boolean,

+hydration?: HydrationHostConfig<T, P, I, TI, C, CX, PL>,
+hydration?: HydrationHostConfig<T, P, I, TI, HI, C, CX, PL>,

+mutation?: MutableUpdatesHostConfig<T, P, I, TI, C, PL>,
+persistence?: PersistentUpdatesHostConfig<T, P, I, TI, C, CC, PL>,
Expand Down Expand Up @@ -150,12 +150,12 @@ type PersistentUpdatesHostConfig<T, P, I, TI, C, CC, PL> = {
replaceContainerChildren(container: C, newChildren: CC): void,
};

type HydrationHostConfig<T, P, I, TI, C, CX, PL> = {
type HydrationHostConfig<T, P, I, TI, HI, C, CX, PL> = {
// 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,
Expand Down Expand Up @@ -269,8 +269,8 @@ function getContextForSubtree(
: parentContext;
}

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
): Reconciler<C, I, TI> {
var {getPublicInstance} = config;

Expand Down Expand Up @@ -324,9 +324,9 @@ export default function<T, P, I, TI, PI, C, CC, CX, PL>(
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 {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ if (__DEV__) {
};
}

export default function<T, P, I, TI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CC, CX, PL>,
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
) {
const hostContext = ReactFiberHostContext(config);
const hydrationContext: HydrationContext<C, CX> = ReactFiberHydrationContext(
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberUpdateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,14 @@ export function insertUpdateIntoFiber<State>(
// 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;
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit acabf11

Please sign in to comment.