Skip to content

Commit

Permalink
Update root children using the appendChild/insertBefore/removeChild m…
Browse files Browse the repository at this point in the history
…ethods

This removes updateContainer and instead uses the regular child mutation
methods to insert into the root container and portals.

Since we're no longer clearing out the container DOM in updateContainer
we have to do that manually during initial mount. This now works on a
document and one of the tests end up unmounting the body when you render
into the document so I had to work around that bit since we don't yet
properly support rendering into the document root.
  • Loading branch information
sebmarkbage committed Nov 23, 2016
1 parent 7ef856a commit 8dab603
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 60 deletions.
4 changes: 0 additions & 4 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
src/renderers/dom/shared/__tests__/ReactEventListener-test.js
* should batch between handlers from different roots

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should control a value in reentrant events

src/renderers/dom/stack/client/__tests__/ReactDOM-test.js
* throws in render() if the mount callback is not a function
* throws in render() if the update callback is not a function
Expand Down Expand Up @@ -124,4 +121,3 @@ src/renderers/shared/stack/reconciler/__tests__/refs-test.js

src/test/__tests__/ReactTestUtils-test.js
* traverses children in the correct order
* should support injected wrapper components as DOM components
2 changes: 2 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should properly control a value even if no event listener exists
* should control a value in reentrant events
* should control values in reentrant events with different targets
* should display `defaultValue` of number 0
* only assigns defaultValue if it changes
Expand Down Expand Up @@ -1555,6 +1556,7 @@ src/test/__tests__/ReactTestUtils-test.js
* can scryRenderedDOMComponentsWithClass with TextComponent
* can scryRenderedDOMComponentsWithClass with className contains \n
* can scryRenderedDOMComponentsWithClass with multiple classes
* should support injected wrapper components as DOM components
* should change the value of an input field
* should change the value of an input field in a component
* should throw when attempting to use ReactTestUtils.Simulate with shallow rendering
Expand Down
25 changes: 14 additions & 11 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ var {
} = ReactDOMFiberComponent;
var { precacheFiberNode } = ReactDOMComponentTree;

const DOCUMENT_NODE = 9;

