From 18a9869dbbb7b279a9009c884d1156bfc509b744 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Nov 2016 18:54:24 +0000 Subject: [PATCH 01/22] Test that SVG elements get created with the right namespace --- .../dom/shared/__tests__/ReactDOMSVG-test.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index 30760d3ae88..ba3b1767d77 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -12,12 +12,14 @@ 'use strict'; var React; +var ReactDOM; var ReactDOMServer; describe('ReactDOMSVG', () => { beforeEach(() => { React = require('React'); + ReactDOM = require('ReactDOM'); ReactDOMServer = require('ReactDOMServer'); }); @@ -30,4 +32,24 @@ describe('ReactDOMSVG', () => { expect(markup).toContain('xlink:href="http://i.imgur.com/w7GCRPb.png"'); }); + it('creates elements with the svg namespace', () => { + var node = document.createElement('div'); + var g; + var image; + ReactDOM.render( + + g = el} strokeWidth="5"> + image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + + , + node + ); + expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(g.getAttribute('stroke-width')).toBe('5'); + expect(image.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect( + image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + }); + }); From b8b2638ba47d0671af8dc68735262ccde1132099 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Nov 2016 19:16:10 +0000 Subject: [PATCH 02/22] Pass root to the renderer methods --- src/renderers/dom/fiber/ReactDOMFiber.js | 18 ++++++++++-------- .../dom/shared/ReactDOMFeatureFlags.js | 2 +- .../shared/fiber/ReactFiberCommitWork.js | 2 +- .../shared/fiber/ReactFiberCompleteWork.js | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index c5da108c5a8..12b878358f4 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -76,10 +76,9 @@ var DOMRenderer = ReactFiberReconciler({ createInstance( type : string, props : Props, - internalInstanceHandle : Object + root : any, // TODO + internalInstanceHandle : Object, ) : Instance { - const root = document.documentElement; // HACK - const domElement : Instance = createElement(type, props, root); precacheFiberNode(internalInstanceHandle, domElement); return domElement; @@ -89,9 +88,12 @@ var DOMRenderer = ReactFiberReconciler({ parentInstance.appendChild(child); }, - finalizeInitialChildren(domElement : Instance, type : string, props : Props) : void { - const root = document.documentElement; // HACK - + finalizeInitialChildren( + domElement : Instance, + type : string, + props : Props, + root : any, // TODO + ) : void { setInitialProperties(domElement, type, props, root); }, @@ -107,10 +109,10 @@ var DOMRenderer = ReactFiberReconciler({ domElement : Instance, oldProps : Props, newProps : Props, - internalInstanceHandle : Object + root : any, // TODO + internalInstanceHandle : Object, ) : void { var type = domElement.tagName.toLowerCase(); // HACK - var root = document.documentElement; // HACK // Update the internal instance handle so that we know which props are // the current ones. precacheFiberNode(internalInstanceHandle, domElement); diff --git a/src/renderers/dom/shared/ReactDOMFeatureFlags.js b/src/renderers/dom/shared/ReactDOMFeatureFlags.js index 5e9d93267cf..17bae0a58af 100644 --- a/src/renderers/dom/shared/ReactDOMFeatureFlags.js +++ b/src/renderers/dom/shared/ReactDOMFeatureFlags.js @@ -13,7 +13,7 @@ var ReactDOMFeatureFlags = { useCreateElement: true, - useFiber: false, + useFiber: true, }; module.exports = ReactDOMFeatureFlags; diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 5a2c07c2f42..34261bca57e 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -303,7 +303,7 @@ module.exports = function( // Commit the work prepared earlier. const newProps = finishedWork.memoizedProps; const oldProps = current.memoizedProps; - commitUpdate(instance, oldProps, newProps, finishedWork); + commitUpdate(instance, oldProps, newProps, finishedWork, document.documentElement /* TODO */); } detachRefIfNeeded(current, finishedWork); return; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index a188cf5813e..278857222bc 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -234,9 +234,9 @@ module.exports = function(config : HostConfig) { // or completeWork depending on we want to add then top->down or // bottom->up. Top->down is faster in IE11. // Finally, finalizeInitialChildren here in completeWork. - const instance = createInstance(workInProgress.type, newProps, workInProgress); + const instance = createInstance(workInProgress.type, newProps, document.documentElement /* TODO */, workInProgress); appendAllChildren(instance, workInProgress); - finalizeInitialChildren(instance, workInProgress.type, newProps); + finalizeInitialChildren(instance, workInProgress.type, newProps, document.documentElement /* TODO */); workInProgress.stateNode = instance; if (workInProgress.ref) { From a450a0844c384d32ac63800e96cd282a438881f2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Nov 2016 21:03:42 +0000 Subject: [PATCH 03/22] Keep track of host instances and containers --- src/renderers/dom/fiber/ReactDOMFiber.js | 18 +++-- .../dom/fiber/ReactDOMFiberComponent.js | 4 +- .../dom/shared/ReactDOMFeatureFlags.js | 2 +- .../dom/shared/__tests__/ReactDOMSVG-test.js | 33 ++++++-- src/renderers/noop/ReactNoop.js | 4 + .../shared/fiber/ReactFiberBeginWork.js | 49 +++++++++++- .../shared/fiber/ReactFiberCommitWork.js | 5 +- .../shared/fiber/ReactFiberCompleteWork.js | 59 +++++--------- .../shared/fiber/ReactFiberHostContext.js | 78 +++++++++++++++++++ .../shared/fiber/ReactFiberScheduler.js | 2 + 10 files changed, 195 insertions(+), 59 deletions(-) create mode 100644 src/renderers/shared/fiber/ReactFiberHostContext.js diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 12b878358f4..f9f3e624c63 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -33,6 +33,7 @@ var warning = require('warning'); var { createElement, + isNewHostContainer, setInitialProperties, updateProperties, } = ReactDOMFiberComponent; @@ -60,6 +61,11 @@ let selectionInformation : ?mixed = null; var DOMRenderer = ReactFiberReconciler({ + isContainerType(type : string) { + type = type.toLowerCase(); // TODO + return isNewHostContainer(type); + }, + prepareForCommit() : void { eventsEnabled = ReactBrowserEventEmitter.isEnabled(); ReactBrowserEventEmitter.setEnabled(false); @@ -76,10 +82,10 @@ var DOMRenderer = ReactFiberReconciler({ createInstance( type : string, props : Props, - root : any, // TODO + containerInstance : Instance | Container, internalInstanceHandle : Object, ) : Instance { - const domElement : Instance = createElement(type, props, root); + const domElement : Instance = createElement(type, props, containerInstance); precacheFiberNode(internalInstanceHandle, domElement); return domElement; }, @@ -92,9 +98,9 @@ var DOMRenderer = ReactFiberReconciler({ domElement : Instance, type : string, props : Props, - root : any, // TODO + containerInstance : Instance | Container, ) : void { - setInitialProperties(domElement, type, props, root); + setInitialProperties(domElement, type, props, containerInstance); }, prepareUpdate( @@ -109,14 +115,14 @@ var DOMRenderer = ReactFiberReconciler({ domElement : Instance, oldProps : Props, newProps : Props, - root : any, // TODO + containerInstance : Instance | Container, internalInstanceHandle : Object, ) : void { var type = domElement.tagName.toLowerCase(); // HACK // Update the internal instance handle so that we know which props are // the current ones. precacheFiberNode(internalInstanceHandle, domElement); - updateProperties(domElement, type, oldProps, newProps, root); + updateProperties(domElement, type, oldProps, newProps, containerInstance); }, createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance { diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 7c6ebd9b3c6..7144448e2b7 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -453,8 +453,6 @@ function updateDOMProperties( var ReactDOMFiberComponent = { - // TODO: Use this to keep track of changes to the host context and use this - // to determine whether we switch to svg and back. // TODO: Does this need to check the current namespace? In case these tags // happen to be valid in some other namespace. isNewHostContainer(tag : string) { @@ -475,7 +473,7 @@ var ReactDOMFiberComponent = { var namespaceURI = rootContainerElement.namespaceURI; if (namespaceURI == null || namespaceURI === DOMNamespaces.svg && - rootContainerElement.tagName === 'foreignObject') { + rootContainerElement.tagName === 'foreignobject') { // TODO: lowercase? namespaceURI = DOMNamespaces.html; } if (namespaceURI === DOMNamespaces.html) { diff --git a/src/renderers/dom/shared/ReactDOMFeatureFlags.js b/src/renderers/dom/shared/ReactDOMFeatureFlags.js index 17bae0a58af..5e9d93267cf 100644 --- a/src/renderers/dom/shared/ReactDOMFeatureFlags.js +++ b/src/renderers/dom/shared/ReactDOMFeatureFlags.js @@ -13,7 +13,7 @@ var ReactDOMFeatureFlags = { useCreateElement: true, - useFiber: true, + useFiber: false, }; module.exports = ReactDOMFeatureFlags; diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index ba3b1767d77..a4b4fca6cd8 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -32,16 +32,26 @@ describe('ReactDOMSVG', () => { expect(markup).toContain('xlink:href="http://i.imgur.com/w7GCRPb.png"'); }); - it('creates elements with the svg namespace', () => { + it('creates elements with svg namespace inside svg tag', () => { var node = document.createElement('div'); - var g; - var image; + var div, foreignDiv, g, image, image2, p; ReactDOM.render( - - g = el} strokeWidth="5"> - image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> - - , +
+ + g = el} strokeWidth="5"> + image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + +
foreignDiv = el} /> + + + +

p = el}> + + image2 = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + +

+
div = el} /> +
, node ); expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); @@ -50,6 +60,13 @@ describe('ReactDOMSVG', () => { expect( image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect( + image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(p.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); }); }); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 9f614a465ea..4be3af9ab8c 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -40,6 +40,10 @@ var instanceCounter = 0; var NoopRenderer = ReactFiberReconciler({ + isContainerType() { + return false; + }, + createInstance(type : string, props : Props) : Instance { const inst = { id: instanceCounter++, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 887d142691c..c1d201b672f 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -34,6 +34,14 @@ var { pushTopLevelContextObject, resetContext, } = require('ReactFiberContext'); +var { + getHostContainerOnStack, + getHostFiberOnStack, + pushHostContainer, + pushHostFiber, + getCurrentRoot, + setCurrentRoot, +} = require('ReactFiberHostContext'); var { IndeterminateComponent, FunctionalComponent, @@ -62,6 +70,12 @@ module.exports = function( scheduleUpdate : (fiber: Fiber) => void ) { + const { + appendInitialChild, + createInstance, + isContainerType, + } = config; + const { adoptClassInstance, constructClassInstance, @@ -254,6 +268,29 @@ module.exports = function( // Abort and don't process children yet. return null; } else { + if (!current && workInProgress.stateNode == null) { + const newProps = workInProgress.pendingProps; + const hostParentFiber = getHostFiberOnStack(); + const hostParentInstance = hostParentFiber != null ? + // TODO: just store the instance itself on stack + hostParentFiber.stateNode : + getCurrentRoot().stateNode.containerInfo; + const hostContainerFiber = getHostContainerOnStack(); + const hostContainerInstance = hostContainerFiber != null ? + // TODO: just store the instance itself on stack + hostContainerFiber.stateNode : + getCurrentRoot().stateNode.containerInfo; + const instance = createInstance(workInProgress.type, newProps, hostContainerInstance, workInProgress); + if (hostParentFiber) { + // TODO: this breaks reuse? + appendInitialChild(hostParentInstance, instance); + } + workInProgress.stateNode = instance; + } + pushHostFiber(workInProgress); + if (isContainerType(workInProgress.type)) { + pushHostContainer(workInProgress); + } reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; } @@ -341,8 +378,9 @@ module.exports = function( function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber { const priorityLevel = workInProgress.pendingWorkPriority; + const isHostComponent = workInProgress.tag === HostComponent; - if (workInProgress.tag === HostComponent && + if (isHostComponent && workInProgress.memoizedProps.hidden && workInProgress.pendingWorkPriority !== OffscreenPriority) { // This subtree still has work, but it should be deprioritized so we need @@ -384,10 +422,16 @@ module.exports = function( cloneChildFibers(current, workInProgress); markChildAsProgressed(current, workInProgress, priorityLevel); + // Put context on the stack because we will work on children - if (isContextProvider(workInProgress)) { + if (isHostComponent) { + if (isContainerType(workInProgress.type)) { + pushHostContainer(workInProgress); + } + } else if (isContextProvider(workInProgress)) { pushContextProvider(workInProgress, false); } + return workInProgress.child; } @@ -401,6 +445,7 @@ module.exports = function( if (!workInProgress.return) { // Don't start new work with context on the stack. resetContext(); + setCurrentRoot(workInProgress); } if (workInProgress.pendingWorkPriority === NoWork || diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 34261bca57e..7cccd7e41c5 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -24,6 +24,7 @@ var { CoroutineComponent, Portal, } = ReactTypeOfWork; +var { getCurrentRoot } = require('ReactFiberHostContext'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -303,7 +304,9 @@ module.exports = function( // Commit the work prepared earlier. const newProps = finishedWork.memoizedProps; const oldProps = current.memoizedProps; - commitUpdate(instance, oldProps, newProps, finishedWork, document.documentElement /* TODO */); + const currentRootFiber = getCurrentRoot(); + const rootInstance = currentRootFiber.stateNode.containerInfo; + commitUpdate(instance, oldProps, newProps, finishedWork, rootInstance); } detachRefIfNeeded(current, finishedWork); return; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 278857222bc..dfb71f293da 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -23,6 +23,12 @@ var { isContextProvider, popContextProvider, } = require('ReactFiberContext'); +var { + getCurrentRoot, + getHostContainerOnStack, + popHostContainer, + popHostFiber, +} = require('ReactFiberHostContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var { @@ -50,6 +56,7 @@ module.exports = function(config : HostConfig) { const finalizeInitialChildren = config.finalizeInitialChildren; const createTextInstance = config.createTextInstance; const prepareUpdate = config.prepareUpdate; + const isRootInstance = config.isRootInstance; function markUpdate(workInProgress : Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -125,35 +132,6 @@ module.exports = function(config : HostConfig) { return workInProgress.stateNode; } - function appendAllChildren(parent : I, workInProgress : Fiber) { - // We only have the top Fiber that was created but we need recurse down its - // children to find all the terminal nodes. - let node = workInProgress.child; - while (node) { - if (node.tag === HostComponent || node.tag === HostText) { - appendInitialChild(parent, node.stateNode); - } else if (node.tag === Portal) { - // If we have a portal child, 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; - continue; - } - if (node === workInProgress) { - return; - } - while (!node.sibling) { - if (!node.return || node.return === workInProgress) { - return; - } - node = node.return; - } - node = node.sibling; - } - } - function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { case FunctionalComponent: @@ -202,6 +180,12 @@ module.exports = function(config : HostConfig) { return null; } case HostComponent: + popHostFiber(); + let hostContainerFiber = getHostContainerOnStack(); + if (workInProgress === hostContainerFiber) { + popHostContainer(); + hostContainerFiber = getHostContainerOnStack(); + } let newProps = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to @@ -229,16 +213,15 @@ module.exports = function(config : HostConfig) { } } - // TODO: Move createInstance to beginWork and keep it on a context - // "stack" as the parent. Then append children as we go in beginWork - // or completeWork depending on we want to add then top->down or - // bottom->up. Top->down is faster in IE11. - // Finally, finalizeInitialChildren here in completeWork. - const instance = createInstance(workInProgress.type, newProps, document.documentElement /* TODO */, workInProgress); - appendAllChildren(instance, workInProgress); - finalizeInitialChildren(instance, workInProgress.type, newProps, document.documentElement /* TODO */); + // TODO: do we want to append children top->down or + // bottom->up? Top->down is faster in IE11. + const hostContainerInstance = hostContainerFiber != null ? + // TODO: just store the instance itself on stack + hostContainerFiber.stateNode : + getCurrentRoot().stateNode.containerInfo; + const instance = workInProgress.stateNode; + finalizeInitialChildren(instance, workInProgress.type, newProps, hostContainerInstance); - workInProgress.stateNode = instance; if (workInProgress.ref) { // If there is a ref on a host node we need to schedule a callback markUpdate(workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js new file mode 100644 index 00000000000..efdea92ff35 --- /dev/null +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -0,0 +1,78 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @providesModule ReactFiberHostContext + * @flow + */ + +'use strict'; + +import type { Fiber } from 'ReactFiber'; + +// Root we're working on. +let currentRootFiber = null; + +// All host fibers. +const hostStack : Array = []; +let hostIndex = -1; + +// Just the container host fibers (e.g. DOM uses this for SVG). +const hostContainerStack : Array = []; +let hostContainerIndex = -1; + +exports.getCurrentRoot = function() : Fiber | null { + return currentRootFiber; +}; + +exports.setCurrentRoot = function(rootFiber : Fiber) { + currentRootFiber = rootFiber; +}; + +exports.resetCurrentRoot = function() { + currentRootFiber = null; +}; + +exports.getHostFiberOnStack = function() : Fiber | null { + if (hostIndex === -1) { + return null; + } + return hostStack[hostIndex]; +}; + +exports.pushHostFiber = function(fiber : Fiber) : void { + hostIndex++; + hostStack[hostIndex] = fiber; +}; + +exports.popHostFiber = function() { + hostStack[hostIndex] = null; + hostIndex--; +}; + +exports.getHostContainerOnStack = function() : Fiber | null { + if (hostContainerIndex === -1) { + return null; + } + return hostContainerStack[hostContainerIndex]; +}; + +exports.pushHostContainer = function(fiber : Fiber) : void { + hostContainerIndex++; + hostContainerStack[hostContainerIndex] = fiber; +}; + +exports.popHostContainer = function() { + hostContainerStack[hostContainerIndex] = null; + hostContainerIndex--; +}; + +exports.resetHostFiberStacks = function() { + currentRootFiber = null; + hostIndex = -1; + hostContainerIndex = -1; +}; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 58fc5445fc4..e0c9bda4454 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -23,6 +23,7 @@ var ReactFiberCommitWork = require('ReactFiberCommitWork'); var ReactCurrentOwner = require('ReactCurrentOwner'); var { cloneFiber } = require('ReactFiber'); +var { resetHostFiberStacks } = require('ReactFiberHostContext'); var { NoWork, @@ -215,6 +216,7 @@ module.exports = function(config : HostConfig) { } resetAfterCommit(); + resetHostFiberStacks(); // Next, we'll perform all life-cycles and ref callbacks. Life-cycles // happens as a separate pass so that all effects in the entire tree have From 3bc1b07db111aaa0cd3c3387aa02b3b9a410e093 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 00:37:05 +0000 Subject: [PATCH 04/22] Keep instances instead of fibers on the stack --- src/renderers/dom/fiber/ReactDOMFiber.js | 8 +-- .../shared/fiber/ReactFiberBeginWork.js | 36 +++++----- .../shared/fiber/ReactFiberCommitWork.js | 7 +- .../shared/fiber/ReactFiberCompleteWork.js | 20 ++---- .../shared/fiber/ReactFiberHostContext.js | 67 +++++++++---------- .../shared/fiber/ReactFiberScheduler.js | 4 +- 6 files changed, 62 insertions(+), 80 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index f9f3e624c63..3f2cb4c31d9 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -98,9 +98,9 @@ var DOMRenderer = ReactFiberReconciler({ domElement : Instance, type : string, props : Props, - containerInstance : Instance | Container, + rootContainerInstance : Container, ) : void { - setInitialProperties(domElement, type, props, containerInstance); + setInitialProperties(domElement, type, props, rootContainerInstance); }, prepareUpdate( @@ -115,14 +115,14 @@ var DOMRenderer = ReactFiberReconciler({ domElement : Instance, oldProps : Props, newProps : Props, - containerInstance : Instance | Container, + rootContainerInstance : Container, internalInstanceHandle : Object, ) : void { var type = domElement.tagName.toLowerCase(); // HACK // Update the internal instance handle so that we know which props are // the current ones. precacheFiberNode(internalInstanceHandle, domElement); - updateProperties(domElement, type, oldProps, newProps, containerInstance); + updateProperties(domElement, type, oldProps, newProps, rootContainerInstance); }, createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance { diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index c1d201b672f..70ca85a257a 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -36,11 +36,9 @@ var { } = require('ReactFiberContext'); var { getHostContainerOnStack, - getHostFiberOnStack, + getHostParentOnStack, pushHostContainer, - pushHostFiber, - getCurrentRoot, - setCurrentRoot, + pushHostParent, } = require('ReactFiberHostContext'); var { IndeterminateComponent, @@ -270,26 +268,18 @@ module.exports = function( } else { if (!current && workInProgress.stateNode == null) { const newProps = workInProgress.pendingProps; - const hostParentFiber = getHostFiberOnStack(); - const hostParentInstance = hostParentFiber != null ? - // TODO: just store the instance itself on stack - hostParentFiber.stateNode : - getCurrentRoot().stateNode.containerInfo; - const hostContainerFiber = getHostContainerOnStack(); - const hostContainerInstance = hostContainerFiber != null ? - // TODO: just store the instance itself on stack - hostContainerFiber.stateNode : - getCurrentRoot().stateNode.containerInfo; - const instance = createInstance(workInProgress.type, newProps, hostContainerInstance, workInProgress); - if (hostParentFiber) { + const hostParent = getHostParentOnStack(); + const hostContainer = getHostContainerOnStack(); + const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); + if (hostParent) { // TODO: this breaks reuse? - appendInitialChild(hostParentInstance, instance); + appendInitialChild(hostParent, instance); } workInProgress.stateNode = instance; } - pushHostFiber(workInProgress); + pushHostParent(workInProgress.stateNode); if (isContainerType(workInProgress.type)) { - pushHostContainer(workInProgress); + pushHostContainer(workInProgress.stateNode); } reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; @@ -425,11 +415,14 @@ module.exports = function( // Put context on the stack because we will work on children if (isHostComponent) { + pushHostParent(workInProgress.stateNode); if (isContainerType(workInProgress.type)) { - pushHostContainer(workInProgress); + pushHostContainer(workInProgress.stateNode); } } else if (isContextProvider(workInProgress)) { pushContextProvider(workInProgress, false); + } else if (workInProgress.tag === HostContainer) { + pushHostContainer(workInProgress.stateNode.containerInfo); } return workInProgress.child; @@ -445,7 +438,6 @@ module.exports = function( if (!workInProgress.return) { // Don't start new work with context on the stack. resetContext(); - setCurrentRoot(workInProgress); } if (workInProgress.pendingWorkPriority === NoWork || @@ -490,6 +482,7 @@ module.exports = function( } else { pushTopLevelContextObject(root.context, false); } + pushHostContainer(workInProgress.stateNode.containerInfo); reconcileChildren(current, workInProgress, workInProgress.pendingProps); // A yield component is just a placeholder, we can just run through the // next one immediately. @@ -515,6 +508,7 @@ module.exports = function( // next one immediately. return null; case Portal: + // TODO: host stack. updatePortalComponent(current, workInProgress); // TODO: is this right? return workInProgress.child; diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 7cccd7e41c5..0f7d8d5f0b2 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -24,7 +24,7 @@ var { CoroutineComponent, Portal, } = ReactTypeOfWork; -var { getCurrentRoot } = require('ReactFiberHostContext'); +var { getRootHostContainerOnStack } = require('ReactFiberHostContext'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -304,9 +304,8 @@ module.exports = function( // Commit the work prepared earlier. const newProps = finishedWork.memoizedProps; const oldProps = current.memoizedProps; - const currentRootFiber = getCurrentRoot(); - const rootInstance = currentRootFiber.stateNode.containerInfo; - commitUpdate(instance, oldProps, newProps, finishedWork, rootInstance); + const rootContainerInstance = getRootHostContainerOnStack(); + commitUpdate(instance, oldProps, newProps, rootContainerInstance, finishedWork); } detachRefIfNeeded(current, finishedWork); return; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index dfb71f293da..8dd2b214652 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -24,10 +24,10 @@ var { popContextProvider, } = require('ReactFiberContext'); var { - getCurrentRoot, + getRootHostContainerOnStack, getHostContainerOnStack, popHostContainer, - popHostFiber, + popHostParent, } = require('ReactFiberHostContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); @@ -180,11 +180,10 @@ module.exports = function(config : HostConfig) { return null; } case HostComponent: - popHostFiber(); - let hostContainerFiber = getHostContainerOnStack(); - if (workInProgress === hostContainerFiber) { + popHostParent(); + const instance : I = workInProgress.stateNode; + if (instance === getHostContainerOnStack()) { popHostContainer(); - hostContainerFiber = getHostContainerOnStack(); } let newProps = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { @@ -198,7 +197,6 @@ module.exports = function(config : HostConfig) { if (!newProps) { newProps = workInProgress.memoizedProps || oldProps; } - const instance : I = workInProgress.stateNode; if (prepareUpdate(instance, oldProps, newProps)) { // This returns true if there was something to update. markUpdate(workInProgress); @@ -215,12 +213,8 @@ module.exports = function(config : HostConfig) { // TODO: do we want to append children top->down or // bottom->up? Top->down is faster in IE11. - const hostContainerInstance = hostContainerFiber != null ? - // TODO: just store the instance itself on stack - hostContainerFiber.stateNode : - getCurrentRoot().stateNode.containerInfo; - const instance = workInProgress.stateNode; - finalizeInitialChildren(instance, workInProgress.type, newProps, hostContainerInstance); + const rootContainerInstance = getRootHostContainerOnStack(); + finalizeInitialChildren(instance, workInProgress.type, newProps, rootContainerInstance); if (workInProgress.ref) { // If there is a ref on a host node we need to schedule a callback diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index efdea92ff35..ac54e6a474b 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -17,62 +17,57 @@ import type { Fiber } from 'ReactFiber'; // Root we're working on. let currentRootFiber = null; -// All host fibers. -const hostStack : Array = []; -let hostIndex = -1; +// All host instances. +const parentStack : Array = []; +let parentIndex = -1; -// Just the container host fibers (e.g. DOM uses this for SVG). -const hostContainerStack : Array = []; -let hostContainerIndex = -1; +// Just the container instances (e.g. DOM uses this for SVG). +const containerStack : Array = []; +let containerIndex = -1; -exports.getCurrentRoot = function() : Fiber | null { - return currentRootFiber; +exports.getHostParentOnStack = function() : mixed | null { + if (parentIndex === -1) { + return null; + } + return parentStack[parentIndex]; }; -exports.setCurrentRoot = function(rootFiber : Fiber) { - currentRootFiber = rootFiber; +exports.pushHostParent = function(instance : mixed) : void { + parentIndex++; + parentStack[parentIndex] = instance; }; -exports.resetCurrentRoot = function() { - currentRootFiber = null; +exports.popHostParent = function() { + parentStack[parentIndex] = null; + parentIndex--; }; -exports.getHostFiberOnStack = function() : Fiber | null { - if (hostIndex === -1) { +exports.getHostContainerOnStack = function() : mixed | null { + if (containerIndex === -1) { return null; } - return hostStack[hostIndex]; -}; - -exports.pushHostFiber = function(fiber : Fiber) : void { - hostIndex++; - hostStack[hostIndex] = fiber; -}; - -exports.popHostFiber = function() { - hostStack[hostIndex] = null; - hostIndex--; + return containerStack[containerIndex]; }; -exports.getHostContainerOnStack = function() : Fiber | null { - if (hostContainerIndex === -1) { +exports.getRootHostContainerOnStack = function() : Fiber | null { + if (containerIndex === -1) { return null; } - return hostContainerStack[hostContainerIndex]; + return containerStack[0]; }; -exports.pushHostContainer = function(fiber : Fiber) : void { - hostContainerIndex++; - hostContainerStack[hostContainerIndex] = fiber; +exports.pushHostContainer = function(instance : mixed) : void { + containerIndex++; + containerStack[containerIndex] = instance; }; exports.popHostContainer = function() { - hostContainerStack[hostContainerIndex] = null; - hostContainerIndex--; + containerStack[containerIndex] = null; + containerIndex--; }; -exports.resetHostFiberStacks = function() { +exports.resetHostStacks = function() { currentRootFiber = null; - hostIndex = -1; - hostContainerIndex = -1; + parentIndex = -1; + containerIndex = -1; }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index e0c9bda4454..e17b07074dc 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -23,7 +23,7 @@ var ReactFiberCommitWork = require('ReactFiberCommitWork'); var ReactCurrentOwner = require('ReactCurrentOwner'); var { cloneFiber } = require('ReactFiber'); -var { resetHostFiberStacks } = require('ReactFiberHostContext'); +var { resetHostStacks } = require('ReactFiberHostContext'); var { NoWork, @@ -216,7 +216,7 @@ module.exports = function(config : HostConfig) { } resetAfterCommit(); - resetHostFiberStacks(); + resetHostStacks(); // Next, we'll perform all life-cycles and ref callbacks. Life-cycles // happens as a separate pass so that all effects in the entire tree have From 21e7b048a31d9102cf7f5f450a09219403f8c5f6 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 13:27:44 +0000 Subject: [PATCH 05/22] Create text instances in begin phase --- .../shared/fiber/ReactFiberBeginWork.js | 23 +++++++++++++++++-- .../shared/fiber/ReactFiberCompleteWork.js | 14 +---------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 70ca85a257a..9c190fdfc92 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -71,6 +71,7 @@ module.exports = function( const { appendInitialChild, createInstance, + createTextInstance, isContainerType, } = config; @@ -491,8 +492,26 @@ module.exports = function( case HostComponent: return updateHostComponent(current, workInProgress); case HostText: - // Nothing to do here. This is terminal. We'll do the completion step - // immediately after. + let newText = workInProgress.pendingProps; + if (typeof newText !== 'string') { + if (workInProgress.stateNode === null) { + throw new Error('We must have new props for new mounts.'); + } else { + // This can happen when we abort work. + // TODO: can it, still? + return null; + } + } + if (!current || workInProgress.stateNode == null) { + const hostParent = getHostParentOnStack(); + const textInstance = createTextInstance(newText, workInProgress); + workInProgress.stateNode = textInstance; + if (hostParent) { + // TODO: this breaks reuse? + appendInitialChild(hostParent, textInstance); + } + } + // This is terminal. We'll do the completion step immediately after. return null; case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 8dd2b214652..7325f90cd7f 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -54,7 +54,6 @@ module.exports = function(config : HostConfig) { const createInstance = config.createInstance; const appendInitialChild = config.appendInitialChild; const finalizeInitialChildren = config.finalizeInitialChildren; - const createTextInstance = config.createTextInstance; const prepareUpdate = config.prepareUpdate; const isRootInstance = config.isRootInstance; @@ -226,7 +225,7 @@ module.exports = function(config : HostConfig) { case HostText: let newText = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { - const oldText = current.memoizedProps; + const oldText = current.memoizedProps; if (newText === null) { // If this was a bail out we need to fall back to memoized text. // This works the same way as HostComponent. @@ -240,17 +239,6 @@ module.exports = function(config : HostConfig) { if (oldText !== newText) { markUpdate(workInProgress); } - } else { - if (typeof newText !== 'string') { - if (workInProgress.stateNode === null) { - throw new Error('We must have new props for new mounts.'); - } else { - // This can happen when we abort work. - return null; - } - } - const textInstance = createTextInstance(newText, workInProgress); - workInProgress.stateNode = textInstance; } workInProgress.memoizedProps = newText; return null; From a81097924813dd364875167e4e415eb59202eec3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 14:07:58 +0000 Subject: [PATCH 06/22] Create instance before bailing on offscreen children Otherwise, the parent gets skipped next time. We could probably create it later but this seems simpler. --- .../shared/fiber/ReactFiberBeginWork.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 9c190fdfc92..7954a19a570 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -231,6 +231,17 @@ module.exports = function( // avoids allocating another HostText fiber and traversing it. nextChildren = null; } + if (!current && workInProgress.stateNode == null) { + const newProps = workInProgress.pendingProps; + const hostParent = getHostParentOnStack(); + const hostContainer = getHostContainerOnStack(); + const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); + if (hostParent) { + // TODO: this breaks reuse? + appendInitialChild(hostParent, instance); + } + workInProgress.stateNode = instance; + } if (workInProgress.pendingProps.hidden && workInProgress.pendingWorkPriority !== OffscreenPriority) { // If this host component is hidden, we can bail out on the children. @@ -267,17 +278,6 @@ module.exports = function( // Abort and don't process children yet. return null; } else { - if (!current && workInProgress.stateNode == null) { - const newProps = workInProgress.pendingProps; - const hostParent = getHostParentOnStack(); - const hostContainer = getHostContainerOnStack(); - const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); - if (hostParent) { - // TODO: this breaks reuse? - appendInitialChild(hostParent, instance); - } - workInProgress.stateNode = instance; - } pushHostParent(workInProgress.stateNode); if (isContainerType(workInProgress.type)) { pushHostContainer(workInProgress.stateNode); From 62b62412df59230bfefde870cea3297081dfebe2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 14:22:10 +0000 Subject: [PATCH 07/22] Tweak magic numbers in incremental tests I don't understand why they changed but probably related to us moving some work into begin phase? --- .../__tests__/ReactIncrementalSideEffects-test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 9e64dc43c13..850d53b795d 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -517,7 +517,7 @@ describe('ReactIncrementalSideEffects', () => { ); } ReactNoop.render(); - ReactNoop.flushDeferredPri(40 + 25); + ReactNoop.flushDeferredPri(40 + 20); expect(ReactNoop.getChildren()).toEqual([ div( span(0), @@ -525,14 +525,14 @@ describe('ReactIncrementalSideEffects', () => { ), ]); ReactNoop.render(); - ReactNoop.flushDeferredPri(35 + 25); + ReactNoop.flushDeferredPri(35 + 20); expect(ReactNoop.getChildren()).toEqual([ div( span(1), div(/*still not rendered yet*/) ), ]); - ReactNoop.flushDeferredPri(30 + 25); + ReactNoop.flushDeferredPri(30 + 20); expect(ReactNoop.getChildren()).toEqual([ div( span(1), @@ -545,7 +545,7 @@ describe('ReactIncrementalSideEffects', () => { ]); var innerSpanA = ReactNoop.getChildren()[0].children[1].children[1]; ReactNoop.render(); - ReactNoop.flushDeferredPri(30 + 25); + ReactNoop.flushDeferredPri(30 + 20); expect(ReactNoop.getChildren()).toEqual([ div( span(2), @@ -623,7 +623,7 @@ describe('ReactIncrementalSideEffects', () => { ops = []; ReactNoop.render(); - ReactNoop.flushDeferredPri(70); + ReactNoop.flushDeferredPri(65); expect(ReactNoop.getChildren()).toEqual([ div( span(1), From b8846ad93752e820dda73199d4c1d867d4b44c60 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 15:45:17 +0000 Subject: [PATCH 08/22] Only push newly created nodes on the parent stack Previously I was pushing nodes on the parent stack regardless of whether they were already in current or not. As a result, insertions during updates were duplicated, and nodes were added to existing parents before commit phase. Luckily we have a test that caught that. --- src/renderers/dom/shared/__tests__/ReactDOM-test.js | 1 + src/renderers/shared/fiber/ReactFiberBeginWork.js | 12 ++++++++---- src/renderers/shared/fiber/ReactFiberHostContext.js | 2 ++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOM-test.js b/src/renderers/dom/shared/__tests__/ReactDOM-test.js index d9bbe6c1061..2986e2c5fb3 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOM-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOM-test.js @@ -224,6 +224,7 @@ describe('ReactDOM', () => { }); expect(document.activeElement.id).toBe('one'); + ReactDOM.render(, container); // input2 gets added, which causes input to get blurred. Then // componentDidUpdate focuses input2 and that should make it down to here, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 7954a19a570..29df7fecb9b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -233,9 +233,9 @@ module.exports = function( } if (!current && workInProgress.stateNode == null) { const newProps = workInProgress.pendingProps; - const hostParent = getHostParentOnStack(); const hostContainer = getHostContainerOnStack(); const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); + const hostParent = getHostParentOnStack(); if (hostParent) { // TODO: this breaks reuse? appendInitialChild(hostParent, instance); @@ -278,7 +278,9 @@ module.exports = function( // Abort and don't process children yet. return null; } else { - pushHostParent(workInProgress.stateNode); + if (!current) { + pushHostParent(workInProgress.stateNode); + } if (isContainerType(workInProgress.type)) { pushHostContainer(workInProgress.stateNode); } @@ -416,7 +418,9 @@ module.exports = function( // Put context on the stack because we will work on children if (isHostComponent) { - pushHostParent(workInProgress.stateNode); + if (!current) { + pushHostParent(workInProgress.stateNode); + } if (isContainerType(workInProgress.type)) { pushHostContainer(workInProgress.stateNode); } @@ -503,9 +507,9 @@ module.exports = function( } } if (!current || workInProgress.stateNode == null) { - const hostParent = getHostParentOnStack(); const textInstance = createTextInstance(newText, workInProgress); workInProgress.stateNode = textInstance; + const hostParent = getHostParentOnStack(); if (hostParent) { // TODO: this breaks reuse? appendInitialChild(hostParent, textInstance); diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index ac54e6a474b..be867e93319 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -25,6 +25,8 @@ let parentIndex = -1; const containerStack : Array = []; let containerIndex = -1; +// TODO: this is all likely broken with portals. + exports.getHostParentOnStack = function() : mixed | null { if (parentIndex === -1) { return null; From 2c5caa0ae1439a7ca8bb761b3a7c8daae4891548 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 15:49:12 +0000 Subject: [PATCH 09/22] Fix lint --- src/renderers/shared/fiber/ReactFiberCompleteWork.js | 9 ++++----- src/renderers/shared/fiber/ReactFiberHostContext.js | 4 ---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 7325f90cd7f..fd62493f1bb 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -51,11 +51,10 @@ var { module.exports = function(config : HostConfig) { - const createInstance = config.createInstance; - const appendInitialChild = config.appendInitialChild; - const finalizeInitialChildren = config.finalizeInitialChildren; - const prepareUpdate = config.prepareUpdate; - const isRootInstance = config.isRootInstance; + const { + finalizeInitialChildren, + prepareUpdate, + } = config; function markUpdate(workInProgress : Fiber) { // Tag the fiber with an update effect. This turns a Placement into diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index be867e93319..c8d11af5f2d 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -14,9 +14,6 @@ import type { Fiber } from 'ReactFiber'; -// Root we're working on. -let currentRootFiber = null; - // All host instances. const parentStack : Array = []; let parentIndex = -1; @@ -69,7 +66,6 @@ exports.popHostContainer = function() { }; exports.resetHostStacks = function() { - currentRootFiber = null; parentIndex = -1; containerIndex = -1; }; From ae91e5998d4b73bab6d5c182311fa12e235ad22e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 16:22:38 +0000 Subject: [PATCH 10/22] Fix Flow I had to wrap HostContext API into a closure so that it's parameterizeable with I and C. --- .../shared/fiber/ReactFiberBeginWork.js | 21 ++-- .../shared/fiber/ReactFiberCommitWork.js | 21 +++- .../shared/fiber/ReactFiberCompleteWork.js | 23 ++-- .../shared/fiber/ReactFiberHostContext.js | 110 +++++++++++------- .../shared/fiber/ReactFiberReconciler.js | 10 +- .../shared/fiber/ReactFiberScheduler.js | 11 +- 6 files changed, 122 insertions(+), 74 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 29df7fecb9b..7e4051287fa 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -14,6 +14,7 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; +import type { HostContext } from 'ReactFiberHostContext'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; import type { PriorityLevel } from 'ReactPriorityLevel'; @@ -34,12 +35,6 @@ var { pushTopLevelContextObject, resetContext, } = require('ReactFiberContext'); -var { - getHostContainerOnStack, - getHostParentOnStack, - pushHostContainer, - pushHostParent, -} = require('ReactFiberHostContext'); var { IndeterminateComponent, FunctionalComponent, @@ -65,6 +60,7 @@ var ReactFiberClassComponent = require('ReactFiberClassComponent'); module.exports = function( config : HostConfig, + hostContext : HostContext, scheduleUpdate : (fiber: Fiber) => void ) { @@ -75,6 +71,13 @@ module.exports = function( isContainerType, } = config; + const { + getHostContainerOnStack, + getHostParentOnStack, + pushHostContainer, + pushHostParent, + } = hostContext; + const { adoptClassInstance, constructClassInstance, @@ -236,8 +239,7 @@ module.exports = function( const hostContainer = getHostContainerOnStack(); const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); const hostParent = getHostParentOnStack(); - if (hostParent) { - // TODO: this breaks reuse? + if (hostParent != null) { appendInitialChild(hostParent, instance); } workInProgress.stateNode = instance; @@ -510,8 +512,7 @@ module.exports = function( const textInstance = createTextInstance(newText, workInProgress); workInProgress.stateNode = textInstance; const hostParent = getHostParentOnStack(); - if (hostParent) { - // TODO: this breaks reuse? + if (hostParent != null) { appendInitialChild(hostParent, textInstance); } } diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 0f7d8d5f0b2..e51edbbf10e 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -13,6 +13,7 @@ 'use strict'; import type { Fiber } from 'ReactFiber'; +import type { HostContext } from 'ReactFiberHostContext'; import type { HostConfig } from 'ReactFiberReconciler'; var ReactTypeOfWork = require('ReactTypeOfWork'); @@ -24,7 +25,6 @@ var { CoroutineComponent, Portal, } = ReactTypeOfWork; -var { getRootHostContainerOnStack } = require('ReactFiberHostContext'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -35,15 +35,21 @@ var { module.exports = function( config : HostConfig, + hostContext : HostContext, trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void ) { - const commitUpdate = config.commitUpdate; - const commitTextUpdate = config.commitTextUpdate; + const { + commitUpdate, + commitTextUpdate, + appendChild, + insertBefore, + removeChild, + } = config; - const appendChild = config.appendChild; - const insertBefore = config.insertBefore; - const removeChild = config.removeChild; + const { + getRootHostContainerOnStack, + } = hostContext; function detachRef(current : Fiber) { const ref = current.ref; @@ -305,6 +311,9 @@ module.exports = function( const newProps = finishedWork.memoizedProps; const oldProps = current.memoizedProps; const rootContainerInstance = getRootHostContainerOnStack(); + if (rootContainerInstance == null) { + throw new Error('Expected to find a root instance on the host stack.'); + } commitUpdate(instance, oldProps, newProps, rootContainerInstance, finishedWork); } detachRefIfNeeded(current, finishedWork); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index fd62493f1bb..cfb7259f073 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -14,6 +14,7 @@ import type { ReactCoroutine } from 'ReactCoroutine'; import type { Fiber } from 'ReactFiber'; +import type { HostContext } from 'ReactFiberHostContext'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; import type { ReifiedYield } from 'ReactReifiedYield'; @@ -23,12 +24,6 @@ var { isContextProvider, popContextProvider, } = require('ReactFiberContext'); -var { - getRootHostContainerOnStack, - getHostContainerOnStack, - popHostContainer, - popHostParent, -} = require('ReactFiberHostContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var { @@ -49,13 +44,22 @@ var { Callback, } = ReactTypeOfSideEffect; -module.exports = function(config : HostConfig) { - +module.exports = function( + config : HostConfig, + hostContext : HostContext, +) { const { finalizeInitialChildren, prepareUpdate, } = config; + const { + getRootHostContainerOnStack, + getHostContainerOnStack, + popHostContainer, + popHostParent, + } = hostContext; + function markUpdate(workInProgress : Fiber) { // Tag the fiber with an update effect. This turns a Placement into // an UpdateAndPlacement. @@ -212,6 +216,9 @@ module.exports = function(config : HostConfig) { // TODO: do we want to append children top->down or // bottom->up? Top->down is faster in IE11. const rootContainerInstance = getRootHostContainerOnStack(); + if (rootContainerInstance == null) { + throw new Error('Expected to find a root instance on the host stack.'); + } finalizeInitialChildren(instance, workInProgress.type, newProps, rootContainerInstance); if (workInProgress.ref) { diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index c8d11af5f2d..bcd3f1ade35 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -12,60 +12,86 @@ 'use strict'; -import type { Fiber } from 'ReactFiber'; +export type HostContext = { + getHostParentOnStack() : I | null, + pushHostParent(instance : I) : void, + popHostParent() : void, -// All host instances. -const parentStack : Array = []; -let parentIndex = -1; + getHostContainerOnStack() : I | C | null, + getRootHostContainerOnStack() : C | null, + pushHostContainer(instance : I | C) : void, + popHostContainer() : void, -// Just the container instances (e.g. DOM uses this for SVG). -const containerStack : Array = []; -let containerIndex = -1; + resetHostStacks() : void, +}; + +module.exports = function() : HostContext { + // Host instances currently on the stack that have not yet been committed. + const parentStack : Array = []; + let parentIndex = -1; + + // Container instances currently on the stack (e.g. DOM uses this for SVG). + const containerStack : Array = []; + let containerIndex = -1; -// TODO: this is all likely broken with portals. + // TODO: this is all likely broken with portals. -exports.getHostParentOnStack = function() : mixed | null { - if (parentIndex === -1) { - return null; + function getHostParentOnStack() : I | null { + if (parentIndex === -1) { + return null; + } + return parentStack[parentIndex]; } - return parentStack[parentIndex]; -}; -exports.pushHostParent = function(instance : mixed) : void { - parentIndex++; - parentStack[parentIndex] = instance; -}; + function pushHostParent(instance : I) : void { + parentIndex++; + parentStack[parentIndex] = instance; + } -exports.popHostParent = function() { - parentStack[parentIndex] = null; - parentIndex--; -}; + function popHostParent() : void { + parentStack[parentIndex] = null; + parentIndex--; + } -exports.getHostContainerOnStack = function() : mixed | null { - if (containerIndex === -1) { - return null; + function getHostContainerOnStack() : I | C | null { + if (containerIndex === -1) { + return null; + } + return containerStack[containerIndex]; } - return containerStack[containerIndex]; -}; -exports.getRootHostContainerOnStack = function() : Fiber | null { - if (containerIndex === -1) { - return null; + function getRootHostContainerOnStack() : C | null { + if (containerIndex === -1) { + return null; + } + return containerStack[0]; } - return containerStack[0]; -}; -exports.pushHostContainer = function(instance : mixed) : void { - containerIndex++; - containerStack[containerIndex] = instance; -}; + function pushHostContainer(instance : I | C) : void { + containerIndex++; + containerStack[containerIndex] = instance; + } -exports.popHostContainer = function() { - containerStack[containerIndex] = null; - containerIndex--; -}; + function popHostContainer() : void { + containerStack[containerIndex] = null; + containerIndex--; + } + + function resetHostStacks() : void { + parentIndex = -1; + containerIndex = -1; + } + + return { + getHostParentOnStack, + pushHostParent, + popHostParent, + + getHostContainerOnStack, + getRootHostContainerOnStack, + pushHostContainer, + popHostContainer, -exports.resetHostStacks = function() { - parentIndex = -1; - containerIndex = -1; + resetHostStacks, + }; }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index ca2fbb7660b..a1cd7922b72 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -42,12 +42,14 @@ type OpaqueNode = Fiber; export type HostConfig = { - createInstance(type : T, props : P, internalInstanceHandle : OpaqueNode) : I, - appendInitialChild(parentInstance : I, child : I) : void, - finalizeInitialChildren(parentInstance : I, type : T, props : P) : void, + isContainerType(type : T) : boolean, + + createInstance(type : T, props : P, containerInstance : I | C, internalInstanceHandle : OpaqueNode) : I, + appendInitialChild(parentInstance : I, child : I | TI) : void, + finalizeInitialChildren(parentInstance : I, type : T, props : P, rootContainerInstance : C) : void, prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean, - commitUpdate(instance : I, oldProps : P, newProps : P, internalInstanceHandle : OpaqueNode) : void, + commitUpdate(instance : I, oldProps : P, newProps : P, rootContainerInstance : C, internalInstanceHandle : OpaqueNode) : void, createTextInstance(text : string, internalInstanceHandle : OpaqueNode) : TI, commitTextUpdate(textInstance : TI, oldText : string, newText : string) : void, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index e17b07074dc..bf7a3d03f7a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -20,10 +20,10 @@ import type { PriorityLevel } from 'ReactPriorityLevel'; var ReactFiberBeginWork = require('ReactFiberBeginWork'); var ReactFiberCompleteWork = require('ReactFiberCompleteWork'); var ReactFiberCommitWork = require('ReactFiberCommitWork'); +var ReactFiberHostContext = require('ReactFiberHostContext'); var ReactCurrentOwner = require('ReactCurrentOwner'); var { cloneFiber } = require('ReactFiber'); -var { resetHostStacks } = require('ReactFiberHostContext'); var { NoWork, @@ -66,10 +66,11 @@ type TrappedError = { }; module.exports = function(config : HostConfig) { - const { beginWork } = ReactFiberBeginWork(config, scheduleUpdate); - const { completeWork } = ReactFiberCompleteWork(config); + const hostContext = ReactFiberHostContext(); + const { beginWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); + const { completeWork } = ReactFiberCompleteWork(config, hostContext); const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } = - ReactFiberCommitWork(config, trapError); + ReactFiberCommitWork(config, hostContext, trapError); const hostScheduleAnimationCallback = config.scheduleAnimationCallback; const hostScheduleDeferredCallback = config.scheduleDeferredCallback; @@ -78,6 +79,8 @@ module.exports = function(config : HostConfig) { const prepareForCommit = config.prepareForCommit; const resetAfterCommit = config.resetAfterCommit; + const resetHostStacks = hostContext.resetHostStacks; + // The priority level to use when scheduling an update. let priorityContext : PriorityLevel = useSyncScheduling ? SynchronousPriority : From 063199e8cacdb478059d4d1d19542cc88a700b5e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 17:40:46 +0000 Subject: [PATCH 11/22] Use the same destructuring style in scheduler as everywhere else --- .../shared/fiber/ReactFiberScheduler.js | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index bf7a3d03f7a..c283edcccc1 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -67,19 +67,22 @@ type TrappedError = { module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(); + const { resetHostStacks } = hostContext; const { beginWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); - const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } = - ReactFiberCommitWork(config, hostContext, trapError); - - const hostScheduleAnimationCallback = config.scheduleAnimationCallback; - const hostScheduleDeferredCallback = config.scheduleDeferredCallback; - const useSyncScheduling = config.useSyncScheduling; - - const prepareForCommit = config.prepareForCommit; - const resetAfterCommit = config.resetAfterCommit; - - const resetHostStacks = hostContext.resetHostStacks; + const { + commitInsertion, + commitDeletion, + commitWork, + commitLifeCycles, + } = ReactFiberCommitWork(config, hostContext, trapError); + const { + scheduleAnimationCallback: hostScheduleAnimationCallback, + scheduleDeferredCallback: hostScheduleDeferredCallback, + useSyncScheduling, + prepareForCommit, + resetAfterCommit, + } = config; // The priority level to use when scheduling an update. let priorityContext : PriorityLevel = useSyncScheduling ? From 00c75b251c4fd08955bc8980cdee6704da4d0309 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 18:09:52 +0000 Subject: [PATCH 12/22] Remove branches that don't seem to run anymore I'm not 100% sure this is right but I can't get tests to fail. --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 12 +++--------- src/renderers/shared/fiber/ReactFiberCompleteWork.js | 7 ++++--- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 7e4051287fa..1efd409f6d1 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -498,17 +498,11 @@ module.exports = function( case HostComponent: return updateHostComponent(current, workInProgress); case HostText: - let newText = workInProgress.pendingProps; + const newText = workInProgress.pendingProps; if (typeof newText !== 'string') { - if (workInProgress.stateNode === null) { - throw new Error('We must have new props for new mounts.'); - } else { - // This can happen when we abort work. - // TODO: can it, still? - return null; - } + throw new Error('We must have new props for new mounts.'); } - if (!current || workInProgress.stateNode == null) { + if (!current) { const textInstance = createTextInstance(newText, workInProgress); workInProgress.stateNode = textInstance; const hostParent = getHostParentOnStack(); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index cfb7259f073..0aeda9a296d 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -184,11 +184,14 @@ module.exports = function( case HostComponent: popHostParent(); const instance : I = workInProgress.stateNode; + if (!instance) { + throw new Error('Expected host instance to be created in begin phase.'); + } if (instance === getHostContainerOnStack()) { popHostContainer(); } let newProps = workInProgress.pendingProps; - if (current && workInProgress.stateNode != null) { + if (current) { // If we have an alternate, that means this is an update and we need to // schedule a side-effect to do the updates. const oldProps = current.memoizedProps; @@ -213,8 +216,6 @@ module.exports = function( } } - // TODO: do we want to append children top->down or - // bottom->up? Top->down is faster in IE11. const rootContainerInstance = getRootHostContainerOnStack(); if (rootContainerInstance == null) { throw new Error('Expected to find a root instance on the host stack.'); From 7debd3a48118b01c5deddf4fa84edd377a9f11f3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 20:22:27 +0000 Subject: [PATCH 13/22] Be explicit about the difference between type and tag I was confused by th HACK comment so I learned how DOM and SVG work with casing and tried to write a more descriptive comment. It also seems like passing fiber.type into finalizeInitialChildren() is a potential problem because DOM code assumes tag is lowercase. So I added a similar "hack" to finalizeInitialChildren() that is identical to the one we have prepareUpdate() so if we fix them later, we fix both. --- src/renderers/dom/fiber/ReactDOMFiber.js | 19 +++++++++--- .../dom/fiber/ReactDOMFiberComponent.js | 31 ++++++++++++------- .../dom/shared/__tests__/ReactDOMSVG-test.js | 12 +++++-- src/renderers/noop/ReactNoop.js | 2 +- .../shared/fiber/ReactFiberCompleteWork.js | 2 +- .../shared/fiber/ReactFiberReconciler.js | 2 +- 6 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 3f2cb4c31d9..a46b4f22746 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -62,7 +62,6 @@ let selectionInformation : ?mixed = null; var DOMRenderer = ReactFiberReconciler({ isContainerType(type : string) { - type = type.toLowerCase(); // TODO return isNewHostContainer(type); }, @@ -96,11 +95,16 @@ var DOMRenderer = ReactFiberReconciler({ finalizeInitialChildren( domElement : Instance, - type : string, props : Props, rootContainerInstance : Container, ) : void { - setInitialProperties(domElement, type, props, rootContainerInstance); + // TODO: we normalize here because DOM renderer expects tag to be lowercase. + // We can change DOM renderer to compare special case against upper case, + // and use tagName (which is upper case for HTML DOM elements). Or we could + // let the renderer "normalize" the fiber type so we don't have to read + // the type from DOM. However we need to remember SVG is case-sensitive. + var tag = domElement.tagName.toLowerCase(); + setInitialProperties(domElement, tag, props, rootContainerInstance); }, prepareUpdate( @@ -118,11 +122,16 @@ var DOMRenderer = ReactFiberReconciler({ rootContainerInstance : Container, internalInstanceHandle : Object, ) : void { - var type = domElement.tagName.toLowerCase(); // HACK + // TODO: we normalize here because DOM renderer expects tag to be lowercase. + // We can change DOM renderer to compare special case against upper case, + // and use tagName (which is upper case for HTML DOM elements). Or we could + // let the renderer "normalize" the fiber type so we don't have to read + // the type from DOM. However we need to remember SVG is case-sensitive. + var tag = domElement.tagName.toLowerCase(); // Update the internal instance handle so that we know which props are // the current ones. precacheFiberNode(internalInstanceHandle, domElement); - updateProperties(domElement, type, oldProps, newProps, rootContainerInstance); + updateProperties(domElement, tag, oldProps, newProps, rootContainerInstance); }, createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance { diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 7144448e2b7..4baa5a7bc90 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -455,39 +455,46 @@ var ReactDOMFiberComponent = { // TODO: Does this need to check the current namespace? In case these tags // happen to be valid in some other namespace. - isNewHostContainer(tag : string) { - return tag === 'svg' || tag === 'foreignobject'; + isNewHostContainer(type : string) { + // We don't need convert user-provided "type" to a lowercase "tag" because + // both cases we're comparing against are SVG tags, which is case sensitive. + return ( + type === 'svg' || + type === 'foreignObject' + ); }, createElement( - tag : string, + type : string, props : Object, rootContainerElement : Element ) : Element { - validateDangerousTag(tag); + validateDangerousTag(type); // TODO: - // tag.toLowerCase(); Do we need to apply lower case only on non-custom elements? + // const tag = type.toLowerCase(); Do we need to apply lower case only on non-custom elements? // We create tags in the namespace of their parent container, except HTML // tags get no namespace. var namespaceURI = rootContainerElement.namespaceURI; if (namespaceURI == null || namespaceURI === DOMNamespaces.svg && - rootContainerElement.tagName === 'foreignobject') { // TODO: lowercase? + // We don't need convert to lowercase because SVG is case sensitive: + rootContainerElement.tagName === 'foreignObject') { namespaceURI = DOMNamespaces.html; } if (namespaceURI === DOMNamespaces.html) { - if (tag === 'svg') { + // We don't need convert to lowercase because SVG is case sensitive. + if (type === 'svg') { namespaceURI = DOMNamespaces.svg; - } else if (tag === 'math') { + } else if (type === 'math') { namespaceURI = DOMNamespaces.mathml; } - // TODO: Make this a new root container element. } var ownerDocument = rootContainerElement.ownerDocument; var domElement : Element; if (namespaceURI === DOMNamespaces.html) { + const tag = type.toLowerCase(); if (tag === 'script') { // Create the script via .innerHTML so its "parser-inserted" flag is // set to true and it does not execute @@ -497,17 +504,17 @@ var ReactDOMFiberComponent = { var firstChild = ((div.firstChild : any) : HTMLScriptElement); domElement = div.removeChild(firstChild); } else if (props.is) { - domElement = ownerDocument.createElement(tag, props.is); + domElement = ownerDocument.createElement(type, props.is); } else { // Separate else branch instead of using `props.is || undefined` above becuase of a Firefox bug. // See discussion in https://github.com/facebook/react/pull/6896 // and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240 - domElement = ownerDocument.createElement(tag); + domElement = ownerDocument.createElement(type); } } else { domElement = ownerDocument.createElementNS( namespaceURI, - tag + type ); } diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index a4b4fca6cd8..78dd69d20dc 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -40,9 +40,9 @@ describe('ReactDOMSVG', () => { g = el} strokeWidth="5"> image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> - +
foreignDiv = el} /> - +

p = el}> @@ -54,18 +54,26 @@ describe('ReactDOMSVG', () => {

, node ); + // SVG tagName is case sensitive. + expect(g.tagName).toBe('g'); expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); expect(g.getAttribute('stroke-width')).toBe('5'); + expect(image.tagName).toBe('image'); expect(image.namespaceURI).toBe('http://www.w3.org/2000/svg'); expect( image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(image2.tagName).toBe('image'); expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg'); expect( image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); + // DOM tagName is capitalized by browsers. + expect(p.tagName).toBe('P'); expect(p.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(div.tagName).toBe('DIV'); expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(foreignDiv.tagName).toBe('DIV'); expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); }); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 4be3af9ab8c..f6ce52d108b 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -60,7 +60,7 @@ var NoopRenderer = ReactFiberReconciler({ parentInstance.children.push(child); }, - finalizeInitialChildren(domElement : Instance, type : string, props : Props) : void { + finalizeInitialChildren(domElement : Instance, props : Props) : void { // Noop }, diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 0aeda9a296d..01320cac6ad 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -220,7 +220,7 @@ module.exports = function( if (rootContainerInstance == null) { throw new Error('Expected to find a root instance on the host stack.'); } - finalizeInitialChildren(instance, workInProgress.type, newProps, rootContainerInstance); + finalizeInitialChildren(instance, newProps, rootContainerInstance); if (workInProgress.ref) { // If there is a ref on a host node we need to schedule a callback diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index a1cd7922b72..8c41cb58dc0 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -46,7 +46,7 @@ export type HostConfig = { createInstance(type : T, props : P, containerInstance : I | C, internalInstanceHandle : OpaqueNode) : I, appendInitialChild(parentInstance : I, child : I | TI) : void, - finalizeInitialChildren(parentInstance : I, type : T, props : P, rootContainerInstance : C) : void, + finalizeInitialChildren(parentInstance : I, props : P, rootContainerInstance : C) : void, prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean, commitUpdate(instance : I, oldProps : P, newProps : P, rootContainerInstance : C, internalInstanceHandle : OpaqueNode) : void, From e6dd70670daa592af11dd44b085b82a5e6f49350 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 20:36:05 +0000 Subject: [PATCH 14/22] Save and restore host context when pushing and popping portals --- scripts/fiber/tests-passing.txt | 1 + .../dom/fiber/__tests__/ReactDOMFiber-test.js | 36 ++++++++++++++ .../dom/shared/__tests__/ReactDOMSVG-test.js | 12 ++--- src/renderers/shared/fiber/ReactFiber.js | 1 + .../shared/fiber/ReactFiberBeginWork.js | 7 ++- .../shared/fiber/ReactFiberCompleteWork.js | 2 + .../shared/fiber/ReactFiberHostContext.js | 49 ++++++++++++++++--- .../shared/fiber/ReactFiberScheduler.js | 4 +- 8 files changed, 95 insertions(+), 17 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index ee3849f47ef..7b23d79c02e 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -504,6 +504,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render one portal * should render many portals * should render nested portals +* should not apply SVG mode across portals * should pass portal context when rendering subtree elsewhere * should update portal context if it changes due to setState * should update portal context if it changes due to re-render diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js index 13f6a41499b..2fc321a9f31 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js @@ -333,6 +333,42 @@ describe('ReactDOMFiber', () => { expect(container.innerHTML).toBe(''); }); + it('should not apply SVG mode across portals', () => { + var portalContainer = document.createElement('div'); + + ReactDOM.render( + + + {ReactDOM.unstable_createPortal( +
portal
, + portalContainer + )} + + , + container + ); + + const div = portalContainer.childNodes[0]; + const image1 = container.firstChild.childNodes[0]; + const image2 = container.firstChild.childNodes[1]; + expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(div.tagName).toBe('DIV'); + expect(image1.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(image1.tagName).toBe('image'); + expect( + image1.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(image2.tagName).toBe('image'); + expect( + image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + + ReactDOM.unmountComponentAtNode(container); + expect(portalContainer.innerHTML).toBe(''); + expect(container.innerHTML).toBe(''); + }); + it('should pass portal context when rendering subtree elsewhere', () => { var portalContainer = document.createElement('div'); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index 78dd69d20dc..5b0f61f3c2b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -55,26 +55,26 @@ describe('ReactDOMSVG', () => { node ); // SVG tagName is case sensitive. - expect(g.tagName).toBe('g'); expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(g.tagName).toBe('g'); expect(g.getAttribute('stroke-width')).toBe('5'); - expect(image.tagName).toBe('image'); expect(image.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(image.tagName).toBe('image'); expect( image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); - expect(image2.tagName).toBe('image'); expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(image2.tagName).toBe('image'); expect( image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); // DOM tagName is capitalized by browsers. - expect(p.tagName).toBe('P'); expect(p.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); - expect(div.tagName).toBe('DIV'); + expect(p.tagName).toBe('P'); expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); - expect(foreignDiv.tagName).toBe('DIV'); + expect(div.tagName).toBe('DIV'); expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(foreignDiv.tagName).toBe('DIV'); }); }); diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 9c3604a5f86..b59318f5fa0 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -343,6 +343,7 @@ exports.createFiberFromPortal = function(portal : ReactPortal, priorityLevel : P fiber.stateNode = { containerInfo: portal.containerInfo, implementation: portal.implementation, + savedHostContext: null, }; return fiber; }; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 1efd409f6d1..e92314cb7e1 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -76,6 +76,7 @@ module.exports = function( getHostParentOnStack, pushHostContainer, pushHostParent, + saveHostContextToPortal, } = hostContext; const { @@ -430,7 +431,10 @@ module.exports = function( pushContextProvider(workInProgress, false); } else if (workInProgress.tag === HostContainer) { pushHostContainer(workInProgress.stateNode.containerInfo); + } else if (workInProgress.tag === Portal) { + saveHostContextToPortal(workInProgress); } + // TODO: this is annoyingly duplicating non-jump codepaths. return workInProgress.child; } @@ -526,9 +530,8 @@ module.exports = function( // next one immediately. return null; case Portal: - // TODO: host stack. + saveHostContextToPortal(workInProgress); updatePortalComponent(current, workInProgress); - // TODO: is this right? return workInProgress.child; case Fragment: updateFragment(current, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 01320cac6ad..d874bf38d02 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -58,6 +58,7 @@ module.exports = function( getHostContainerOnStack, popHostContainer, popHostParent, + restoreHostContextFromPortal, } = hostContext; function markUpdate(workInProgress : Fiber) { @@ -266,6 +267,7 @@ module.exports = function( // TODO: Only mark this as an update if we have any pending callbacks. markUpdate(workInProgress); workInProgress.memoizedProps = workInProgress.pendingProps; + restoreHostContextFromPortal(workInProgress); return null; // Error cases diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index bcd3f1ade35..f6c4f31701a 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -12,6 +12,8 @@ 'use strict'; +import type { Fiber } from 'ReactFiber'; + export type HostContext = { getHostParentOnStack() : I | null, pushHostParent(instance : I) : void, @@ -22,20 +24,20 @@ export type HostContext = { pushHostContainer(instance : I | C) : void, popHostContainer() : void, - resetHostStacks() : void, + resetHostContext() : void, + saveHostContextToPortal(portal : Fiber): void, + restoreHostContextFromPortal(portal : Fiber): void, }; module.exports = function() : HostContext { // Host instances currently on the stack that have not yet been committed. - const parentStack : Array = []; + let parentStack : Array = []; let parentIndex = -1; // Container instances currently on the stack (e.g. DOM uses this for SVG). - const containerStack : Array = []; + let containerStack : Array = []; let containerIndex = -1; - // TODO: this is all likely broken with portals. - function getHostParentOnStack() : I | null { if (parentIndex === -1) { return null; @@ -77,11 +79,42 @@ module.exports = function() : HostContext { containerIndex--; } - function resetHostStacks() : void { + function resetHostContext() : void { parentIndex = -1; containerIndex = -1; } + function saveHostContextToPortal(portal : Fiber) { + const stateNode = portal.stateNode; + // We don't throw if it already exists here because it might exist + // if something inside threw, and we started from the top. + // TODO: add tests for error boundaries inside portals when both are stable. + stateNode.savedHostContext = { + containerStack, + containerIndex, + parentStack, + parentIndex, + }; + containerStack = []; + containerIndex = -1; + parentStack = []; + parentIndex = -1; + pushHostContainer(stateNode.containerInfo); + } + + function restoreHostContextFromPortal(portal : Fiber) { + const stateNode = portal.stateNode; + const savedHostContext = stateNode.savedHostContext; + stateNode.savedHostContext = null; + if (savedHostContext == null) { + throw new Error('A portal has no host context saved on it.'); + } + containerStack = savedHostContext.containerStack; + containerIndex = savedHostContext.containerIndex; + parentStack = savedHostContext.parentStack; + parentIndex = savedHostContext.parentIndex; + } + return { getHostParentOnStack, pushHostParent, @@ -92,6 +125,8 @@ module.exports = function() : HostContext { pushHostContainer, popHostContainer, - resetHostStacks, + resetHostContext, + saveHostContextToPortal, + restoreHostContextFromPortal, }; }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index c283edcccc1..d92c715798a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -67,7 +67,7 @@ type TrappedError = { module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(); - const { resetHostStacks } = hostContext; + const { resetHostContext } = hostContext; const { beginWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); const { @@ -222,7 +222,7 @@ module.exports = function(config : HostConfig) { } resetAfterCommit(); - resetHostStacks(); + resetHostContext(); // Next, we'll perform all life-cycles and ref callbacks. Life-cycles // happens as a separate pass so that all effects in the entire tree have From 0d417207661481aeaa52ad2716f9672b2fa8bcec Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 26 Nov 2016 01:09:19 +0000 Subject: [PATCH 15/22] Revert parent context and adding children in the begin phase We can address this later separately as it is a more hot path. This doesn't affect correctness of SVG container behavior. --- .../dom/shared/__tests__/ReactDOM-test.js | 1 - .../shared/fiber/ReactFiberBeginWork.js | 37 +++++++----------- .../shared/fiber/ReactFiberCompleteWork.js | 38 ++++++++++++++++++- .../shared/fiber/ReactFiberHostContext.js | 36 ------------------ 4 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOM-test.js b/src/renderers/dom/shared/__tests__/ReactDOM-test.js index 2986e2c5fb3..d9bbe6c1061 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOM-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOM-test.js @@ -224,7 +224,6 @@ describe('ReactDOM', () => { }); expect(document.activeElement.id).toBe('one'); - ReactDOM.render(, container); // input2 gets added, which causes input to get blurred. Then // componentDidUpdate focuses input2 and that should make it down to here, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index e92314cb7e1..40d6fb4c072 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -65,7 +65,6 @@ module.exports = function( ) { const { - appendInitialChild, createInstance, createTextInstance, isContainerType, @@ -73,9 +72,7 @@ module.exports = function( const { getHostContainerOnStack, - getHostParentOnStack, pushHostContainer, - pushHostParent, saveHostContextToPortal, } = hostContext; @@ -239,10 +236,6 @@ module.exports = function( const newProps = workInProgress.pendingProps; const hostContainer = getHostContainerOnStack(); const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); - const hostParent = getHostParentOnStack(); - if (hostParent != null) { - appendInitialChild(hostParent, instance); - } workInProgress.stateNode = instance; } if (workInProgress.pendingProps.hidden && @@ -281,9 +274,6 @@ module.exports = function( // Abort and don't process children yet. return null; } else { - if (!current) { - pushHostParent(workInProgress.stateNode); - } if (isContainerType(workInProgress.type)) { pushHostContainer(workInProgress.stateNode); } @@ -421,18 +411,23 @@ module.exports = function( // Put context on the stack because we will work on children if (isHostComponent) { - if (!current) { - pushHostParent(workInProgress.stateNode); - } if (isContainerType(workInProgress.type)) { pushHostContainer(workInProgress.stateNode); } - } else if (isContextProvider(workInProgress)) { - pushContextProvider(workInProgress, false); - } else if (workInProgress.tag === HostContainer) { - pushHostContainer(workInProgress.stateNode.containerInfo); - } else if (workInProgress.tag === Portal) { - saveHostContextToPortal(workInProgress); + } else { + switch (workInProgress.tag) { + case ClassComponent: + if (isContextProvider(workInProgress)) { + pushContextProvider(workInProgress, false); + } + break; + case HostContainer: + pushHostContainer(workInProgress.stateNode.containerInfo); + break; + case Portal: + saveHostContextToPortal(workInProgress); + break; + } } // TODO: this is annoyingly duplicating non-jump codepaths. @@ -509,10 +504,6 @@ module.exports = function( if (!current) { const textInstance = createTextInstance(newText, workInProgress); workInProgress.stateNode = textInstance; - const hostParent = getHostParentOnStack(); - if (hostParent != null) { - appendInitialChild(hostParent, textInstance); - } } // This is terminal. We'll do the completion step immediately after. return null; diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index d874bf38d02..1c2011d5142 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -49,6 +49,7 @@ module.exports = function( hostContext : HostContext, ) { const { + appendInitialChild, finalizeInitialChildren, prepareUpdate, } = config; @@ -57,7 +58,6 @@ module.exports = function( getRootHostContainerOnStack, getHostContainerOnStack, popHostContainer, - popHostParent, restoreHostContextFromPortal, } = hostContext; @@ -135,6 +135,35 @@ module.exports = function( return workInProgress.stateNode; } + function appendAllChildren(parent : I, workInProgress : Fiber) { + // We only have the top Fiber that was created but we need recurse down its + // children to find all the terminal nodes. + let node = workInProgress.child; + while (node) { + if (node.tag === HostComponent || node.tag === HostText) { + appendInitialChild(parent, node.stateNode); + } else if (node.tag === Portal) { + // If we have a portal child, 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; + continue; + } + if (node === workInProgress) { + return; + } + while (!node.sibling) { + if (!node.return || node.return === workInProgress) { + return; + } + node = node.return; + } + node = node.sibling; + } + } + function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { case FunctionalComponent: @@ -183,7 +212,6 @@ module.exports = function( return null; } case HostComponent: - popHostParent(); const instance : I = workInProgress.stateNode; if (!instance) { throw new Error('Expected host instance to be created in begin phase.'); @@ -221,6 +249,12 @@ module.exports = function( if (rootContainerInstance == null) { throw new Error('Expected to find a root instance on the host stack.'); } + // TODO: Keep the instance on a context "stack" as the parent. + // Then append children as we go in beginWork or completeWork + // depending on we want to add then top->down or bottom->up. + // Top->down is faster in IE11. + // Finally, finalizeInitialChildren here in completeWork. + appendAllChildren(instance, workInProgress); finalizeInitialChildren(instance, newProps, rootContainerInstance); if (workInProgress.ref) { diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index f6c4f31701a..898026af39e 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -15,10 +15,6 @@ import type { Fiber } from 'ReactFiber'; export type HostContext = { - getHostParentOnStack() : I | null, - pushHostParent(instance : I) : void, - popHostParent() : void, - getHostContainerOnStack() : I | C | null, getRootHostContainerOnStack() : C | null, pushHostContainer(instance : I | C) : void, @@ -30,31 +26,10 @@ export type HostContext = { }; module.exports = function() : HostContext { - // Host instances currently on the stack that have not yet been committed. - let parentStack : Array = []; - let parentIndex = -1; - // Container instances currently on the stack (e.g. DOM uses this for SVG). let containerStack : Array = []; let containerIndex = -1; - function getHostParentOnStack() : I | null { - if (parentIndex === -1) { - return null; - } - return parentStack[parentIndex]; - } - - function pushHostParent(instance : I) : void { - parentIndex++; - parentStack[parentIndex] = instance; - } - - function popHostParent() : void { - parentStack[parentIndex] = null; - parentIndex--; - } - function getHostContainerOnStack() : I | C | null { if (containerIndex === -1) { return null; @@ -80,7 +55,6 @@ module.exports = function() : HostContext { } function resetHostContext() : void { - parentIndex = -1; containerIndex = -1; } @@ -92,13 +66,9 @@ module.exports = function() : HostContext { stateNode.savedHostContext = { containerStack, containerIndex, - parentStack, - parentIndex, }; containerStack = []; containerIndex = -1; - parentStack = []; - parentIndex = -1; pushHostContainer(stateNode.containerInfo); } @@ -111,15 +81,9 @@ module.exports = function() : HostContext { } containerStack = savedHostContext.containerStack; containerIndex = savedHostContext.containerIndex; - parentStack = savedHostContext.parentStack; - parentIndex = savedHostContext.parentIndex; } return { - getHostParentOnStack, - pushHostParent, - popHostParent, - getHostContainerOnStack, getRootHostContainerOnStack, pushHostContainer, From dd93f0c91886f583883e48ba944940b58e3a21a9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 28 Nov 2016 16:40:34 +0000 Subject: [PATCH 16/22] Add a test for SVG updates This tests the "jump" reuse code path in particular. --- .../dom/shared/__tests__/ReactDOMSVG-test.js | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index 5b0f61f3c2b..c221e35a84d 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -32,7 +32,7 @@ describe('ReactDOMSVG', () => { expect(markup).toContain('xlink:href="http://i.imgur.com/w7GCRPb.png"'); }); - it('creates elements with svg namespace inside svg tag', () => { + it('creates elements with SVG namespace inside SVG tag during mount', () => { var node = document.createElement('div'); var div, foreignDiv, g, image, image2, p; ReactDOM.render( @@ -77,4 +77,47 @@ describe('ReactDOMSVG', () => { expect(foreignDiv.tagName).toBe('DIV'); }); + it('creates elements with SVG namespace inside SVG tag during update', () => { + var inst, foreignDiv, g, image; + + class App extends React.Component { + state = {step: 0}; + render() { + inst = this; + const {step} = this.state; + if (step === 0) { + return null; + } + return ( + g = el} strokeWidth="5"> + image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + +
foreignDiv = el} /> + + + ); + } + } + + var node = document.createElement('div'); + ReactDOM.render( + + + , + node + ); + inst.setState({step: 1}); + + expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(g.tagName).toBe('g'); + expect(g.getAttribute('stroke-width')).toBe('5'); + expect(image.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(image.tagName).toBe('image'); + expect( + image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(foreignDiv.tagName).toBe('DIV'); + }); + }); From 94cd7db7dbec11ab7e26a4155791b3482baf2820 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 13:50:24 +0000 Subject: [PATCH 17/22] Record tests --- scripts/fiber/tests-passing.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 7b23d79c02e..8ac1c726f5b 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -655,6 +655,8 @@ src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js * creates initial namespaced markup +* creates elements with SVG namespace inside SVG tag during mount +* creates elements with SVG namespace inside SVG tag during update src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * updates a mounted text component in place From 2099a2a176437edd011898ec86410420dc433ab8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 14:08:43 +0000 Subject: [PATCH 18/22] Read ownerDocument from the root container instance This way createInstance() depends on the innermost container only for reading the namespace. --- src/renderers/dom/fiber/ReactDOMFiber.js | 3 ++- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 7 ++++--- src/renderers/shared/fiber/ReactFiberBeginWork.js | 15 +++++++++++++-- .../shared/fiber/ReactFiberReconciler.js | 2 +- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index a46b4f22746..fb2603a9dc6 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -81,10 +81,11 @@ var DOMRenderer = ReactFiberReconciler({ createInstance( type : string, props : Props, + rootContainerInstance : Container, containerInstance : Instance | Container, internalInstanceHandle : Object, ) : Instance { - const domElement : Instance = createElement(type, props, containerInstance); + const domElement : Instance = createElement(type, props, rootContainerInstance, containerInstance); precacheFiberNode(internalInstanceHandle, domElement); return domElement; }, diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 4baa5a7bc90..ee7e7603ac4 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -467,7 +467,8 @@ var ReactDOMFiberComponent = { createElement( type : string, props : Object, - rootContainerElement : Element + rootContainerElement : Element, + containerElement : Element ) : Element { validateDangerousTag(type); // TODO: @@ -475,11 +476,11 @@ var ReactDOMFiberComponent = { // We create tags in the namespace of their parent container, except HTML // tags get no namespace. - var namespaceURI = rootContainerElement.namespaceURI; + var namespaceURI = containerElement.namespaceURI; if (namespaceURI == null || namespaceURI === DOMNamespaces.svg && // We don't need convert to lowercase because SVG is case sensitive: - rootContainerElement.tagName === 'foreignObject') { + containerElement.tagName === 'foreignObject') { namespaceURI = DOMNamespaces.html; } if (namespaceURI === DOMNamespaces.html) { diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 40d6fb4c072..0c0074ef567 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -72,6 +72,7 @@ module.exports = function( const { getHostContainerOnStack, + getRootHostContainerOnStack, pushHostContainer, saveHostContextToPortal, } = hostContext; @@ -234,8 +235,18 @@ module.exports = function( } if (!current && workInProgress.stateNode == null) { const newProps = workInProgress.pendingProps; - const hostContainer = getHostContainerOnStack(); - const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress); + const containerInstance = getHostContainerOnStack(); + const rootContainerInstance = getRootHostContainerOnStack(); + if (rootContainerInstance == null) { + throw new Error('Expected to find a root instance on the host stack.'); + } + const instance = createInstance( + workInProgress.type, + newProps, + rootContainerInstance, + containerInstance, + workInProgress + ); workInProgress.stateNode = instance; } if (workInProgress.pendingProps.hidden && diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 8c41cb58dc0..9e07b9575ac 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -44,7 +44,7 @@ export type HostConfig = { isContainerType(type : T) : boolean, - createInstance(type : T, props : P, containerInstance : I | C, internalInstanceHandle : OpaqueNode) : I, + createInstance(type : T, props : P, rootContainerInstance : C, containerInstance : I | C, internalInstanceHandle : OpaqueNode) : I, appendInitialChild(parentInstance : I, child : I | TI) : void, finalizeInitialChildren(parentInstance : I, props : P, rootContainerInstance : C) : void, From c08a24a809771f421b760496e85e7ccaa83bd7c0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 16:23:41 +0000 Subject: [PATCH 19/22] Track namespaces instead of creating instances early While we might want to create instance in the begin phase, we shouldn't let DOM guide reconciler design. Instead, we are adding a new concept of "host context". In case of ReactDOMFiber, it's just the current namespace. We are keeping a stack of host context values, ignoring those that are referentially equal. The renderer receives the parent context and type, and can return a new context. --- src/renderers/dom/fiber/ReactDOMFiber.js | 10 +- .../dom/fiber/ReactDOMFiberComponent.js | 50 ++++----- src/renderers/noop/ReactNoop.js | 4 +- .../shared/fiber/ReactFiberBeginWork.js | 56 ++------- .../shared/fiber/ReactFiberCommitWork.js | 13 +-- .../shared/fiber/ReactFiberCompleteWork.js | 61 ++++++---- .../shared/fiber/ReactFiberHostContext.js | 106 ++++++++++++------ .../shared/fiber/ReactFiberReconciler.js | 8 +- .../shared/fiber/ReactFiberScheduler.js | 4 +- .../ReactIncrementalSideEffects-test.js | 10 +- 10 files changed, 164 insertions(+), 158 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index fb2603a9dc6..0ec30e71aff 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -33,7 +33,7 @@ var warning = require('warning'); var { createElement, - isNewHostContainer, + getNamespace, setInitialProperties, updateProperties, } = ReactDOMFiberComponent; @@ -61,8 +61,8 @@ let selectionInformation : ?mixed = null; var DOMRenderer = ReactFiberReconciler({ - isContainerType(type : string) { - return isNewHostContainer(type); + getHostContext(parentHostContext : string | null, type : string) { + return getNamespace(parentHostContext, type); }, prepareForCommit() : void { @@ -82,10 +82,10 @@ var DOMRenderer = ReactFiberReconciler({ type : string, props : Props, rootContainerInstance : Container, - containerInstance : Instance | Container, + hostContext : string | null, internalInstanceHandle : Object, ) : Instance { - const domElement : Instance = createElement(type, props, rootContainerInstance, containerInstance); + const domElement : Instance = createElement(type, props, rootContainerInstance, hostContext); precacheFiberNode(internalInstanceHandle, domElement); return domElement; }, diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index ee7e7603ac4..ed007490e75 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -46,6 +46,11 @@ var CHILDREN = 'children'; var STYLE = 'style'; var HTML = '__html'; +var { + svg: SVG_NAMESPACE, + math: MATH_NAMESPACE, +} = DOMNamespaces; + // Node type for document fragments (Node.DOCUMENT_FRAGMENT_NODE). var DOC_FRAGMENT_TYPE = 11; @@ -452,23 +457,28 @@ function updateDOMProperties( } var ReactDOMFiberComponent = { - - // TODO: Does this need to check the current namespace? In case these tags - // happen to be valid in some other namespace. - isNewHostContainer(type : string) { - // We don't need convert user-provided "type" to a lowercase "tag" because - // both cases we're comparing against are SVG tags, which is case sensitive. - return ( - type === 'svg' || - type === 'foreignObject' - ); + getNamespace(parentNamespace : string | null, type : string) : string | null { + if (parentNamespace == null) { + switch (type) { + case 'svg': + return SVG_NAMESPACE; + case 'math': + return MATH_NAMESPACE; + default: + return null; + } + } + if (parentNamespace === SVG_NAMESPACE && type === 'foreignObject') { + return null; + } + return parentNamespace; }, createElement( type : string, props : Object, rootContainerElement : Element, - containerElement : Element + namespaceURI : string | null ) : Element { validateDangerousTag(type); // TODO: @@ -476,25 +486,9 @@ var ReactDOMFiberComponent = { // We create tags in the namespace of their parent container, except HTML // tags get no namespace. - var namespaceURI = containerElement.namespaceURI; - if (namespaceURI == null || - namespaceURI === DOMNamespaces.svg && - // We don't need convert to lowercase because SVG is case sensitive: - containerElement.tagName === 'foreignObject') { - namespaceURI = DOMNamespaces.html; - } - if (namespaceURI === DOMNamespaces.html) { - // We don't need convert to lowercase because SVG is case sensitive. - if (type === 'svg') { - namespaceURI = DOMNamespaces.svg; - } else if (type === 'math') { - namespaceURI = DOMNamespaces.mathml; - } - } - var ownerDocument = rootContainerElement.ownerDocument; var domElement : Element; - if (namespaceURI === DOMNamespaces.html) { + if (namespaceURI == null) { const tag = type.toLowerCase(); if (tag === 'script') { // Create the script via .innerHTML so its "parser-inserted" flag is diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index f6ce52d108b..0a162ac34d9 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -40,8 +40,8 @@ var instanceCounter = 0; var NoopRenderer = ReactFiberReconciler({ - isContainerType() { - return false; + getHostContext() { + return null; }, createInstance(type : string, props : Props) : Instance { diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 0c0074ef567..81548f9fc77 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -58,22 +58,15 @@ var { var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactFiberClassComponent = require('ReactFiberClassComponent'); -module.exports = function( - config : HostConfig, - hostContext : HostContext, +module.exports = function( + config : HostConfig, + hostContext : HostContext, scheduleUpdate : (fiber: Fiber) => void ) { const { - createInstance, - createTextInstance, - isContainerType, - } = config; - - const { - getHostContainerOnStack, - getRootHostContainerOnStack, - pushHostContainer, + setRootHostContainer, + maybePushHostContext, saveHostContextToPortal, } = hostContext; @@ -233,22 +226,6 @@ module.exports = function( // avoids allocating another HostText fiber and traversing it. nextChildren = null; } - if (!current && workInProgress.stateNode == null) { - const newProps = workInProgress.pendingProps; - const containerInstance = getHostContainerOnStack(); - const rootContainerInstance = getRootHostContainerOnStack(); - if (rootContainerInstance == null) { - throw new Error('Expected to find a root instance on the host stack.'); - } - const instance = createInstance( - workInProgress.type, - newProps, - rootContainerInstance, - containerInstance, - workInProgress - ); - workInProgress.stateNode = instance; - } if (workInProgress.pendingProps.hidden && workInProgress.pendingWorkPriority !== OffscreenPriority) { // If this host component is hidden, we can bail out on the children. @@ -285,9 +262,7 @@ module.exports = function( // Abort and don't process children yet. return null; } else { - if (isContainerType(workInProgress.type)) { - pushHostContainer(workInProgress.stateNode); - } + maybePushHostContext(workInProgress); reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; } @@ -422,9 +397,7 @@ module.exports = function( // Put context on the stack because we will work on children if (isHostComponent) { - if (isContainerType(workInProgress.type)) { - pushHostContainer(workInProgress.stateNode); - } + maybePushHostContext(workInProgress); } else { switch (workInProgress.tag) { case ClassComponent: @@ -433,7 +406,7 @@ module.exports = function( } break; case HostContainer: - pushHostContainer(workInProgress.stateNode.containerInfo); + setRootHostContainer(workInProgress.stateNode.containerInfo); break; case Portal: saveHostContextToPortal(workInProgress); @@ -499,7 +472,7 @@ module.exports = function( } else { pushTopLevelContextObject(root.context, false); } - pushHostContainer(workInProgress.stateNode.containerInfo); + setRootHostContainer(workInProgress.stateNode.containerInfo); reconcileChildren(current, workInProgress, workInProgress.pendingProps); // A yield component is just a placeholder, we can just run through the // next one immediately. @@ -508,15 +481,8 @@ module.exports = function( case HostComponent: return updateHostComponent(current, workInProgress); case HostText: - const newText = workInProgress.pendingProps; - if (typeof newText !== 'string') { - throw new Error('We must have new props for new mounts.'); - } - if (!current) { - const textInstance = createTextInstance(newText, workInProgress); - workInProgress.stateNode = textInstance; - } - // This is terminal. We'll do the completion step immediately after. + // Nothing to do here. This is terminal. We'll do the completion step + // immediately after. return null; case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index e51edbbf10e..3e1d3e5cac0 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -33,9 +33,9 @@ var { Callback, } = require('ReactTypeOfSideEffect'); -module.exports = function( - config : HostConfig, - hostContext : HostContext, +module.exports = function( + config : HostConfig, + hostContext : HostContext, trapError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => void ) { @@ -48,7 +48,7 @@ module.exports = function( } = config; const { - getRootHostContainerOnStack, + getRootHostContainer, } = hostContext; function detachRef(current : Fiber) { @@ -310,10 +310,7 @@ module.exports = function( // Commit the work prepared earlier. const newProps = finishedWork.memoizedProps; const oldProps = current.memoizedProps; - const rootContainerInstance = getRootHostContainerOnStack(); - if (rootContainerInstance == null) { - throw new Error('Expected to find a root instance on the host stack.'); - } + const rootContainerInstance = getRootHostContainer(); commitUpdate(instance, oldProps, newProps, rootContainerInstance, finishedWork); } detachRefIfNeeded(current, finishedWork); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 1c2011d5142..aa70b608bf8 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -44,20 +44,22 @@ var { Callback, } = ReactTypeOfSideEffect; -module.exports = function( - config : HostConfig, - hostContext : HostContext, +module.exports = function( + config : HostConfig, + hostContext : HostContext, ) { const { + createInstance, + createTextInstance, appendInitialChild, finalizeInitialChildren, prepareUpdate, } = config; const { - getRootHostContainerOnStack, - getHostContainerOnStack, - popHostContainer, + getRootHostContainer, + maybePopHostContext, + getCurrentHostContext, restoreHostContextFromPortal, } = hostContext; @@ -212,15 +214,8 @@ module.exports = function( return null; } case HostComponent: - const instance : I = workInProgress.stateNode; - if (!instance) { - throw new Error('Expected host instance to be created in begin phase.'); - } - if (instance === getHostContainerOnStack()) { - popHostContainer(); - } let newProps = workInProgress.pendingProps; - if (current) { + if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to // schedule a side-effect to do the updates. const oldProps = current.memoizedProps; @@ -231,6 +226,7 @@ module.exports = function( if (!newProps) { newProps = workInProgress.memoizedProps || oldProps; } + const instance : I = workInProgress.stateNode; if (prepareUpdate(instance, oldProps, newProps)) { // This returns true if there was something to update. markUpdate(workInProgress); @@ -245,29 +241,35 @@ module.exports = function( } } - const rootContainerInstance = getRootHostContainerOnStack(); - if (rootContainerInstance == null) { - throw new Error('Expected to find a root instance on the host stack.'); - } - // TODO: Keep the instance on a context "stack" as the parent. - // Then append children as we go in beginWork or completeWork - // depending on we want to add then top->down or bottom->up. - // Top->down is faster in IE11. - // Finally, finalizeInitialChildren here in completeWork. + const rootContainerInstance = getRootHostContainer(); + const currentHostContext = getCurrentHostContext(); + // TODO: Move createInstance to beginWork and keep it on a context + // "stack" as the parent. Then append children as we go in beginWork + // or completeWork depending on we want to add then top->down or + // bottom->up. Top->down is faster in IE11. + const instance = createInstance( + workInProgress.type, + newProps, + rootContainerInstance, + currentHostContext, + workInProgress + ); appendAllChildren(instance, workInProgress); finalizeInitialChildren(instance, newProps, rootContainerInstance); + workInProgress.stateNode = instance; if (workInProgress.ref) { // If there is a ref on a host node we need to schedule a callback markUpdate(workInProgress); } } + maybePopHostContext(workInProgress); workInProgress.memoizedProps = newProps; return null; case HostText: let newText = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { - const oldText = current.memoizedProps; + const oldText = current.memoizedProps; if (newText === null) { // If this was a bail out we need to fall back to memoized text. // This works the same way as HostComponent. @@ -281,6 +283,17 @@ module.exports = function( if (oldText !== newText) { markUpdate(workInProgress); } + } else { + if (typeof newText !== 'string') { + if (workInProgress.stateNode === null) { + throw new Error('We must have new props for new mounts.'); + } else { + // This can happen when we abort work. + return null; + } + } + const textInstance = createTextInstance(newText, workInProgress); + workInProgress.stateNode = textInstance; } workInProgress.memoizedProps = newText; return null; diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index 898026af39e..6cb7ef3d215 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -13,49 +13,77 @@ 'use strict'; import type { Fiber } from 'ReactFiber'; +import type { HostConfig } from 'ReactFiberReconciler'; -export type HostContext = { - getHostContainerOnStack() : I | C | null, - getRootHostContainerOnStack() : C | null, - pushHostContainer(instance : I | C) : void, - popHostContainer() : void, +export type HostContext = { + getRootHostContainer() : C, + setRootHostContainer(container : C) : void, + + getCurrentHostContext() : CX | null, + maybePushHostContext(fiber : Fiber) : void, + maybePopHostContext(fiber : Fiber) : void, resetHostContext() : void, saveHostContextToPortal(portal : Fiber): void, restoreHostContextFromPortal(portal : Fiber): void, }; -module.exports = function() : HostContext { - // Container instances currently on the stack (e.g. DOM uses this for SVG). - let containerStack : Array = []; - let containerIndex = -1; +module.exports = function( + config : HostConfig +) : HostContext { + const { + getHostContext, + } = config; - function getHostContainerOnStack() : I | C | null { - if (containerIndex === -1) { - return null; + let rootHostContainer : C | null = null; + let hostContextFiberStack : Array = []; + let hostContextValueStack : Array = []; + let hostContextIndex = -1; + + function getRootHostContainer() : C { + if (rootHostContainer === null) { + throw new Error('Expected to find a root container instance.'); } - return containerStack[containerIndex]; + return rootHostContainer; + } + + function setRootHostContainer(instance : C) : void { + rootHostContainer = instance; } - function getRootHostContainerOnStack() : C | null { - if (containerIndex === -1) { + function getCurrentHostContext() : CX | null { + if (hostContextIndex === -1) { return null; } - return containerStack[0]; + return hostContextValueStack[hostContextIndex]; } - function pushHostContainer(instance : I | C) : void { - containerIndex++; - containerStack[containerIndex] = instance; + function maybePushHostContext(fiber : Fiber) : void { + const parentHostContext = getCurrentHostContext(); + const currentHostContext = getHostContext(parentHostContext, fiber.type); + if (parentHostContext === currentHostContext) { + return; + } + hostContextIndex++; + hostContextFiberStack[hostContextIndex] = fiber; + hostContextValueStack[hostContextIndex] = currentHostContext; } - function popHostContainer() : void { - containerStack[containerIndex] = null; - containerIndex--; + function maybePopHostContext(fiber : Fiber) : void { + if (hostContextIndex === -1) { + return; + } + if (fiber !== hostContextFiberStack[hostContextIndex]) { + return; + } + hostContextFiberStack[hostContextIndex] = null; + hostContextValueStack[hostContextIndex] = null; + hostContextIndex--; } - function resetHostContext() : void { - containerIndex = -1; + function resetHostContext() { + rootHostContainer = null; + hostContextIndex = -1; } function saveHostContextToPortal(portal : Fiber) { @@ -64,12 +92,16 @@ module.exports = function() : HostContext { // if something inside threw, and we started from the top. // TODO: add tests for error boundaries inside portals when both are stable. stateNode.savedHostContext = { - containerStack, - containerIndex, + rootHostContainer, + hostContextFiberStack, + hostContextValueStack, + hostContextIndex, }; - containerStack = []; - containerIndex = -1; - pushHostContainer(stateNode.containerInfo); + rootHostContainer = null; + hostContextFiberStack = []; + hostContextValueStack = []; + hostContextIndex = -1; + setRootHostContainer(stateNode.containerInfo); } function restoreHostContextFromPortal(portal : Fiber) { @@ -79,15 +111,19 @@ module.exports = function() : HostContext { if (savedHostContext == null) { throw new Error('A portal has no host context saved on it.'); } - containerStack = savedHostContext.containerStack; - containerIndex = savedHostContext.containerIndex; + rootHostContainer = savedHostContext.rootHostContainer; + hostContextFiberStack = savedHostContext.hostContextFiberStack; + hostContextValueStack = savedHostContext.hostContextValueStack; + hostContextIndex = savedHostContext.hostContextIndex; } return { - getHostContainerOnStack, - getRootHostContainerOnStack, - pushHostContainer, - popHostContainer, + getRootHostContainer, + setRootHostContainer, + + maybePushHostContext, + maybePopHostContext, + getCurrentHostContext, resetHostContext, saveHostContextToPortal, diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 9e07b9575ac..2af99bd032c 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -40,11 +40,11 @@ export type Deadline = { type OpaqueNode = Fiber; -export type HostConfig = { +export type HostConfig = { - isContainerType(type : T) : boolean, + getHostContext(parentHostContext : CX | null, type : T) : CX, - createInstance(type : T, props : P, rootContainerInstance : C, containerInstance : I | C, internalInstanceHandle : OpaqueNode) : I, + createInstance(type : T, props : P, rootContainerInstance : C, hostContext : CX | null, internalInstanceHandle : OpaqueNode) : I, appendInitialChild(parentInstance : I, child : I | TI) : void, finalizeInitialChildren(parentInstance : I, props : P, rootContainerInstance : C) : void, @@ -92,7 +92,7 @@ getContextForSubtree._injectFiber(function(fiber : Fiber) { parentContext; }); -module.exports = function(config : HostConfig) : Reconciler { +module.exports = function(config : HostConfig) : Reconciler { var { scheduleWork, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index d92c715798a..c8628f56f18 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -65,8 +65,8 @@ type TrappedError = { error: any, }; -module.exports = function(config : HostConfig) { - const hostContext = ReactFiberHostContext(); +module.exports = function(config : HostConfig) { + const hostContext = ReactFiberHostContext(config); const { resetHostContext } = hostContext; const { beginWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 850d53b795d..9e64dc43c13 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -517,7 +517,7 @@ describe('ReactIncrementalSideEffects', () => { ); } ReactNoop.render(); - ReactNoop.flushDeferredPri(40 + 20); + ReactNoop.flushDeferredPri(40 + 25); expect(ReactNoop.getChildren()).toEqual([ div( span(0), @@ -525,14 +525,14 @@ describe('ReactIncrementalSideEffects', () => { ), ]); ReactNoop.render(); - ReactNoop.flushDeferredPri(35 + 20); + ReactNoop.flushDeferredPri(35 + 25); expect(ReactNoop.getChildren()).toEqual([ div( span(1), div(/*still not rendered yet*/) ), ]); - ReactNoop.flushDeferredPri(30 + 20); + ReactNoop.flushDeferredPri(30 + 25); expect(ReactNoop.getChildren()).toEqual([ div( span(1), @@ -545,7 +545,7 @@ describe('ReactIncrementalSideEffects', () => { ]); var innerSpanA = ReactNoop.getChildren()[0].children[1].children[1]; ReactNoop.render(); - ReactNoop.flushDeferredPri(30 + 20); + ReactNoop.flushDeferredPri(30 + 25); expect(ReactNoop.getChildren()).toEqual([ div( span(2), @@ -623,7 +623,7 @@ describe('ReactIncrementalSideEffects', () => { ops = []; ReactNoop.render(); - ReactNoop.flushDeferredPri(65); + ReactNoop.flushDeferredPri(70); expect(ReactNoop.getChildren()).toEqual([ div( span(1), From 1b25dfbb1e5c0c1eaea24d403a57f3da216c3fb5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 17:20:15 +0000 Subject: [PATCH 20/22] Pop child context before reading own context and clarify API It wasn't quite clear from the API which context was being returned by the renderer. Changed the API to specifically ask for child context, and thus to pop it before getting the current context. This fixes the case with to which I intended to give SVG namespace. --- src/renderers/dom/fiber/ReactDOMFiber.js | 6 +++--- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 2 +- .../dom/shared/__tests__/ReactDOMSVG-test.js | 12 ++++++++---- src/renderers/noop/ReactNoop.js | 2 +- src/renderers/shared/fiber/ReactFiberCompleteWork.js | 6 +++--- src/renderers/shared/fiber/ReactFiberHostContext.js | 12 ++++++------ src/renderers/shared/fiber/ReactFiberReconciler.js | 2 +- 7 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 0ec30e71aff..c4ec840a3ca 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -33,7 +33,7 @@ var warning = require('warning'); var { createElement, - getNamespace, + getChildNamespace, setInitialProperties, updateProperties, } = ReactDOMFiberComponent; @@ -61,8 +61,8 @@ let selectionInformation : ?mixed = null; var DOMRenderer = ReactFiberReconciler({ - getHostContext(parentHostContext : string | null, type : string) { - return getNamespace(parentHostContext, type); + getChildHostContext(parentHostContext : string | null, type : string) { + return getChildNamespace(parentHostContext, type); }, prepareForCommit() : void { diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index ed007490e75..af61a1391ae 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -457,7 +457,7 @@ function updateDOMProperties( } var ReactDOMFiberComponent = { - getNamespace(parentNamespace : string | null, type : string) : string | null { + getChildNamespace(parentNamespace : string | null, type : string) : string | null { if (parentNamespace == null) { switch (type) { case 'svg': diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index c221e35a84d..4a6ed4818da 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -34,13 +34,13 @@ describe('ReactDOMSVG', () => { it('creates elements with SVG namespace inside SVG tag during mount', () => { var node = document.createElement('div'); - var div, foreignDiv, g, image, image2, p; + var div, foreignDiv, foreignObject, g, image, image2, p; ReactDOM.render(
g = el} strokeWidth="5"> image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> - + foreignObject = el}>
foreignDiv = el} /> @@ -63,6 +63,8 @@ describe('ReactDOMSVG', () => { expect( image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(foreignObject.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(foreignObject.tagName).toBe('foreignObject'); expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg'); expect(image2.tagName).toBe('image'); expect( @@ -78,7 +80,7 @@ describe('ReactDOMSVG', () => { }); it('creates elements with SVG namespace inside SVG tag during update', () => { - var inst, foreignDiv, g, image; + var inst, foreignObject, foreignDiv, g, image; class App extends React.Component { state = {step: 0}; @@ -91,7 +93,7 @@ describe('ReactDOMSVG', () => { return ( g = el} strokeWidth="5"> image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> - + foreignObject = el}>
foreignDiv = el} /> @@ -116,6 +118,8 @@ describe('ReactDOMSVG', () => { expect( image.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(foreignObject.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(foreignObject.tagName).toBe('foreignObject'); expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); expect(foreignDiv.tagName).toBe('DIV'); }); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 0a162ac34d9..8b394288803 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -40,7 +40,7 @@ var instanceCounter = 0; var NoopRenderer = ReactFiberReconciler({ - getHostContext() { + getChildHostContext() { return null; }, diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index aa70b608bf8..9c8b61a66a0 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -59,7 +59,7 @@ module.exports = function( const { getRootHostContainer, maybePopHostContext, - getCurrentHostContext, + getHostContext, restoreHostContextFromPortal, } = hostContext; @@ -214,6 +214,7 @@ module.exports = function( return null; } case HostComponent: + maybePopHostContext(workInProgress); let newProps = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to @@ -242,7 +243,7 @@ module.exports = function( } const rootContainerInstance = getRootHostContainer(); - const currentHostContext = getCurrentHostContext(); + const currentHostContext = getHostContext(); // TODO: Move createInstance to beginWork and keep it on a context // "stack" as the parent. Then append children as we go in beginWork // or completeWork depending on we want to add then top->down or @@ -263,7 +264,6 @@ module.exports = function( markUpdate(workInProgress); } } - maybePopHostContext(workInProgress); workInProgress.memoizedProps = newProps; return null; case HostText: diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index 6cb7ef3d215..388463d2a21 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -19,7 +19,7 @@ export type HostContext = { getRootHostContainer() : C, setRootHostContainer(container : C) : void, - getCurrentHostContext() : CX | null, + getHostContext() : CX | null, maybePushHostContext(fiber : Fiber) : void, maybePopHostContext(fiber : Fiber) : void, @@ -32,7 +32,7 @@ module.exports = function( config : HostConfig ) : HostContext { const { - getHostContext, + getChildHostContext, } = config; let rootHostContainer : C | null = null; @@ -51,7 +51,7 @@ module.exports = function( rootHostContainer = instance; } - function getCurrentHostContext() : CX | null { + function getHostContext() : CX | null { if (hostContextIndex === -1) { return null; } @@ -59,8 +59,8 @@ module.exports = function( } function maybePushHostContext(fiber : Fiber) : void { - const parentHostContext = getCurrentHostContext(); - const currentHostContext = getHostContext(parentHostContext, fiber.type); + const parentHostContext = getHostContext(); + const currentHostContext = getChildHostContext(parentHostContext, fiber.type); if (parentHostContext === currentHostContext) { return; } @@ -123,7 +123,7 @@ module.exports = function( maybePushHostContext, maybePopHostContext, - getCurrentHostContext, + getHostContext, resetHostContext, saveHostContextToPortal, diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 2af99bd032c..cd0dce848a6 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -42,7 +42,7 @@ type OpaqueNode = Fiber; export type HostConfig = { - getHostContext(parentHostContext : CX | null, type : T) : CX, + getChildHostContext(parentHostContext : CX | null, type : T) : CX, createInstance(type : T, props : P, rootContainerInstance : C, hostContext : CX | null, internalInstanceHandle : OpaqueNode) : I, appendInitialChild(parentInstance : I, child : I | TI) : void, From 5ee8dcf88617bbac02ab647e600bcf6b72dabf7e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 18:03:26 +0000 Subject: [PATCH 21/22] Give SVG namespace to itself --- src/renderers/dom/fiber/ReactDOMFiber.js | 3 ++- .../dom/fiber/ReactDOMFiberComponent.js | 27 ++++++++++++------- .../dom/shared/__tests__/ReactDOMSVG-test.js | 12 ++++++--- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index c4ec840a3ca..45dc4665ae0 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -62,7 +62,8 @@ let selectionInformation : ?mixed = null; var DOMRenderer = ReactFiberReconciler({ getChildHostContext(parentHostContext : string | null, type : string) { - return getChildNamespace(parentHostContext, type); + const parentNamespace = parentHostContext; + return getChildNamespace(parentNamespace, type); }, prepareForCommit() : void { diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index af61a1391ae..fdb38771962 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -456,21 +456,29 @@ function updateDOMProperties( } } +// Assumes there is no parent namespace. +function getIntrinsicNamespace(type : string) : string | null { + switch (type) { + case 'svg': + return SVG_NAMESPACE; + case 'math': + return MATH_NAMESPACE; + default: + return null; + } +} + var ReactDOMFiberComponent = { getChildNamespace(parentNamespace : string | null, type : string) : string | null { if (parentNamespace == null) { - switch (type) { - case 'svg': - return SVG_NAMESPACE; - case 'math': - return MATH_NAMESPACE; - default: - return null; - } + // No parent namespace: potential entry point. + return getIntrinsicNamespace(type); } if (parentNamespace === SVG_NAMESPACE && type === 'foreignObject') { + // We're leaving SVG. return null; } + // By default, pass namespace below. return parentNamespace; }, @@ -478,7 +486,7 @@ var ReactDOMFiberComponent = { type : string, props : Object, rootContainerElement : Element, - namespaceURI : string | null + parentNamespace : string | null ) : Element { validateDangerousTag(type); // TODO: @@ -488,6 +496,7 @@ var ReactDOMFiberComponent = { // tags get no namespace. var ownerDocument = rootContainerElement.ownerDocument; var domElement : Element; + var namespaceURI = parentNamespace || getIntrinsicNamespace(type); if (namespaceURI == null) { const tag = type.toLowerCase(); if (tag === 'script') { diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index 4a6ed4818da..37818dc3fa9 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -34,10 +34,10 @@ describe('ReactDOMSVG', () => { it('creates elements with SVG namespace inside SVG tag during mount', () => { var node = document.createElement('div'); - var div, foreignDiv, foreignObject, g, image, image2, p; + var div, foreignDiv, foreignObject, g, image, image2, p, svg; ReactDOM.render(
- + svg = el}> g = el} strokeWidth="5"> image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> foreignObject = el}> @@ -55,6 +55,8 @@ describe('ReactDOMSVG', () => { node ); // SVG tagName is case sensitive. + expect(svg.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(svg.tagName).toBe('svg'); expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); expect(g.tagName).toBe('g'); expect(g.getAttribute('stroke-width')).toBe('5'); @@ -80,7 +82,7 @@ describe('ReactDOMSVG', () => { }); it('creates elements with SVG namespace inside SVG tag during update', () => { - var inst, foreignObject, foreignDiv, g, image; + var inst, foreignObject, foreignDiv, g, image, svg; class App extends React.Component { state = {step: 0}; @@ -103,13 +105,15 @@ describe('ReactDOMSVG', () => { var node = document.createElement('div'); ReactDOM.render( - + svg = el}> , node ); inst.setState({step: 1}); + expect(svg.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(svg.tagName).toBe('svg'); expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); expect(g.tagName).toBe('g'); expect(g.getAttribute('stroke-width')).toBe('5'); From 5f4f36d21e08602b75b48a1b851e67a1d25e4790 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 20:37:18 +0000 Subject: [PATCH 22/22] Don't allocate unnecessarily when reconciling portals We create stacks lazily so that if portal doesn't contain s, we don't need to allocate. We also reuse the same object for portal host context state instead of creating a new one every time. --- .../dom/fiber/__tests__/ReactDOMFiber-test.js | 20 ++++++++++ .../shared/fiber/ReactFiberHostContext.js | 39 +++++++++++-------- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js index 2fc321a9f31..04c290495e4 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js @@ -364,6 +364,26 @@ describe('ReactDOMFiber', () => { image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href') ).toBe('http://i.imgur.com/w7GCRPb.png'); + ReactDOM.render( + + + {ReactDOM.unstable_createPortal( + portal, + portalContainer + )} + + , + container + ); + + const span = portalContainer.childNodes[0]; + expect(span.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(span.tagName).toBe('SPAN'); + expect(container.firstChild.childNodes[0]).toBe(image1); + const g = container.firstChild.childNodes[1]; + expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(g.tagName).toBe('g'); + ReactDOM.unmountComponentAtNode(container); expect(portalContainer.innerHTML).toBe(''); expect(container.innerHTML).toBe(''); diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index 388463d2a21..58ce8f8cdf4 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -36,8 +36,8 @@ module.exports = function( } = config; let rootHostContainer : C | null = null; - let hostContextFiberStack : Array = []; - let hostContextValueStack : Array = []; + let hostContextFiberStack : Array | null = null; + let hostContextValueStack : Array | null = null; let hostContextIndex = -1; function getRootHostContainer() : C { @@ -55,6 +55,9 @@ module.exports = function( if (hostContextIndex === -1) { return null; } + if (hostContextValueStack == null) { + throw new Error('Expected host context stacks to exist when index is more than -1.'); + } return hostContextValueStack[hostContextIndex]; } @@ -65,7 +68,9 @@ module.exports = function( return; } hostContextIndex++; + hostContextFiberStack = hostContextFiberStack || []; hostContextFiberStack[hostContextIndex] = fiber; + hostContextValueStack = hostContextValueStack || []; hostContextValueStack[hostContextIndex] = currentHostContext; } @@ -73,6 +78,9 @@ module.exports = function( if (hostContextIndex === -1) { return; } + if (hostContextFiberStack == null || hostContextValueStack == null) { + throw new Error('Expected host context stacks to exist when index is more than -1.'); + } if (fiber !== hostContextFiberStack[hostContextIndex]) { return; } @@ -87,27 +95,26 @@ module.exports = function( } function saveHostContextToPortal(portal : Fiber) { - const stateNode = portal.stateNode; - // We don't throw if it already exists here because it might exist - // if something inside threw, and we started from the top. // TODO: add tests for error boundaries inside portals when both are stable. - stateNode.savedHostContext = { - rootHostContainer, - hostContextFiberStack, - hostContextValueStack, - hostContextIndex, - }; - rootHostContainer = null; - hostContextFiberStack = []; - hostContextValueStack = []; + const stateNode = portal.stateNode; + if (!stateNode.savedHostContext) { + // We assume host context never changes between passes so store it once lazily. + stateNode.savedHostContext = { + rootHostContainer, + hostContextFiberStack, + hostContextValueStack, + hostContextIndex, + }; + } + rootHostContainer = stateNode.containerInfo; + hostContextFiberStack = null; + hostContextValueStack = null; hostContextIndex = -1; - setRootHostContainer(stateNode.containerInfo); } function restoreHostContextFromPortal(portal : Fiber) { const stateNode = portal.stateNode; const savedHostContext = stateNode.savedHostContext; - stateNode.savedHostContext = null; if (savedHostContext == null) { throw new Error('A portal has no host context saved on it.'); }