Skip to content

Commit

Permalink
Have the renderer refine Hydratable Instance to Instance or Text Inst…
Browse files Browse the repository at this point in the history
…ance

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 committed Nov 8, 2017
1 parent 5195083 commit 1a26613
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 17 deletions.
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 @@ -454,22 +454,27 @@ var 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
21 changes: 15 additions & 6 deletions packages/react-reconciler/src/ReactFiberHydrationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,26 @@ module.exports = function<T, P, I, TI, HI, 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 @@ -218,12 +228,12 @@ module.exports = function<T, P, I, TI, HI, 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 @@ -239,7 +249,6 @@ module.exports = function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
nextHydratableInstance,
);
}
fiber.stateNode = nextInstance;
hydrationParentFiber = fiber;
nextHydratableInstance = getFirstHydratableChild(nextInstance);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ type PersistentUpdatesHostConfig<T, P, I, TI, C, CC, PL> = {

type HydrationHostConfig<T, P, I, TI, HI, C, CX, PL> = {
// Optional hydration
canHydrateInstance(instance: HI, type: T, props: P): boolean,
canHydrateTextInstance(instance: HI, text: string): boolean,
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(
Expand Down

0 comments on commit 1a26613

Please sign in to comment.