ReactDOMInjection.inject();
ReactControlledComponent.injection.injectFiberControlledHostComponent(
ReactDOMFiberComponent
Expand Down Expand Up @@ -75,19 +77,13 @@ function recursivelyAppendChildren(parent : Element, child : HostChildren<Instan

var DOMRenderer = ReactFiberReconciler({

updateContainer(container : Container, children : HostChildren<Instance | TextInstance>) : void {
// TODO: Containers should update similarly to other parents.
container.innerHTML = '';
recursivelyAppendChildren(container, children);
},

createInstance(
type : string,
props : Props,
children : HostChildren<Instance | TextInstance>,
internalInstanceHandle : Object
) : Instance {
const root = document.body; // HACK
const root = document.documentElement; // HACK

const domElement : Instance = createElement(type, props, root);
precacheFiberNode(internalInstanceHandle, domElement);
Expand All @@ -111,7 +107,7 @@ var DOMRenderer = ReactFiberReconciler({
internalInstanceHandle : Object
) : void {
var type = domElement.tagName.toLowerCase(); // HACK
var root = document.body; // HACK
var root = document.documentElement; // HACK
// Update the internal instance handle so that we know which props are
// the current ones.
precacheFiberNode(internalInstanceHandle, domElement);
Expand All @@ -128,19 +124,19 @@ var DOMRenderer = ReactFiberReconciler({
textInstance.nodeValue = newText;
},

appendChild(parentInstance : Instance, child : Instance | TextInstance) : void {
appendChild(parentInstance : Instance | Container, child : Instance | TextInstance) : void {
parentInstance.appendChild(child);
},

insertBefore(
parentInstance : Instance,
parentInstance : Instance | Container,
child : Instance | TextInstance,
beforeChild : Instance | TextInstance
) : void {
parentInstance.insertBefore(child, beforeChild);
},

removeChild(parentInstance : Instance, child : Instance | TextInstance) : void {
removeChild(parentInstance : Instance | Container, child : Instance | TextInstance) : void {
parentInstance.removeChild(child);
},

Expand All @@ -165,8 +161,15 @@ function warnAboutUnstableUse() {
}

function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any, any>, element : ReactElement<any>, container : DOMContainerElement, callback: ?Function) {
if (container.nodeType === DOCUMENT_NODE) {
container = container.documentElement;
}
let root;
if (!container._reactRootContainer) {
// First clear any existing content.
while (container.lastChild) {
container.removeChild(container.lastChild);
}
root = container._reactRootContainer = DOMRenderer.mountContainer(element, container, parentComponent, callback);
} else {
DOMRenderer.updateContainer(element, root = container._reactRootContainer, parentComponent, callback);
Expand Down
10 changes: 3 additions & 7 deletions src/renderers/noop/ReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ function flattenChildren(children : HostChildren<Instance | TextInstance>) {

var NoopRenderer = ReactFiberReconciler({

updateContainer(containerInfo : Container, children : HostChildren<Instance | TextInstance>) : void {
containerInfo.children = flattenChildren(children);
},

createInstance(type : string, props : Props, children : HostChildren<Instance | TextInstance>) : Instance {
const inst = {
tag: TERMINAL_TAG,
Expand Down Expand Up @@ -104,7 +100,7 @@ var NoopRenderer = ReactFiberReconciler({
textInstance.text = newText;
},

appendChild(parentInstance : Instance, child : Instance | TextInstance) : void {
appendChild(parentInstance : Instance | Container, child : Instance | TextInstance) : void {
const index = parentInstance.children.indexOf(child);
if (index !== -1) {
parentInstance.children.splice(index, 1);
Expand All @@ -113,7 +109,7 @@ var NoopRenderer = ReactFiberReconciler({
},

insertBefore(
parentInstance : Instance,
parentInstance : Instance | Container,
child : Instance | TextInstance,
beforeChild : Instance | TextInstance
) : void {
Expand All @@ -128,7 +124,7 @@ var NoopRenderer = ReactFiberReconciler({
parentInstance.children.splice(beforeIndex, 0, child);
},

removeChild(parentInstance : Instance, child : Instance | TextInstance) : void {
removeChild(parentInstance : Instance | Container, child : Instance | TextInstance) : void {
const index = parentInstance.children.indexOf(child);
if (index === -1) {
throw new Error('This child does not exist.');
Expand Down
19 changes: 18 additions & 1 deletion src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,24 @@ module.exports = function<T, P, I, TI, C>(
}

function updatePortalComponent(current, workInProgress) {
reconcileChildren(current, workInProgress, workInProgress.pendingProps);
const priorityLevel = workInProgress.pendingWorkPriority;
const nextChildren = workInProgress.pendingProps;
if (!current) {
// Portals are special because we don't append the children during mount
// but at commit. Therefore we need to track insertions which the normal
// flow doesn't do during mount. This doesn't happen at the root because
// the root always starts with a "current" with a null child.
// TODO: Consider unifying this with how the root works.
workInProgress.child = reconcileChildFibersInPlace(
workInProgress,
workInProgress.child,
nextChildren,
priorityLevel
);
markChildAsProgressed(current, workInProgress, priorityLevel);
} else {
reconcileChildren(current, workInProgress, nextChildren);
}
}

/*
Expand Down
55 changes: 32 additions & 23 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
'use strict';

import type { Fiber } from 'ReactFiber';
import type { FiberRoot } from 'ReactFiberRoot';
import type { HostConfig } from 'ReactFiberReconciler';

var ReactTypeOfWork = require('ReactTypeOfWork');
Expand All @@ -37,7 +36,6 @@ module.exports = function<T, P, I, TI, C>(
trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void
) {

const updateContainer = config.updateContainer;
const commitUpdate = config.commitUpdate;
const commitTextUpdate = config.commitTextUpdate;

Expand Down Expand Up @@ -68,22 +66,30 @@ module.exports = function<T, P, I, TI, C>(
}
}

function getHostParent(fiber : Fiber) : ?I {
function getHostParent(fiber : Fiber) : null | I | C {
let parent = fiber.return;
while (parent) {
switch (parent.tag) {
case HostComponent:
return parent.stateNode;
case HostContainer:
// TODO: Currently we use the updateContainer feature to update these,
// but we should be able to handle this case too.
return null;
return parent.stateNode.containerInfo;
case Portal:
return parent.stateNode.containerInfo;
}
parent = parent.return;
}
return null;
}

function isHostParent(fiber : Fiber) : boolean {
return (
fiber.tag === HostComponent ||
fiber.tag === HostContainer ||
fiber.tag === Portal
);
}

function getHostSibling(fiber : Fiber) : ?I {
// We're going to search forward into the tree until we find a sibling host
// node. Unfortunately, if multiple insertions are done in a row we have to
Expand All @@ -93,7 +99,7 @@ module.exports = function<T, P, I, TI, C>(
siblings: while (true) {
// If we didn't find anything, let's try the next sibling.
while (!node.sibling) {
if (!node.return || node.return.tag === HostComponent) {
if (!node.return || isHostParent(node.return)) {
// If we pop out of the root or hit the parent the fiber we are the
// last sibling.
return null;
Expand Down Expand Up @@ -140,6 +146,10 @@ module.exports = function<T, P, I, TI, C>(
} else {
appendChild(parent, node.stateNode);
}
} else if (node.tag === Portal) {
// If the insertion itself is a portal, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
} else if (node.child) {
// TODO: Coroutines need to visit the stateNode.
node = node.child;
Expand Down Expand Up @@ -197,6 +207,18 @@ module.exports = function<T, P, I, TI, C>(
if (parent) {
removeChild(parent, node.stateNode);
}
} else if (node.tag === Portal) {
// If this is a portal, then the parent is actually the portal itself.
// We need to keep track of which parent we're removing from.
// TODO: This uses a recursive call. We can get rid of that by mutating
// the parent binding and restoring it by searching for the host parent
// again when we pop past a portal.
const portalParent = node.stateNode.containerInfo;
let child = node.child;
while (child) {
unmountHostComponents(portalParent, child);
child = child.sibling;
}
} else {
commitUnmount(node);
if (node.child) {
Expand Down Expand Up @@ -254,11 +276,6 @@ module.exports = function<T, P, I, TI, C>(
detachRef(current);
return;
}
case Portal: {
const containerInfo : C = current.stateNode.containerInfo;
updateContainer(containerInfo, null);
return;
}
}
}

Expand All @@ -268,14 +285,6 @@ module.exports = function<T, P, I, TI, C>(
detachRefIfNeeded(current, finishedWork);
return;
}
case HostContainer: {
// TODO: Attach children to root container.
const children = finishedWork.output;
const root : FiberRoot = finishedWork.stateNode;
const containerInfo : C = root.containerInfo;
updateContainer(containerInfo, children);
return;
}
case HostComponent: {
const instance : I = finishedWork.stateNode;
if (instance != null && current) {
Expand All @@ -297,10 +306,10 @@ module.exports = function<T, P, I, TI, C>(
commitTextUpdate(textInstance, oldText, newText);
return;
}
case HostContainer: {
return;
}
case Portal: {
const children = finishedWork.child;
const containerInfo : C = finishedWork.stateNode.containerInfo;
updateContainer(containerInfo, children);
return;
}
default:
Expand Down
8 changes: 3 additions & 5 deletions src/renderers/shared/fiber/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,8 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
fiberRoot.context = fiberRoot.pendingContext;
fiberRoot.pendingContext = null;
}
// We don't know if a container has updated any children so we always
// need to update it right now. We schedule this side-effect before
// all the other side-effects in the subtree. We need to schedule it
// before so that the entire tree is up-to-date before the life-cycles
// are invoked.
// TODO: Only mark this as an update if we have any pending callbacks
// on it.
markUpdate(workInProgress);
return null;
}
Expand Down Expand Up @@ -253,6 +250,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
transferOutput(workInProgress.child, workInProgress);
return null;
case Portal:
// TODO: Only mark this as an update if we have any pending callbacks.
markUpdate(workInProgress);
workInProgress.output = null;
workInProgress.memoizedProps = workInProgress.pendingProps;
Expand Down
12 changes: 3 additions & 9 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,16 @@ type OpaqueNode = Fiber;

export type HostConfig<T, P, I, TI, C> = {

// TODO: We don't currently have a quick way to detect that children didn't
// reorder so we host will always need to check the set. We should make a flag
// or something so that it can bailout easily.

updateContainer(containerInfo : C, children : HostChildren<I | TI>) : void,

createInstance(type : T, props : P, children : HostChildren<I | TI>, internalInstanceHandle : OpaqueNode) : I,
prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean,
commitUpdate(instance : I, oldProps : P, newProps : P, internalInstanceHandle : OpaqueNode) : void,

createTextInstance(text : string, internalInstanceHandle : OpaqueNode) : TI,
commitTextUpdate(textInstance : TI, oldText : string, newText : string) : void,

appendChild(parentInstance : I, child : I | TI) : void,
insertBefore(parentInstance : I, child : I | TI, beforeChild : I | TI) : void,
removeChild(parentInstance : I, child : I | TI) : void,
appendChild(parentInstance : I | C, child : I | TI) : void,
insertBefore(parentInstance : I | C, child : I | TI, beforeChild : I | TI) : void,
removeChild(parentInstance : I | C, child : I | TI) : void,

scheduleAnimationCallback(callback : () => void) : void,
scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void,
Expand Down

0 comments on commit 8dab603

Please sign in to comment.