Save and restore host context when pushing and popping portals

Author: gaearon

scripts/fiber/tests-passing.txt

@@ -492,6 +492,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

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(
+        <svg>
+          <image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
+          {ReactDOM.unstable_createPortal(
+            <div>portal</div>,
+            portalContainer
+          )}
+          <image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
+        </svg>,
+        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');
 

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');
   });
 
 });

src/renderers/shared/fiber/ReactFiber.js

@@ -343,6 +343,7 @@ exports.createFiberFromPortal = function(portal : ReactPortal, priorityLevel : P
   fiber.stateNode = {
     containerInfo: portal.containerInfo,
     implementation: portal.implementation,
+    savedHostContext: null,
   };
   return fiber;
 };

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -76,6 +76,7 @@ module.exports = function<T, P, I, TI, C>(
     getHostParentOnStack,
     pushHostContainer,
     pushHostParent,
+    saveHostContextToPortal,
   } = hostContext;
 
   const {
@@ -430,7 +431,10 @@ module.exports = function<T, P, I, TI, C>(
       pushContextProvider(workInProgress, false);
     } else if (workInProgress.tag === HostContainer) {
       pushHostContainer(workInProgress.stateNode.containerInfo);
+    } else if (workInProgress.tag === Portal) {
+      saveHostContextToPortal(workInProgress);
     }
+    // TODO: this is annoyingly duplicating non-jump codepaths.
 
     return workInProgress.child;
   }
@@ -526,9 +530,8 @@ module.exports = function<T, P, I, TI, C>(
         // next one immediately.
         return null;
       case Portal:
-        // TODO: host stack.
+        saveHostContextToPortal(workInProgress);
         updatePortalComponent(current, workInProgress);
-        // TODO: is this right?
         return workInProgress.child;
       case Fragment:
         updateFragment(current, workInProgress);

src/renderers/shared/fiber/ReactFiberCompleteWork.js

@@ -58,6 +58,7 @@ module.exports = function<T, P, I, TI, C>(
     getHostContainerOnStack,
     popHostContainer,
     popHostParent,
+    restoreHostContextFromPortal,
   } = hostContext;
 
   function markUpdate(workInProgress : Fiber) {
@@ -255,6 +256,7 @@ module.exports = function<T, P, I, TI, C>(
         // 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

src/renderers/shared/fiber/ReactFiberHostContext.js

@@ -12,6 +12,8 @@
 
 'use strict';
 
+import type { Fiber } from 'ReactFiber';
+
 export type HostContext<I, C> = {
   getHostParentOnStack() : I | null,
   pushHostParent(instance : I) : void,
@@ -22,20 +24,20 @@ export type HostContext<I, C> = {
   pushHostContainer(instance : I | C) : void,
   popHostContainer() : void,
 
-  resetHostStacks() : void,
+  resetHostContext() : void,
+  saveHostContextToPortal(portal : Fiber): void,
+  restoreHostContextFromPortal(portal : Fiber): void,
 };
 
 module.exports = function<I, C>() : HostContext<I, C> {
   // Host instances currently on the stack that have not yet been committed.
-  const parentStack : Array<I | null> = [];
+  let parentStack : Array<I | null> = [];
   let parentIndex = -1;
 
   // Container instances currently on the stack (e.g. DOM uses this for SVG).
-  const containerStack : Array<C | I | null> = [];
+  let containerStack : Array<C | I | null> = [];
   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<I, C>() : HostContext<I, C> {
     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<I, C>() : HostContext<I, C> {
     pushHostContainer,
     popHostContainer,
 
-    resetHostStacks,
+    resetHostContext,
+    saveHostContextToPortal,
+    restoreHostContextFromPortal,
   };
 };

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -67,7 +67,7 @@ type TrappedError = {
 
 module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
   const hostContext = ReactFiberHostContext();
-  const { resetHostStacks } = hostContext;
+  const { resetHostContext } = hostContext;
   const { beginWork } = ReactFiberBeginWork(config, hostContext, scheduleUpdate);
   const { completeWork } = ReactFiberCompleteWork(config, hostContext);
   const {
@@ -222,7 +222,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     }
 
     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