From 4f7455df58501a16f762a9a6476e21055a9f1466 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Nov 2016 18:54:24 +0000 Subject: [PATCH 01/30] 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 30760d3ae88d..ba3b1767d77c 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 ebb80e87d6109bc22a1c392d4a555dd7b63a9d48 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Nov 2016 19:16:10 +0000 Subject: [PATCH 02/30] 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 c4c851d8e33f..63052d15b89b 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 5e9d93267cf9..17bae0a58afb 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 36cf684b285b..ccbab6dcdc4e 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -339,7 +339,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 f683a08331e7..b8a4d0556299 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 ef639f6871d436077688859bfe5d361a0bc7b1ea Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Nov 2016 21:03:42 +0000 Subject: [PATCH 03/30] 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 63052d15b89b..1d703c5ea69e 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); }, shouldSetTextContent(props : Props) : boolean { diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 7c6ebd9b3c65..7144448e2b7c 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 17bae0a58afb..5e9d93267cf9 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 ba3b1767d77c..a4b4fca6cd8c 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 1e2435aa62cd..6b3441b179a7 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 812614dd34e4..b7abffc20715 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, @@ -63,6 +71,12 @@ module.exports = function( scheduleUpdate : (fiber: Fiber) => void ) { + const { + appendInitialChild, + createInstance, + isContainerType, + } = config; + const { adoptClassInstance, constructClassInstance, @@ -268,6 +282,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; } @@ -355,8 +392,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 @@ -398,10 +436,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; } @@ -415,6 +459,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 ccbab6dcdc4e..624a76932ac5 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -24,6 +24,7 @@ var { HostPortal, CoroutineComponent, } = ReactTypeOfWork; +var { getCurrentRoot } = require('ReactFiberHostContext'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -339,7 +340,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 b8a4d0556299..47d9e445302e 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 === HostPortal) { - // 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 000000000000..efdea92ff356 --- /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 4154a365255f..42bd869c5556 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, @@ -228,6 +229,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 9aaf31be9fd56277d0e7bbf49d52bbb4abdb9c3b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 00:37:05 +0000 Subject: [PATCH 04/30] 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 1d703c5ea69e..70438d746640 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); }, shouldSetTextContent(props : Props) : boolean { diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index b7abffc20715..ff3b7f15db1b 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, @@ -284,26 +282,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; @@ -439,11 +429,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; @@ -459,7 +452,6 @@ module.exports = function( if (!workInProgress.return) { // Don't start new work with context on the stack. resetContext(); - setCurrentRoot(workInProgress); } if (workInProgress.pendingWorkPriority === NoWork || @@ -504,6 +496,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. @@ -529,6 +522,7 @@ module.exports = function( // next one immediately. return null; case HostPortal: + // 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 624a76932ac5..97cbf5696917 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -24,7 +24,7 @@ var { HostPortal, CoroutineComponent, } = ReactTypeOfWork; -var { getCurrentRoot } = require('ReactFiberHostContext'); +var { getRootHostContainerOnStack } = require('ReactFiberHostContext'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -340,9 +340,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 47d9e445302e..4de6aab10b13 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 efdea92ff356..ac54e6a474bb 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 42bd869c5556..461a5d759ac9 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, @@ -229,7 +229,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 d94aee11f741b035fc928dc75deeec05825a1d94 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 13:27:44 +0000 Subject: [PATCH 05/30] 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 ff3b7f15db1b..b2a44952e3ec 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -72,6 +72,7 @@ module.exports = function( const { appendInitialChild, createInstance, + createTextInstance, isContainerType, } = config; @@ -505,8 +506,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 4de6aab10b13..6e3e189e9999 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 db2018cb5bc1f8a97388343e968f5862330d1536 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 14:07:58 +0000 Subject: [PATCH 06/30] 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 | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index b2a44952e3ec..adc10649646b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -244,7 +244,17 @@ module.exports = function( // empty, we need to schedule the text content to be reset. workInProgress.effectTag |= ContentReset; } - + 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. @@ -281,17 +291,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 2a090c16e19d459b64cd6e3420923edc8016b65c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 14:22:10 +0000 Subject: [PATCH 07/30] 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 9e64dc43c130..850d53b795d8 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 6899b38ff93f6b4dc0ba3235c749df6f9cbfc209 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 15:45:17 +0000 Subject: [PATCH 08/30] 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 d9bbe6c10615..2986e2c5fb3a 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 adc10649646b..09c6b2e5433b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -246,9 +246,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); @@ -291,7 +291,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); } @@ -429,7 +431,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); } @@ -516,9 +520,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 ac54e6a474bb..be867e93319d 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 b08ce9de25b199f30716e22a6258373b0b967f3f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 15:49:12 +0000 Subject: [PATCH 09/30] 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 6e3e189e9999..f141b526e40f 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 be867e93319d..c8d11af5f2d6 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 44189e0ed110d4aed016c9d0cc949fcfa39e5625 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 16:22:38 +0000 Subject: [PATCH 10/30] 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 | 23 ++-- .../shared/fiber/ReactFiberCompleteWork.js | 23 ++-- .../shared/fiber/ReactFiberHostContext.js | 110 +++++++++++------- .../shared/fiber/ReactFiberReconciler.js | 10 +- .../shared/fiber/ReactFiberScheduler.js | 11 +- 6 files changed, 123 insertions(+), 75 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 09c6b2e5433b..aa1e45c04a96 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, @@ -66,6 +61,7 @@ var ReactFiberClassComponent = require('ReactFiberClassComponent'); module.exports = function( config : HostConfig, + hostContext : HostContext, scheduleUpdate : (fiber: Fiber) => void ) { @@ -76,6 +72,13 @@ module.exports = function( isContainerType, } = config; + const { + getHostContainerOnStack, + getHostParentOnStack, + pushHostContainer, + pushHostParent, + } = hostContext; + const { adoptClassInstance, constructClassInstance, @@ -249,8 +252,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; @@ -523,8 +525,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 97cbf5696917..bb582db6e8d5 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 { HostPortal, CoroutineComponent, } = ReactTypeOfWork; -var { getRootHostContainerOnStack } = require('ReactFiberHostContext'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -36,16 +36,22 @@ var { module.exports = function( config : HostConfig, + hostContext : HostContext, captureError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => Fiber | null ) { - const commitUpdate = config.commitUpdate; - const resetTextContent = config.resetTextContent; - const commitTextUpdate = config.commitTextUpdate; + const { + commitUpdate, + resetTextContent, + 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; @@ -341,6 +347,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 f141b526e40f..2aec1be3b4fe 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 c8d11af5f2d6..bcd3f1ade354 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 58cfbd19bb38..0ce704a03628 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, shouldSetTextContent(props : P) : boolean, resetTextContent(instance : I) : void, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 461a5d759ac9..240a0399d491 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, @@ -62,11 +62,12 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; module.exports = function(config : HostConfig) { + const hostContext = ReactFiberHostContext(); const { beginWork, beginFailedWork } = - ReactFiberBeginWork(config, scheduleUpdate); - const { completeWork } = ReactFiberCompleteWork(config); + ReactFiberBeginWork(config, hostContext, scheduleUpdate); + const { completeWork } = ReactFiberCompleteWork(config, hostContext); const { commitPlacement, commitDeletion, commitWork, commitLifeCycles } = - ReactFiberCommitWork(config, captureError); + ReactFiberCommitWork(config, hostContext, captureError); const hostScheduleAnimationCallback = config.scheduleAnimationCallback; const hostScheduleDeferredCallback = config.scheduleDeferredCallback; @@ -75,6 +76,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 77fb9267e452d07cca16ac8008e37fe6e1d262ba Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 17:40:46 +0000 Subject: [PATCH 11/30] 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 240a0399d491..661eafb5a36a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -63,20 +63,23 @@ var timeHeuristicForUnitOfWork = 1; module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(); + const { resetHostStacks } = hostContext; const { beginWork, beginFailedWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); - const { commitPlacement, commitDeletion, commitWork, commitLifeCycles } = - ReactFiberCommitWork(config, hostContext, captureError); - - 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 { + commitPlacement, + commitDeletion, + commitWork, + commitLifeCycles, + } = ReactFiberCommitWork(config, hostContext, captureError); + const { + scheduleAnimationCallback: hostScheduleAnimationCallback, + scheduleDeferredCallback: hostScheduleDeferredCallback, + useSyncScheduling, + prepareForCommit, + resetAfterCommit, + } = config; // The priority level to use when scheduling an update. let priorityContext : PriorityLevel = useSyncScheduling ? From ce39123c4e91ee39d5c67f4f263bf65d1d7b9b42 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 18:09:52 +0000 Subject: [PATCH 12/30] 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 aa1e45c04a96..63f1c284fb09 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -511,17 +511,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 2aec1be3b4fe..adb3cf95508c 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 dfcf07b0cc82cee8b55a51dd24880b9a2e89b03b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 20:22:27 +0000 Subject: [PATCH 13/30] 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 70438d746640..eb561afa2696 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); }, shouldSetTextContent(props : Props) : boolean { diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 7144448e2b7c..4baa5a7bc907 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 a4b4fca6cd8c..78dd69d20dc9 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 6b3441b179a7..baefe1274481 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 adb3cf95508c..17c9d1e29fd0 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 0ce704a03628..bf102032c663 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 93de77f87387d3ac13a177730ddaf02054ea6fe2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Nov 2016 20:36:05 +0000 Subject: [PATCH 14/30] 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 4459f673833f..75feed1612e3 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 13f6a41499b3..2fc321a9f313 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 78dd69d20dc9..5b0f61f3c2bb 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 98e47ecaab3f..de73b081703a 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -351,6 +351,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 63f1c284fb09..788c7d9b7f7d 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -77,6 +77,7 @@ module.exports = function( getHostParentOnStack, pushHostContainer, pushHostParent, + saveHostContextToPortal, } = hostContext; const { @@ -443,7 +444,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; } @@ -539,9 +543,8 @@ module.exports = function( // next one immediately. return null; case HostPortal: - // 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 17c9d1e29fd0..98f8ab5cd328 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 bcd3f1ade354..f6c4f31701ac 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 661eafb5a36a..82aeafde865e 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -63,7 +63,7 @@ var timeHeuristicForUnitOfWork = 1; module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(); - const { resetHostStacks } = hostContext; + const { resetHostContext } = hostContext; const { beginWork, beginFailedWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); @@ -235,7 +235,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 6ab662c5b0ff5cad2aae17158b5e500ecef6b38b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 26 Nov 2016 01:09:19 +0000 Subject: [PATCH 15/30] 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 2986e2c5fb3a..d9bbe6c10615 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 788c7d9b7f7d..9fe0341437eb 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -66,7 +66,6 @@ module.exports = function( ) { const { - appendInitialChild, createInstance, createTextInstance, isContainerType, @@ -74,9 +73,7 @@ module.exports = function( const { getHostContainerOnStack, - getHostParentOnStack, pushHostContainer, - pushHostParent, saveHostContextToPortal, } = hostContext; @@ -252,10 +249,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 && @@ -294,9 +287,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); } @@ -434,18 +424,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. @@ -522,10 +517,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 98f8ab5cd328..4a35c00c376b 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 f6c4f31701ac..898026af39e7 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 840285b10528bc6392bae65bf91aea58747dec4d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 28 Nov 2016 16:40:34 +0000 Subject: [PATCH 16/30] 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 5b0f61f3c2bb..c221e35a84d4 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 c2b8888816003d74509b2ccad6b0ce82ca61125b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 13:50:24 +0000 Subject: [PATCH 17/30] 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 75feed1612e3..366580b57031 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -657,6 +657,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 8dbd419f11de131f0e28717281b586ca517c6097 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 14:08:43 +0000 Subject: [PATCH 18/30] 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 eb561afa2696..75fa9620a6e3 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 4baa5a7bc907..ee7e7603ac4f 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 9fe0341437eb..3ac3c4c6ba8a 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -73,6 +73,7 @@ module.exports = function( const { getHostContainerOnStack, + getRootHostContainerOnStack, pushHostContainer, saveHostContextToPortal, } = hostContext; @@ -247,8 +248,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 bf102032c663..9c4e46e7ef62 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 c332d6fafa100948739db002360a9fb32e10f9eb Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 16:23:41 +0000 Subject: [PATCH 19/30] 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 75fa9620a6e3..e13b2b9740a7 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 ee7e7603ac4f..ed007490e757 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 baefe1274481..d7c14d2e6c07 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 3ac3c4c6ba8a..0c42e4fb73b6 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -59,22 +59,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; @@ -246,22 +239,6 @@ module.exports = function( // empty, we need to schedule the text content to be reset. workInProgress.effectTag |= ContentReset; } - 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. @@ -298,9 +275,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; } @@ -435,9 +410,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: @@ -446,7 +419,7 @@ module.exports = function( } break; case HostContainer: - pushHostContainer(workInProgress.stateNode.containerInfo); + setRootHostContainer(workInProgress.stateNode.containerInfo); break; case Portal: saveHostContextToPortal(workInProgress); @@ -512,7 +485,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. @@ -521,15 +494,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 bb582db6e8d5..82b15ed1cc42 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -34,9 +34,9 @@ var { ContentReset, } = require('ReactTypeOfSideEffect'); -module.exports = function( - config : HostConfig, - hostContext : HostContext, +module.exports = function( + config : HostConfig, + hostContext : HostContext, captureError : (failedFiber : Fiber, error: Error, isUnmounting : boolean) => Fiber | null ) { @@ -50,7 +50,7 @@ module.exports = function( } = config; const { - getRootHostContainerOnStack, + getRootHostContainer, } = hostContext; function detachRef(current : Fiber) { @@ -346,10 +346,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 4a35c00c376b..7f6a1ce1400b 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 898026af39e7..6cb7ef3d215c 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 9c4e46e7ef62..0f9e23ed6608 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, @@ -95,7 +95,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 82aeafde865e..9b4fd792bf78 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -61,8 +61,8 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; -module.exports = function(config : HostConfig) { - const hostContext = ReactFiberHostContext(); +module.exports = function(config : HostConfig) { + const hostContext = ReactFiberHostContext(config); const { resetHostContext } = hostContext; const { beginWork, beginFailedWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 850d53b795d8..9e64dc43c130 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 836f8294ef19ae7d2d1453c80bff4d77bf56c4fb Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 17:20:15 +0000 Subject: [PATCH 20/30] 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 e13b2b9740a7..dcc87a9f5ccb 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 ed007490e757..af61a1391ae7 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 c221e35a84d4..4a6ed4818da8 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 d7c14d2e6c07..a365648f3403 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 7f6a1ce1400b..cf895e47a091 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 6cb7ef3d215c..388463d2a218 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 0f9e23ed6608..31869db29f92 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 a37d5e99516f561475f56758de3846ced131df81 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 18:03:26 +0000 Subject: [PATCH 21/30] 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 dcc87a9f5ccb..5b579517c912 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 af61a1391ae7..fdb387719629 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 4a6ed4818da8..37818dc3fa94 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 1b3178b9bddecd02a48dfc90e4a63c0b148043f4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Nov 2016 20:37:18 +0000 Subject: [PATCH 22/30] 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 2fc321a9f313..04c290495e4e 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 388463d2a218..58ce8f8cdf47 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.'); } From 84e23cac8799443a76e5fa516ce23e095f3ea1f0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 2 Dec 2016 23:35:27 +0000 Subject: [PATCH 23/30] Add more tests for edge cases --- .../dom/fiber/__tests__/ReactDOMFiber-test.js | 318 +++++++++++++++--- .../dom/shared/__tests__/ReactDOMSVG-test.js | 111 +++--- 2 files changed, 342 insertions(+), 87 deletions(-) diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js index 04c290495e4e..250fcc9b3f66 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js @@ -188,6 +188,39 @@ describe('ReactDOMFiber', () => { } if (ReactDOMFeatureFlags.useFiber) { + var svgEls, htmlEls, mathEls; + var expectSVG = {ref: el => svgEls.push(el)}; + var expectHTML = {ref: el => htmlEls.push(el)}; + var expectMath = {ref: el => mathEls.push(el)}; + + var portal = function(tree) { + return ReactDOM.unstable_createPortal( + tree, + document.createElement('div') + ); + }; + + var assertNamespacesMatch = function(tree) { + container = document.createElement('div'); + svgEls = []; + htmlEls = []; + mathEls = []; + + ReactDOM.render(tree, container); + svgEls.forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + }); + htmlEls.forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + }); + mathEls.forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/1998/Math/MathML'); + }); + + ReactDOM.unmountComponentAtNode(container); + expect(container.innerHTML).toBe(''); + }; + it('should render one portal', () => { var portalContainer = document.createElement('div'); @@ -333,60 +366,253 @@ 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 + it('should keep track of namespace across portals (simple)', () => { + assertNamespacesMatch( + + + {portal( +
)} - - , - container + + + ); + assertNamespacesMatch( + + + {portal( +
+ )} + + + ); + assertNamespacesMatch( +
+

+ {portal( + + + + )} +

+

); + }); - 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'); + it('should keep track of namespace across portals (medium)', () => { + assertNamespacesMatch( +
+ + + {portal( + + + + )} + +

