From 78add2dc639e4a8aaa9ea77bec64e45fac6aa11b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 23 Nov 2016 15:30:00 -0800 Subject: [PATCH] Update root children using the appendChild/insertBefore/removeChild methods 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. --- scripts/fiber/tests-failing.txt | 1 - scripts/fiber/tests-passing.txt | 1 + src/renderers/dom/fiber/ReactDOMFiber.js | 30 +++++----- src/renderers/noop/ReactNoop.js | 10 +--- .../shared/fiber/ReactFiberBeginWork.js | 19 ++++++- .../shared/fiber/ReactFiberCommitWork.js | 55 +++++++++++-------- .../shared/fiber/ReactFiberCompleteWork.js | 8 +-- .../shared/fiber/ReactFiberReconciler.js | 12 +--- 8 files changed, 76 insertions(+), 60 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 8084ad6f82..63a7de9dff 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -120,4 +120,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 diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 52430274b6..cd096d16be 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1560,6 +1560,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 diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index f09f9ddbe1..1bd071d04f 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -38,6 +38,8 @@ var { } = ReactDOMFiberComponent; var { precacheFiberNode } = ReactDOMComponentTree; +const DOCUMENT_NODE = 9; + ReactDOMInjection.inject(); ReactControlledComponent.injection.injectFiberControlledHostComponent( ReactDOMFiberComponent @@ -88,19 +90,13 @@ var DOMRenderer = ReactFiberReconciler({ eventsEnabled = null; }, - updateContainer(container : Container, children : HostChildren) : void { - // TODO: Containers should update similarly to other parents. - container.innerHTML = ''; - recursivelyAppendChildren(container, children); - }, - createInstance( type : string, props : Props, children : HostChildren, internalInstanceHandle : Object ) : Instance { - const root = document.body; // HACK + const root = document.documentElement; // HACK const domElement : Instance = createElement(type, props, root); precacheFiberNode(internalInstanceHandle, domElement); @@ -124,7 +120,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); @@ -141,19 +137,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); }, @@ -177,9 +173,15 @@ function warnAboutUnstableUse() { warned = true; } -function renderSubtreeIntoContainer(parentComponent : ?ReactComponent, element : ReactElement, container : DOMContainerElement, callback: ?Function) { +function renderSubtreeIntoContainer(parentComponent : ?ReactComponent, element : ReactElement, containerNode : DOMContainerElement | Document, callback: ?Function) { + let container : DOMContainerElement = + containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any); 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); @@ -194,12 +196,12 @@ var ReactDOM = { return renderSubtreeIntoContainer(null, element, container, callback); }, - unstable_renderSubtreeIntoContainer(parentComponent : ReactComponent, element : ReactElement, container : DOMContainerElement, callback: ?Function) { + unstable_renderSubtreeIntoContainer(parentComponent : ReactComponent, element : ReactElement, containerNode : DOMContainerElement | Document, callback: ?Function) { invariant( parentComponent != null && ReactInstanceMap.has(parentComponent), 'parentComponent must be a valid React Component' ); - return renderSubtreeIntoContainer(parentComponent, element, container, callback); + return renderSubtreeIntoContainer(parentComponent, element, containerNode, callback); }, unmountComponentAtNode(container : DOMContainerElement) { diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 5bd32f08c7..febe0b2c77 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -67,10 +67,6 @@ function flattenChildren(children : HostChildren) { var NoopRenderer = ReactFiberReconciler({ - updateContainer(containerInfo : Container, children : HostChildren) : void { - containerInfo.children = flattenChildren(children); - }, - createInstance(type : string, props : Props, children : HostChildren) : Instance { const inst = { tag: TERMINAL_TAG, @@ -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); @@ -113,7 +109,7 @@ var NoopRenderer = ReactFiberReconciler({ }, insertBefore( - parentInstance : Instance, + parentInstance : Instance | Container, child : Instance | TextInstance, beforeChild : Instance | TextInstance ) : void { @@ -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.'); diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 5d669db552..887d142691 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -300,7 +300,24 @@ module.exports = function( } 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); + } } /* diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 3b195734df..9e00ff0457 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -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'); @@ -37,7 +36,6 @@ module.exports = function( trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void ) { - const updateContainer = config.updateContainer; const commitUpdate = config.commitUpdate; const commitTextUpdate = config.commitTextUpdate; @@ -68,22 +66,30 @@ module.exports = function( } } - 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 @@ -93,7 +99,7 @@ module.exports = function( 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; @@ -142,6 +148,10 @@ module.exports = function( } 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; @@ -201,6 +211,18 @@ module.exports = function( 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) { @@ -260,11 +282,6 @@ module.exports = function( detachRef(current); return; } - case Portal: { - const containerInfo : C = current.stateNode.containerInfo; - updateContainer(containerInfo, null); - return; - } } } @@ -274,14 +291,6 @@ module.exports = function( 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) { @@ -303,10 +312,10 @@ module.exports = function( 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: diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index dc4ab68443..4a8f35b21e 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -167,11 +167,8 @@ module.exports = function(config : HostConfig) { 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; } @@ -253,6 +250,7 @@ module.exports = function(config : HostConfig) { 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; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 15b415f88d..13828107a5 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -47,12 +47,6 @@ type OpaqueNode = Fiber; export type HostConfig = { - // 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) : void, - createInstance(type : T, props : P, children : HostChildren, internalInstanceHandle : OpaqueNode) : I, prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean, commitUpdate(instance : I, oldProps : P, newProps : P, internalInstanceHandle : OpaqueNode) : void, @@ -60,9 +54,9 @@ export type HostConfig = { 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,