Treat portals as fragments in test renderer

Author: Brian Vaughn

packages/react-reconciler/src/ReactFiber.js

@@ -16,7 +16,10 @@ import type {ExpirationTime} from './ReactFiberExpirationTime';
 import type {UpdateQueue} from './ReactUpdateQueue';
 
 import invariant from 'fbjs/lib/invariant';
-import {enableProfilerTimer} from 'shared/ReactFeatureFlags';
+import {
+  convertPortalsToFragments,
+  enableProfilerTimer,
+} from 'shared/ReactFeatureFlags';
 import {NoEffect} from 'shared/ReactTypeOfSideEffect';
 import {
   IndeterminateComponent,
@@ -513,7 +516,8 @@ export function createFiberFromPortal(
   expirationTime: ExpirationTime,
 ): Fiber {
   const pendingProps = portal.children !== null ? portal.children : [];
-  const fiber = createFiber(HostPortal, pendingProps, portal.key, mode);
+  const tag = convertPortalsToFragments ? Fragment : HostPortal;
+  const fiber = createFiber(tag, pendingProps, portal.key, mode);
   fiber.expirationTime = expirationTime;
   fiber.stateNode = {
     containerInfo: portal.containerInfo,

packages/react-test-renderer/src/ReactTestHostConfig.js

@@ -57,11 +57,6 @@ export function appendChild(
   parentInstance: Instance | Container,
   child: Instance | TextInstance,
 ): void {
-  // Detect and ignore ReactDOM.createPortal() usage.
-  // Test renderer's toJSON() method knows how to handle this case.
-  if (parentInstance instanceof HTMLElement) {
-    return;
-  }
   const index = parentInstance.children.indexOf(child);
   if (index !== -1) {
     parentInstance.children.splice(index, 1);
@@ -86,11 +81,6 @@ export function removeChild(
   parentInstance: Instance | Container,
   child: Instance | TextInstance,
 ): void {
-  // Detect and ignore ReactDOM.createPortal() usage.
-  // Test renderer's toJSON() method knows how to handle this case.
-  if (parentInstance instanceof HTMLElement) {
-    return;
-  }
   const index = parentInstance.children.indexOf(child);
   parentInstance.children.splice(index, 1);
 }

packages/react-test-renderer/src/ReactTestRenderer.js

@@ -11,8 +11,6 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber';
 import type {FiberRoot} from 'react-reconciler/src/ReactFiberRoot';
 import type {Instance, TextInstance} from './ReactTestHostConfig';
 
-import React from 'react';
-import {typeOf} from 'react-is';
 import * as TestRenderer from 'react-reconciler/inline.test';
 import {batchedUpdates} from 'events/ReactGenericBatching';
 import {findCurrentFiberUsingSlowPath} from 'react-reconciler/reflection';
@@ -73,15 +71,7 @@ function toJSON(inst: Instance | TextInstance): ReactTestRendererNode {
       const {children, ...props} = inst.props;
       /* eslint-enable */
       let renderedChildren = null;
-      if (inst.props != null && inst.props.children != null) {
-        // At least some of this element's children are from another renderer
-        // (e.g. ReactDOM.createPortal)
-        // In this case, treat all children as potentially from the other renderer.
-        // If we don't, the JSON representation may contain overlaps.
-        renderedChildren = React.Children.toArray(inst.props.children).map(
-          reactElementToJSON,
-        );
-      } else if (inst.children && inst.children.length) {
+      if (inst.children && inst.children.length) {
         renderedChildren = inst.children.map(toJSON);
       }
 
@@ -99,25 +89,6 @@ function toJSON(inst: Instance | TextInstance): ReactTestRendererNode {
   }
 }
 
-function reactElementToJSON(inst: any): ReactTestRendererNode {
-  if (typeof inst === 'object') {
-    /* eslint-disable no-unused-vars */
-    // We don't include the `children` prop in JSON.
-    // Instead, we will include the actual rendered children.
-    const {children, ...props} = inst.props != null ? inst.props : {};
-    const type = typeOf(inst);
-    return {
-      type: type == null ? 'unknown' : type.toString(),
-      props,
-      children: React.Children.toArray(
-        inst.children || inst.props.children,
-      ).map(reactElementToJSON),
-    };
-  } else {
-    return inst;
-  }
-}
-
 function childrenToTree(node) {
   if (!node) {
     return null;

packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js renamed to packages/react-test-renderer/src/__tests__/ReactTestRendererPortals-test.internal.js

@@ -14,22 +14,25 @@ const ReactDOM = require('react-dom');
 
 // Isolate test renderer.
 jest.resetModules();
+jest.mock('shared/ReactFeatureFlags', () =>
+  require('shared/forks/ReactFeatureFlags.test-renderer'),
+);
 const ReactTestRenderer = require('react-test-renderer');
 
 describe('ReactTestRenderer', () => {
-  it('should support ReactDOM portal usage', () => {
+  it('should support ReactDOM.createPortal', () => {
     const container = document.createElement('div');
     let rendered = ReactTestRenderer.create(
       <div>
-        {ReactDOM.createPortal(<span>Rendered by ReactDOM</span>, container)}
+        {ReactDOM.createPortal(<span>ReactDOM portal</span>, container)}
       </div>,
     );
     expect(rendered.toJSON()).toMatchSnapshot();
 
     rendered.update(
       <div>
-        <span>Rendered by ReactTestRenderer</span>
-        {ReactDOM.createPortal(<span>Rendered by ReactDOM</span>, container)}
+        <span>ReactTestRenderer span</span>
+        {ReactDOM.createPortal(<span>ReactDOM portal</span>, container)}
       </div>,
     );
     expect(rendered.toJSON()).toMatchSnapshot();

packages/react-test-renderer/src/__tests__/__snapshots__/ReactTestRenderer-test.js.snap

@@ -0,0 +1,20 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`ReactTestRenderer should support ReactDOM.createPortal 1`] = `
+<div>
+  <span>
+    ReactDOM portal
+  </span>
+</div>
+`;
+
+exports[`ReactTestRenderer should support ReactDOM.createPortal 2`] = `
+<div>
+  <span>
+    ReactTestRenderer span
+  </span>
+  <span>
+    ReactDOM portal
+  </span>
+</div>
+`;

packages/react-test-renderer/src/__tests__/__snapshots__/ReactTestRendererPortals-test.internal.js.snap

@@ -42,6 +42,10 @@ export const enableProfilerTimer = __DEV__;
 // Fires getDerivedStateFromProps for state *or* props changes
 export const fireGetDerivedStateFromPropsOnStateUpdates = true;
 
+// Return a fragment from createPortal()
+// This enables TestRenderer to consume portals from e.g. ReactDOM
+export const convertPortalsToFragments = false;
+
 // Only used in www builds.
 export function addUserTimingListener() {
   invariant(false, 'Not implemented.');

packages/shared/ReactFeatureFlags.js

@@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = __DEV__;
 export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
 export const enableProfilerTimer = __DEV__;
 export const fireGetDerivedStateFromPropsOnStateUpdates = true;
+export const convertPortalsToFragments = false;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.native-fabric-fb.js

@@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = false;
 export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
 export const enableProfilerTimer = false;
 export const fireGetDerivedStateFromPropsOnStateUpdates = true;
+export const convertPortalsToFragments = false;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.native-fabric-oss.js

@@ -27,6 +27,7 @@ export const {
 // The rest of the flags are static for better dead code elimination.
 export const enableUserTimingAPI = __DEV__;
 export const warnAboutLegacyContextAPI = __DEV__;
+export const convertPortalsToFragments = false;
 
 // Only used in www builds.
 export function addUserTimingListener() {