+

+ ); + assertNamespacesMatch( + + + {portal( + + + +

+ + + +

+ + + + )} + + + ); + assertNamespacesMatch( +

+ {portal( + + {portal( +
+ )} + + + )} +

+

+ ); + assertNamespacesMatch( + + + {portal( +
+ )} + + + + + ); + }); - ReactDOM.render( - - - {ReactDOM.unstable_createPortal( - portal, - portalContainer + it('should keep track of namespace across portals (complex)', () => { + assertNamespacesMatch( +
+ {portal( + + + )} - - , - container +

+ + + + + + + + + +

+

+ ); + assertNamespacesMatch( +
+ + + + {portal( + + + + + + + + )} + + +

+ {portal(

)} +

+ + + + +

+ + ); + assertNamespacesMatch( +

+ + +

+ {portal( + + + + + +

+ + {portal(

)} + + + + )} +

+ + + +

+ ); + }); - 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'); + it('should unwind namespaces on uncaught errors', () => { + function BrokenRender() { + throw new Error('Hello'); + } - ReactDOM.unmountComponentAtNode(container); - expect(portalContainer.innerHTML).toBe(''); - expect(container.innerHTML).toBe(''); + expect(() => { + assertNamespacesMatch( + + + + ); + }).toThrow('Hello'); + assertNamespacesMatch( +

+ ); + }); + + it('should unwind namespaces on caught errors', () => { + function BrokenRender() { + throw new Error('Hello'); + } + + class ErrorBoundary extends React.Component { + state = {error: null}; + unstable_handleError(error) { + this.setState({error}); + } + render() { + if (this.state.error) { + return

; + } + return this.props.children; + } + } + + assertNamespacesMatch( + + + + + + + + + + + ); + assertNamespacesMatch( +

+ ); + }); + + it('should unwind namespaces on caught errors in a portal', () => { + function BrokenRender() { + throw new Error('Hello'); + } + + class ErrorBoundary extends React.Component { + state = {error: null}; + unstable_handleError(error) { + this.setState({error}); + } + render() { + if (this.state.error) { + return ; + } + return this.props.children; + } + } + + assertNamespacesMatch( + + + {portal( + + ) + + )} + + + + ); }); it('should pass portal context when rendering subtree elsewhere', () => { diff --git a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js index 37818dc3fa94..fd50a20d7ef3 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js @@ -34,55 +34,65 @@ 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, svg; + var div, div2, div3, foreignObject, foreignObject2, g, image, image2, image3, p, svg, svg2, svg3, svg4; ReactDOM.render(
svg = el}> g = el} strokeWidth="5"> - image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> - foreignObject = el}> -
foreignDiv = el} /> + svg2 = el}> + foreignObject = el}> + svg3 = el}> + svg4 = el} /> + image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + +
div = el} /> + + + image2 = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + foreignObject2 = el}> +
div2 = el} />

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

