📦 React 15 support (#2426)

* REACT=15 env variable support, test-react15 package for deps * remove isomorphic from main test-commons import because it only supports React 16 * fix message * fix core tests * add circle job `test-15` * remove portal logs * REACT env var in circle * icons isotest * isotests run in 15 or 16, test-commons scripts use require imports * revert isomorphic entry change * fix test imports * react >=15.3.0 peer dependency (for PureComponent) * update docs about react version * Portal docs section about React 15 * rename circle jobs * isReact15 => cannotCreatePortal * revert require() syntax where possible * peer dependency `^15.3.0 || 16` * revert to commonjs syntax with a comment * semantic merge circle config * circle cache * remove console color from enzyme message seems to bork karma-junit-reorter karma-runner/karma-junit-reporter#116
palantir · May 1, 2018 · 9a93f09 · 9a93f09 · blueprint-bot · May 1, 2018
1 parent 813e93f
commit 9a93f09
Show file tree

Hide file tree

Showing 22 changed files with 242 additions and 78 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -98,12 +98,43 @@ jobs:
             - packages/*/dist
             - packages/*/lib
 
-  test:
+  test-react-15:
     docker:
       - image: circleci/node:8.10.0-browsers
         environment:
             CHROME_BIN: "/usr/bin/google-chrome"
-    parallelism: 4
+    environment:
+      JUNIT_REPORT_PATH: reports
+      REACT: 15 # use React 15 for this job
+    parallelism: 3
+    steps:
+      - checkout
+      - attach_workspace:
+          at: '.'
+      - restore_cache:
+          key: v9-dependency-cache-{{ checksum "yarn.lock" }}
+      - run: mkdir ./reports
+      - run:
+          command: |
+              case $CIRCLE_NODE_INDEX in \
+              0) yarn lerna run --parallel test:pre ;; \
+              1) yarn lerna run --parallel test:iso ;; \
+              2) yarn lerna run --parallel test:karma ;; \
+              esac
+          when: always
+      - store_test_results:
+          path: ./reports
+      - store_artifacts:
+          path: ./reports
+
+  test-react-16:
+    docker:
+      - image: circleci/node:8.10.0-browsers
+        environment:
+            CHROME_BIN: "/usr/bin/google-chrome"
+    environment:
+      JUNIT_REPORT_PATH: reports
+    parallelism: 3
     steps:
       - checkout
       - attach_workspace:
@@ -118,8 +149,6 @@ jobs:
               1) yarn lerna run --parallel test:iso ;; \
               2) yarn lerna run --parallel test:karma ;; \
               esac
-          environment:
-            JUNIT_REPORT_PATH: reports
           when: always
       - store_test_results:
           path: ./reports
@@ -167,7 +196,9 @@ workflows:
           requires: [install-dependencies]
       - dist:
           requires: [compile]
-      - test:
+      - test-react-15:
+          requires: [compile]
+      - test-react-16:
           requires: [compile]
       - deploy-preview:
           requires: [dist]

diff --git a/packages/core/package.json b/packages/core/package.json
@@ -45,8 +45,8 @@
     "tslib": "^1.9.0"
   },
   "peerDependencies": {
-    "react": "^16.2.0",
-    "react-dom": "^16.0.0",
+    "react": "^15.3.0 || 16",
+    "react-dom": "^15.3.0 || 16",
     "react-transition-group": "^2.2.1"
   },
   "devDependencies": {

diff --git a/packages/core/src/components/button/abstractButton.tsx b/packages/core/src/components/button/abstractButton.tsx
@@ -144,19 +144,17 @@ export abstract class AbstractButton<H extends React.HTMLAttributes<any>> extend
 
     protected renderChildren(): React.ReactNode {
         const { children, icon, loading, rightIcon, text } = this.props;
-        return (
-            <>
-                {loading && <Spinner className={classNames(Classes.SMALL, Classes.BUTTON_SPINNER)} />}
-                <Icon icon={icon} />
-                {(text || children) && (
-                    <span className={Classes.BUTTON_TEXT}>
-                        {text}
-                        {children}
-                    </span>
-                )}
-                <Icon icon={rightIcon} />
-            </>
-        );
+        return [
+            loading && <Spinner key="loading" className={classNames(Classes.SMALL, Classes.BUTTON_SPINNER)} />,
+            <Icon key="leftIcon" icon={icon} />,
+            (text || children) && (
+                <span key="text" className={Classes.BUTTON_TEXT}>
+                    {text}
+                    {children}
+                </span>
+            ),
+            <Icon key="rightIcon" icon={rightIcon} />,
+        ];
     }
 }
 