-
div = el} /> +
div3 = el} />
, node ); - // SVG tagName is case sensitive. - expect(svg.namespaceURI).toBe('http://www.w3.org/2000/svg'); - expect(svg.tagName).toBe('svg'); + [svg, svg2, svg3, svg4].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + // SVG tagName is case sensitive. + expect(el.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'); - 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(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( - image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href') - ).toBe('http://i.imgur.com/w7GCRPb.png'); - // DOM tagName is capitalized by browsers. expect(p.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + // DOM tagName is capitalized by browsers. expect(p.tagName).toBe('P'); - expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); - expect(div.tagName).toBe('DIV'); - expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); - expect(foreignDiv.tagName).toBe('DIV'); + [image, image2, image3].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(el.tagName).toBe('image'); + expect( + el.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + }); + [foreignObject, foreignObject2].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(el.tagName).toBe('foreignObject'); + }); + [div, div2, div3].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + expect(el.tagName).toBe('DIV'); + }); }); it('creates elements with SVG namespace inside SVG tag during update', () => { - var inst, foreignObject, foreignDiv, g, image, svg; + var inst, div, div2, foreignObject, foreignObject2, g, image, image2, svg, svg2, svg3, svg4; class App extends React.Component { state = {step: 0}; @@ -94,9 +104,18 @@ describe('ReactDOMSVG', () => { } return ( g = el} strokeWidth="5"> - image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> - foreignObject = el}> -
foreignDiv = el} /> + svg2 = el}> + foreignObject = el}> + svg3 = el}> + svg4 = el} /> + image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + +
div = el} /> + + + image2 = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" /> + foreignObject2 = el}> +
div2 = el} /> ); @@ -112,20 +131,30 @@ describe('ReactDOMSVG', () => { ); inst.setState({step: 1}); - expect(svg.namespaceURI).toBe('http://www.w3.org/2000/svg'); - expect(svg.tagName).toBe('svg'); + [svg, svg2, svg3, svg4].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + // SVG tagName is case sensitive. + expect(el.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'); - 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(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'); + [image, image2].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(el.tagName).toBe('image'); + expect( + el.getAttributeNS('http://www.w3.org/1999/xlink', 'href') + ).toBe('http://i.imgur.com/w7GCRPb.png'); + }); + [foreignObject, foreignObject2].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(el.tagName).toBe('foreignObject'); + }); + [div, div2].forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + // DOM tagName is capitalized by browsers. + expect(el.tagName).toBe('DIV'); + }); }); }); From aa57380ec8370ea506ebf3080ac2acf26988f177 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 2 Dec 2016 23:42:15 +0000 Subject: [PATCH 24/30] Fix up math namespace --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index fdb387719629..826dcc5b6a02 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -48,7 +48,7 @@ var HTML = '__html'; var { svg: SVG_NAMESPACE, - math: MATH_NAMESPACE, + mathml: MATH_NAMESPACE, } = DOMNamespaces; // Node type for document fragments (Node.DOCUMENT_FRAGMENT_NODE). From b31cb3df5cfc1dba3b3455a004c8fb846242168e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 3 Dec 2016 00:32:45 +0000 Subject: [PATCH 25/30] Maintain a separate container stack --- src/renderers/shared/fiber/ReactFiber.js | 1 - .../shared/fiber/ReactFiberBeginWork.js | 19 +-- .../shared/fiber/ReactFiberCompleteWork.js | 8 +- .../shared/fiber/ReactFiberHostContext.js | 154 ++++++++++-------- .../shared/fiber/ReactFiberScheduler.js | 7 +- 5 files changed, 101 insertions(+), 88 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index de73b081703a..98e47ecaab3f 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -351,7 +351,6 @@ 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 0c42e4fb73b6..6c9a8cd63f9b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -66,9 +66,9 @@ module.exports = function( ) { const { - setRootHostContainer, - maybePushHostContext, - saveHostContextToPortal, + pushHostContext, + pushHostContainer, + resetHostContainer, } = hostContext; const { @@ -275,7 +275,7 @@ module.exports = function( // Abort and don't process children yet. return null; } else { - maybePushHostContext(workInProgress); + pushHostContext(workInProgress); reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; } @@ -410,7 +410,7 @@ module.exports = function( // Put context on the stack because we will work on children if (isHostComponent) { - maybePushHostContext(workInProgress); + pushHostContext(workInProgress); } else { switch (workInProgress.tag) { case ClassComponent: @@ -419,10 +419,8 @@ module.exports = function( } break; case HostContainer: - setRootHostContainer(workInProgress.stateNode.containerInfo); - break; case Portal: - saveHostContextToPortal(workInProgress); + pushHostContainer(workInProgress.stateNode.containerInfo); break; } } @@ -441,6 +439,7 @@ module.exports = function( if (!workInProgress.return) { // Don't start new work with context on the stack. resetContext(); + resetHostContainer(); } if (workInProgress.pendingWorkPriority === NoWork || @@ -485,7 +484,7 @@ module.exports = function( } else { pushTopLevelContextObject(root.context, false); } - setRootHostContainer(workInProgress.stateNode.containerInfo); + 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. @@ -511,7 +510,7 @@ module.exports = function( // next one immediately. return null; case HostPortal: - saveHostContextToPortal(workInProgress); + pushHostContainer(workInProgress.stateNode.containerInfo); updatePortalComponent(current, workInProgress); return workInProgress.child; case Fragment: diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index cf895e47a091..397c8bc09b64 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -58,9 +58,9 @@ module.exports = function( const { getRootHostContainer, - maybePopHostContext, + popHostContext, getHostContext, - restoreHostContextFromPortal, + popHostContainer, } = hostContext; function markUpdate(workInProgress : Fiber) { @@ -214,7 +214,7 @@ module.exports = function( return null; } case HostComponent: - maybePopHostContext(workInProgress); + popHostContext(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 @@ -314,7 +314,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); + popHostContainer(); return null; // Error cases diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index 58ce8f8cdf47..cc06e9e9a1ad 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -17,15 +17,14 @@ import type { HostConfig } from 'ReactFiberReconciler'; export type HostContext = { getRootHostContainer() : C, - setRootHostContainer(container : C) : void, - getHostContext() : CX | null, - maybePushHostContext(fiber : Fiber) : void, - maybePopHostContext(fiber : Fiber) : void, - resetHostContext() : void, - saveHostContextToPortal(portal : Fiber): void, - restoreHostContextFromPortal(portal : Fiber): void, + pushHostContext(fiber : Fiber) : void, + popHostContext(fiber : Fiber) : void, + + pushHostContainer(container : C) : void, + popHostContainer() : void, + resetHostContainer() : void, }; module.exports = function( @@ -35,105 +34,118 @@ module.exports = function( getChildHostContext, } = config; - let rootHostContainer : C | null = null; - let hostContextFiberStack : Array | null = null; - let hostContextValueStack : Array | null = null; - let hostContextIndex = -1; + type ContainerState = { + rootInstance : C, + contextFibers : Array | null, + contextValues : Array | null, + contextIndex: number + }; + + function createContainerState(rootInstance) { + return { + rootInstance, + contextFibers: null, + contextValues: null, + contextDepth: -1, + }; + } + + // State of the current tree. + let containerState : ContainerState = createContainerState(null); + + // If we meet any portals, we'll go deeper. + let containerStack : Array = [containerState]; + let containerDepth : number = 0; function getRootHostContainer() : C { - if (rootHostContainer === null) { + const {rootInstance} = containerState; + if (rootInstance == null) { throw new Error('Expected to find a root container instance.'); } - return rootHostContainer; + return rootInstance; + } + + function pushHostContainer(portalHostContainer) { + containerDepth++; + if (containerDepth === containerStack.length) { + containerState = createContainerState(portalHostContainer); + containerStack[containerDepth] = containerState; + } else { + containerState = containerStack[containerDepth]; + } } - function setRootHostContainer(instance : C) : void { - rootHostContainer = instance; + function popHostContainer() { + containerDepth--; + containerState = containerStack[containerDepth]; } function getHostContext() : CX | null { - if (hostContextIndex === -1) { + const {contextDepth, contextValues} = containerState; + if (contextDepth === -1) { return null; } - if (hostContextValueStack == null) { + if (contextValues == null) { throw new Error('Expected host context stacks to exist when index is more than -1.'); } - return hostContextValueStack[hostContextIndex]; + return contextValues[contextDepth]; } - function maybePushHostContext(fiber : Fiber) : void { + function pushHostContext(fiber : Fiber) : void { const parentHostContext = getHostContext(); const currentHostContext = getChildHostContext(parentHostContext, fiber.type); if (parentHostContext === currentHostContext) { return; } - hostContextIndex++; - hostContextFiberStack = hostContextFiberStack || []; - hostContextFiberStack[hostContextIndex] = fiber; - hostContextValueStack = hostContextValueStack || []; - hostContextValueStack[hostContextIndex] = currentHostContext; + let {contextDepth, contextFibers, contextValues} = containerState; + if (contextFibers == null) { + contextFibers = []; + containerState.contextFibers = contextFibers; + } + if (contextValues == null) { + contextValues = []; + containerState.contextValues = contextValues; + } + contextDepth++; + containerState.contextDepth = contextDepth; + contextFibers[contextDepth] = fiber; + contextValues[contextDepth] = currentHostContext; } - function maybePopHostContext(fiber : Fiber) : void { - if (hostContextIndex === -1) { + function popHostContext(fiber : Fiber) : void { + let {contextDepth} = containerState; + if (contextDepth === -1) { return; } - if (hostContextFiberStack == null || hostContextValueStack == null) { + const {contextFibers, contextValues} = containerState; + if (contextFibers == null || contextValues == null) { throw new Error('Expected host context stacks to exist when index is more than -1.'); } - if (fiber !== hostContextFiberStack[hostContextIndex]) { + if (fiber !== contextFibers[contextDepth]) { return; } - hostContextFiberStack[hostContextIndex] = null; - hostContextValueStack[hostContextIndex] = null; - hostContextIndex--; - } - - function resetHostContext() { - rootHostContainer = null; - hostContextIndex = -1; - } - - function saveHostContextToPortal(portal : Fiber) { - // TODO: add tests for error boundaries inside portals when both are stable. - 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; + contextFibers[contextDepth] = null; + contextValues[contextDepth] = null; + contextDepth--; + containerState.contextDepth = contextDepth; } - function restoreHostContextFromPortal(portal : Fiber) { - const stateNode = portal.stateNode; - const savedHostContext = stateNode.savedHostContext; - if (savedHostContext == null) { - throw new Error('A portal has no host context saved on it.'); - } - rootHostContainer = savedHostContext.rootHostContainer; - hostContextFiberStack = savedHostContext.hostContextFiberStack; - hostContextValueStack = savedHostContext.hostContextValueStack; - hostContextIndex = savedHostContext.hostContextIndex; + function resetHostContainer() { + containerDepth = 0; + containerState = containerStack[0]; + containerState.rootInstance = null; + containerState.contextDepth = -1; } return { getRootHostContainer, - setRootHostContainer, - - maybePushHostContext, - maybePopHostContext, getHostContext, - resetHostContext, - saveHostContextToPortal, - restoreHostContextFromPortal, + pushHostContext, + popHostContext, + + pushHostContainer, + popHostContainer, + resetHostContainer, }; }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 9b4fd792bf78..1a520c790753 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -63,7 +63,7 @@ var timeHeuristicForUnitOfWork = 1; module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(config); - const { resetHostContext } = hostContext; + const { popHostContainer } = hostContext; const { beginWork, beginFailedWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); @@ -235,7 +235,10 @@ module.exports = function(config : HostConfig Date: Sat, 3 Dec 2016 00:44:28 +0000 Subject: [PATCH 26/30] Fix rebase mistakes --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 4 ++-- src/renderers/shared/fiber/ReactFiberCompleteWork.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 6c9a8cd63f9b..c46c63cb74f6 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -418,8 +418,8 @@ module.exports = function( pushContextProvider(workInProgress, false); } break; - case HostContainer: - case Portal: + case HostRoot: + case HostPortal: pushHostContainer(workInProgress.stateNode.containerInfo); break; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 397c8bc09b64..a4a0143069c7 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -144,7 +144,7 @@ module.exports = function( while (node) { if (node.tag === HostComponent || node.tag === HostText) { appendInitialChild(parent, node.stateNode); - } else if (node.tag === Portal) { + } else if (node.tag === HostPortal) { // 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. From 42408332d49d66c6ae4c6af8caf9b7f351ab5032 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 3 Dec 2016 01:33:24 +0000 Subject: [PATCH 27/30] Unwind context on errors --- .../dom/fiber/__tests__/ReactDOMFiber-test.js | 18 +++++++-- .../shared/fiber/ReactFiberBeginWork.js | 3 ++ .../shared/fiber/ReactFiberHostContext.js | 38 +++++++++---------- .../shared/fiber/ReactFiberScheduler.js | 29 ++++++++++++-- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js index 250fcc9b3f66..8d9347740041 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js @@ -605,12 +605,22 @@ describe('ReactDOMFiber', () => { {portal( - - ) - +
+ + ) + +
)} - + { + /* + * TODO: enable. Currently this leads to stack overflow + * but it might be a bug in error boundaries rather than SVG or portals. + portal( +
+ ) + */ + } ); }); diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index c46c63cb74f6..27270e17194e 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -430,6 +430,9 @@ module.exports = function( } function bailoutOnLowPriority(current, workInProgress) { + if (workInProgress.tag === HostPortal) { + pushHostContainer(workInProgress.stateNode.containerInfo); + } // TODO: What if this is currently in progress? // How can that happen? How is this not being cloned? return null; diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index cc06e9e9a1ad..1d52d48a836b 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -38,7 +38,7 @@ module.exports = function( rootInstance : C, contextFibers : Array | null, contextValues : Array | null, - contextIndex: number + contextDepth: number }; function createContainerState(rootInstance) { @@ -50,43 +50,44 @@ module.exports = function( }; } - // State of the current tree. - let containerState : ContainerState = createContainerState(null); - // If we meet any portals, we'll go deeper. - let containerStack : Array = [containerState]; - let containerDepth : number = 0; + let containerStack : Array = []; + let containerDepth : number = -1; function getRootHostContainer() : C { - const {rootInstance} = containerState; - if (rootInstance == null) { - throw new Error('Expected to find a root container instance.'); + if (containerDepth === -1) { + throw new Error('Expected to find a root container.'); } + const containerState = containerStack[containerDepth]; + const {rootInstance} = containerState; return rootInstance; } function pushHostContainer(portalHostContainer) { containerDepth++; if (containerDepth === containerStack.length) { - containerState = createContainerState(portalHostContainer); - containerStack[containerDepth] = containerState; - } else { - containerState = containerStack[containerDepth]; + containerStack[containerDepth] = createContainerState(portalHostContainer); } } function popHostContainer() { + if (containerDepth === -1) { + throw new Error('Already reached the root.'); + } containerDepth--; - containerState = containerStack[containerDepth]; } function getHostContext() : CX | null { + if (containerDepth == null) { + throw new Error('Expected to find a root container.'); + } + const containerState = containerStack[containerDepth]; const {contextDepth, contextValues} = containerState; if (contextDepth === -1) { return null; } if (contextValues == null) { - throw new Error('Expected host context stacks to exist when index is more than -1.'); + throw new Error('Expected context values to exist.'); } return contextValues[contextDepth]; } @@ -97,6 +98,7 @@ module.exports = function( if (parentHostContext === currentHostContext) { return; } + const containerState = containerStack[containerDepth]; let {contextDepth, contextFibers, contextValues} = containerState; if (contextFibers == null) { contextFibers = []; @@ -113,6 +115,7 @@ module.exports = function( } function popHostContext(fiber : Fiber) : void { + const containerState = containerStack[containerDepth]; let {contextDepth} = containerState; if (contextDepth === -1) { return; @@ -131,10 +134,7 @@ module.exports = function( } function resetHostContainer() { - containerDepth = 0; - containerState = containerStack[0]; - containerState.rootInstance = null; - containerState.contextDepth = -1; + containerDepth = -1; } return { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 1a520c790753..4bcf440566a7 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -48,6 +48,8 @@ var { var { HostRoot, + HostComponent, + HostPortal, ClassComponent, } = require('ReactTypeOfWork'); @@ -63,7 +65,7 @@ var timeHeuristicForUnitOfWork = 1; module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(config); - const { popHostContainer } = hostContext; + const { popHostContainer, popHostContext, resetHostContainer } = hostContext; const { beginWork, beginFailedWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config, hostContext); @@ -236,9 +238,9 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig Date: Thu, 8 Dec 2016 17:15:34 +0000 Subject: [PATCH 28/30] Reset the container state when reusing the object --- src/renderers/shared/fiber/ReactFiberHostContext.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index 1d52d48a836b..5a8e580a1215 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -67,6 +67,12 @@ module.exports = function( containerDepth++; if (containerDepth === containerStack.length) { containerStack[containerDepth] = createContainerState(portalHostContainer); + } else { + const containerState = containerStack[containerDepth]; + containerState.rootInstance = portalHostContainer; + containerState.contextFibers = null; + containerState.contextValues = null; + containerState.contextDepth = -1; } } From f64f7869f5d2e754064a12ac60c805bcac3b831f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 8 Dec 2016 18:24:07 +0000 Subject: [PATCH 29/30] Add getChildHostContext() to ReactART --- src/renderers/art/ReactARTFiber.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/renderers/art/ReactARTFiber.js b/src/renderers/art/ReactARTFiber.js index 467f97646348..8655b8b8e255 100644 --- a/src/renderers/art/ReactARTFiber.js +++ b/src/renderers/art/ReactARTFiber.js @@ -485,6 +485,10 @@ const ARTRenderer = ReactFiberReconciler({ // Noop }, + getChildHostContext() { + return null; + }, + scheduleAnimationCallback: window.requestAnimationFrame, scheduleDeferredCallback: window.requestIdleCallback, From 79074b10d509b57bd5a105232f3f13d8b67fc39c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 8 Dec 2016 18:24:09 +0000 Subject: [PATCH 30/30] Record tests --- scripts/fiber/tests-passing.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 6d972d0bdb9e..cb71a3431df8 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -514,7 +514,12 @@ 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 keep track of namespace across portals (simple) +* should keep track of namespace across portals (medium) +* should keep track of namespace across portals (complex) +* should unwind namespaces on uncaught errors +* should unwind namespaces on caught errors +* should unwind namespaces on caught errors in a portal * 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