diff --git a/packages/core/src/components/portal/portal.md b/packages/core/src/components/portal/portal.md
@@ -30,4 +30,18 @@ application.
 `Portal` supports the following options on its [React context](https://facebook.github.io/react/docs/context.html).
 To use them, supply a child context to a subtree that contains the Portals you want to customize.
 
+<div class="@ns-callout @ns-intent-primary @ns-icon-info-sign">
+    Blueprint uses the React 15-compatible `getChildContext()` API instead of the newer React 16.3 `React.createContext()` API.
+</div>
+
 @interface IPortalContext
+
+@### React 15
+
+In a **React 15** environment, `Portal` will use `ReactDOM.unstable_renderSubtreeIntoContainer` to achieve the teleportation effect, which has a few caveats:
+
+1. `Portal` `children` are wrapped in an extra `<div>` inside the portal container element.
+1. Test harnesses such as `enzyme` cannot trivially find elements "through" Portals as they're not in the same React tree.
+1. React `context` _is_ preserved (this one's a good thing).
+
+In a **React 16+** environment, the `Portal` component will use [`ReactDOM.createPortal`](https://reactjs.org/docs/portals.html) which preserves the React tree perfectly and does not require any of the above caveats.
diff --git a/packages/core/src/components/portal/portal.tsx b/packages/core/src/components/portal/portal.tsx
@@ -9,9 +9,13 @@ import * as ReactDOM from "react-dom";
 
 import * as Classes from "../../common/classes";
 import * as Errors from "../../common/errors";
-import { HTMLDivProps, IProps } from "../../common/props";
+import { IProps } from "../../common/props";
+import { isFunction } from "../../common/utils";
 
-export interface IPortalProps extends IProps, HTMLDivProps {
+/** Detect if `React.createPortal()` API method does not exist. */
+const cannotCreatePortal = !isFunction(ReactDOM.createPortal);
+
+export interface IPortalProps extends IProps {
     /**
      * Callback invoked when the children of this `Portal` have been added to the DOM.
      */
@@ -54,7 +58,7 @@ export class Portal extends React.Component<IPortalProps, IPortalState> {
         // Only render `children` once this component has mounted in a browser environment, so they are
         // immediately attached to the DOM tree and can do DOM things like measuring or `autoFocus`.
         // See long comment on componentDidMount in https://reactjs.org/docs/portals.html#event-bubbling-through-portals
-        if (typeof document === "undefined" || !this.state.hasMounted) {
+        if (cannotCreatePortal || typeof document === "undefined" || !this.state.hasMounted) {
             return null;
         } else {
             return ReactDOM.createPortal(this.props.children, this.portalElement);
@@ -65,6 +69,9 @@ export class Portal extends React.Component<IPortalProps, IPortalState> {
         this.portalElement = this.createContainerElement();
         document.body.appendChild(this.portalElement);
         this.setState({ hasMounted: true }, this.props.onChildrenMount);
+        if (cannotCreatePortal) {
+            this.unstableRenderNoPortal();
+        }
     }
 
     public componentDidUpdate(prevProps: IPortalProps) {
@@ -73,10 +80,16 @@ export class Portal extends React.Component<IPortalProps, IPortalState> {
             this.portalElement.classList.remove(prevProps.className);
             maybeAddClass(this.portalElement.classList, this.props.className);
         }
+        if (cannotCreatePortal) {
+            this.unstableRenderNoPortal();
+        }
     }
 
     public componentWillUnmount() {
         if (this.portalElement != null) {
+            if (cannotCreatePortal) {
+                ReactDOM.unmountComponentAtNode(this.portalElement);
+            }
             this.portalElement.remove();
         }
     }
@@ -90,6 +103,14 @@ export class Portal extends React.Component<IPortalProps, IPortalState> {
         }
         return container;
     }
+
+    private unstableRenderNoPortal() {
+        ReactDOM.unstable_renderSubtreeIntoContainer(
+            /* parentComponent */ this,
+            <div>{this.props.children}</div>,
+            this.portalElement,
+        );
+    }
 }
 
 function maybeAddClass(classList: DOMTokenList, className?: string) {

diff --git a/packages/core/test/alert/alertTests.tsx b/packages/core/test/alert/alertTests.tsx
@@ -11,6 +11,7 @@ import { SinonStub, spy, stub } from "sinon";
 
 import * as Errors from "../../src/common/errors";
 import { Alert, Button, Classes, IAlertProps, IButtonProps, Icon, Intent, Keys } from "../../src/index";
+import { findInPortal } from "../utils";
 
 describe("<Alert>", () => {
     it("renders its content correctly", () => {
@@ -136,7 +137,7 @@ describe("<Alert>", () => {
                     <p>There is no going back.</p>
                 </Alert>,
             );
-            const overlay = alert.find("." + Classes.OVERLAY).hostNodes();
+            const overlay = findInPortal(alert, "." + Classes.OVERLAY).first();
 
             overlay.simulate("keydown", { which: Keys.ESCAPE });
             assert.isTrue(onCancel.notCalled);
@@ -155,7 +156,7 @@ describe("<Alert>", () => {
                     <p>There is no going back.</p>
                 </Alert>,
             );
-            const backdrop = alert.find("." + Classes.OVERLAY_BACKDROP).hostNodes();
+            const backdrop = findInPortal(alert, "." + Classes.OVERLAY_BACKDROP).hostNodes();
 
             backdrop.simulate("mousedown");
             assert.isTrue(onCancel.notCalled);

diff --git a/packages/core/test/buttons/buttonTests.tsx b/packages/core/test/buttons/buttonTests.tsx
@@ -28,8 +28,8 @@ function buttonTestSuite(component: React.ComponentClass<any>, tagName: string)
 
         it('icon="style" renders Icon as first child', () => {
             const wrapper = button({ icon: "style" });
-            const firstChild = wrapper.children().childAt(0);
-            assert.isTrue(firstChild.is(Icon));
+            const firstChild = wrapper.find(Icon).at(0);
+            assert.strictEqual(firstChild.prop("icon"), "style");
         });
 
         it("renders the button text prop", () => {

diff --git a/packages/core/test/editable-text/editableTextTests.tsx b/packages/core/test/editable-text/editableTextTests.tsx
@@ -61,14 +61,10 @@ describe("<EditableText>", () => {
 
         it("calls onChange when escape key pressed and value is unconfirmed", () => {
             const changeSpy = spy();
-            const input = mount(
-                <EditableText isEditing={true} onChange={changeSpy} placeholder="Edit..." defaultValue="alphabet" />,
-            ).find("input");
-
-            input.simulate("keydown", { which: Keys.ESCAPE });
-            assert.equal(changeSpy.callCount, 0, "onChange called incorrectly"); // no change so no invoke
-
-            input.simulate("change", { target: { value: "hello" } }).simulate("keydown", { which: Keys.ESCAPE });
+            mount(<EditableText isEditing={true} onChange={changeSpy} placeholder="Edit..." defaultValue="alphabet" />)
+                .find("input")
+                .simulate("change", { target: { value: "hello" } })
+                .simulate("keydown", { which: Keys.ESCAPE });
             assert.equal(changeSpy.callCount, 2, "onChange not called twice"); // change & escape
             assert.deepEqual(changeSpy.args[1], ["alphabet"], `unexpected argument "${changeSpy.args[1][0]}"`);
         });

diff --git a/packages/core/test/isotest.js b/packages/core/test/isotest.js
@@ -6,13 +6,12 @@
 // @ts-check
 const { generateIsomorphicTests } = require("@blueprintjs/test-commons");
 const React = require("react");
-// TODO: get this to work with require("@std/esm")(module)("../lib/esm")
 const Core = require("../lib/cjs");
 
 const tooltipContent = { content: React.createElement("h1", {}, "content") };
 const customProps = {
     Hotkey: { combo: "mod+s", global: true, label: "save" },
-    Icon: { iconName: "pt-icon-build" },
+    Icon: { iconName: "build" },
     KeyCombo: { combo: "?" },
     Overlay: { lazy: false, usePortal: false },
     SVGTooltip: tooltipContent,

diff --git a/packages/core/test/menu/menuItemTests.tsx b/packages/core/test/menu/menuItemTests.tsx
@@ -89,7 +89,7 @@ describe("MenuItem", () => {
         const handleClose = spy();
         const menu = <MenuItem text="Graph" shouldDismissPopover={false} />;
         const wrapper = mount(
-            <Popover content={menu} isOpen={true} onInteraction={handleClose}>
+            <Popover content={menu} isOpen={true} onInteraction={handleClose} usePortal={false}>
                 <Button />
             </Popover>,
         );

diff --git a/packages/core/test/overlay/overlayTests.tsx b/packages/core/test/overlay/overlayTests.tsx
@@ -10,9 +10,9 @@ import * as React from "react";
 import { spy } from "sinon";
 
 import { dispatchMouseEvent } from "@blueprintjs/test-commons";
-
 import * as Keys from "../../src/common/keys";
 import { Classes, IOverlayProps, Overlay, Portal, Utils } from "../../src/index";
+import { findInPortal } from "../utils";
 
 const BACKDROP_SELECTOR = `.${Classes.OVERLAY_BACKDROP}`;
 
@@ -215,15 +215,16 @@ describe("<Overlay>", () => {
             const onClose = spy();
             mountWrapper(
                 <Overlay isOpen={true} onClose={onClose}>
-                    <div>
+                    <div id="outer-element">
                         {createOverlayContents()}
                         <Overlay isOpen={true}>
                             <div id="inner-element">{createOverlayContents()}</div>
                         </Overlay>
                     </div>
                 </Overlay>,
             );
-            wrapper.find("#inner-element").simulate("mousedown");
+            // this hackery is necessary for React 15 support, where Portals break trees.
+            findInPortal(findInPortal(wrapper, "#outer-element"), "#inner-element").simulate("mousedown");
             assert.isTrue(onClose.notCalled);
         });
 

diff --git a/packages/core/test/popover/popoverTests.tsx b/packages/core/test/popover/popoverTests.tsx
@@ -147,12 +147,12 @@ describe("<Popover>", () => {
 
     it("renders Portal when usePortal=true", () => {
         wrapper = renderPopover({ isOpen: true, usePortal: true });
-        assert.lengthOf(wrapper.find(Portal).find(`.${Classes.POPOVER}`), 1);
+        assert.lengthOf(wrapper.find(Portal), 1);
     });
 
     it("does not render Portal when usePortal=false", () => {
         wrapper = renderPopover({ isOpen: true, usePortal: false });
-        assert.lengthOf(wrapper.find(Portal).find(`.${Classes.POPOVER}`), 0);
+        assert.lengthOf(wrapper.find(Portal), 0);
     });
 
     it("empty content disables it and warns", () => {

diff --git a/packages/core/test/utils.ts b/packages/core/test/utils.ts
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2018 Palantir Technologies, Inc. All rights reserved.
+ * 
+ * Licensed under the terms of the LICENSE file distributed with this project.
+ */
+
+import { ReactWrapper } from "enzyme";
+import { Portal } from "../src";
+
+export function findInPortal(overlay: ReactWrapper, selector: string) {
+    // React 16: createPortal preserves React tree so simple find works.
+    const element = overlay.find(Portal).find(selector);
+    if (element.exists()) {
+        return element;
+    }
+
+    // React 15: unstable_renderSubtree does not preserve tree so we must create new wrapper.
+    const portal = overlay.find(Portal).instance();
+    const portalChildren = new ReactWrapper(portal.props.children as JSX.Element[]);
+    if (portalChildren.is(selector)) {
+        return portalChildren;
+    }
+    return portalChildren.find(selector);
+}