From 05257017a90a1e57d40eff7f49a80a55efafbc88 Mon Sep 17 00:00:00 2001 From: Gilad Gray Date: Mon, 23 Apr 2018 16:47:15 -0700 Subject: [PATCH 01/22] REACT=15 env variable support, test-react15 package for deps --- packages/test-commons/bootstrap.js | 9 ++- packages/test-commons/package.json | 3 +- packages/test-react15/package.json | 14 +++++ .../webpack.config.karma.js | 11 ++++ yarn.lock | 56 ++++++++++++++++++- 5 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 packages/test-react15/package.json diff --git a/packages/test-commons/bootstrap.js b/packages/test-commons/bootstrap.js index a1ee139fc1..d1ed4dfe4a 100644 --- a/packages/test-commons/bootstrap.js +++ b/packages/test-commons/bootstrap.js @@ -2,9 +2,12 @@ * Copyright 2017 Palantir Technologies, Inc. All rights reserved. */ -import "./polyfill"; +require("./polyfill"); -import * as Enzyme from "enzyme"; -import * as Adapter from "enzyme-adapter-react-16"; +const Enzyme = require("enzyme"); +// REACT env variable should be 15 or 16. +const Adapter = require(`enzyme-adapter-react-${process.env.REACT}`); Enzyme.configure({ adapter: new Adapter() }); +// tslint:disable-next-line:no-console +console.info(`Enzyme configured with ${adapter.name}.`); diff --git a/packages/test-commons/package.json b/packages/test-commons/package.json index e7ba2c9b49..d239e3fb53 100644 --- a/packages/test-commons/package.json +++ b/packages/test-commons/package.json @@ -15,10 +15,11 @@ "chai": "^4.1.2", "core-js": "^2.5.3", "enzyme": "^3.3.0", + "enzyme-adapter-react-15": "^1.0.5", "enzyme-adapter-react-16": "^1.1.1" }, "peerDependencies": { - "react": "^16.2.0" + "react": "^15.3.0 || ^16.2.0" }, "devDependencies": { "react": "^16.2.0", diff --git a/packages/test-react15/package.json b/packages/test-react15/package.json new file mode 100644 index 0000000000..279e18c6e1 --- /dev/null +++ b/packages/test-react15/package.json @@ -0,0 +1,14 @@ +{ + "name": "test-react15", + "version": "1.0.0", + "description": "React 15 dependencies for testing", + "main": "index.js", + "license": "MIT", + "private": true, + "dependencies": { + "enzyme-adapter-react-15": "^1.0.5", + "react": "^15.6.0", + "react-dom": "^15.6.0", + "react-test-renderer": "^15.6.0" + } +} diff --git a/packages/webpack-build-scripts/webpack.config.karma.js b/packages/webpack-build-scripts/webpack.config.karma.js index f52edc3cda..6226a3c69b 100644 --- a/packages/webpack-build-scripts/webpack.config.karma.js +++ b/packages/webpack-build-scripts/webpack.config.karma.js @@ -7,6 +7,8 @@ const CircularDependencyPlugin = require("circular-dependency-plugin"); const path = require("path"); const webpack = require("webpack"); +const REACT = process.env.REACT || "16"; + /** * This differs significantly from the base webpack config, so we don't even end up extending from it. */ @@ -16,6 +18,14 @@ module.exports = { devtool: "inline-source-map", resolve: { + // swap versions of React packages when this env variable is set + alias: REACT === "15" ? { + // use path.resolve for directory (require.resolve returns main file) + "prop-types": path.resolve(__dirname, "../test-react15/node_modules/prop-types"), + "react": path.resolve(__dirname, "../test-react15/node_modules/react"), + "react-dom": path.resolve(__dirname, "../test-react15/node_modules/react-dom"), + "react-test-renderer": path.resolve(__dirname, "../test-react15/node_modules/react-test-renderer"), + } : {}, extensions: [".css", ".js", ".ts", ".tsx"], }, @@ -58,6 +68,7 @@ module.exports = { new webpack.DefinePlugin({ "process.env": { NODE_ENV: JSON.stringify("test"), + REACT: JSON.stringify(REACT), }, }), diff --git a/yarn.lock b/yarn.lock index 38081dfbc5..5fd854ec91 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1659,6 +1659,14 @@ create-hmac@^1.1.0, create-hmac@^1.1.2, create-hmac@^1.1.4: safe-buffer "^5.0.1" sha.js "^2.4.8" +create-react-class@^15.6.0: + version "15.6.3" + resolved "https://registry.yarnpkg.com/create-react-class/-/create-react-class-15.6.3.tgz#2d73237fb3f970ae6ebe011a9e66f46dbca80036" + dependencies: + fbjs "^0.8.9" + loose-envify "^1.3.1" + object-assign "^4.1.1" + cross-env@^5.1.3: version "5.1.3" resolved "https://registry.yarnpkg.com/cross-env/-/cross-env-5.1.3.tgz#f8ae18faac87692b0a8b4d2f7000d4ec3a85dfd7" @@ -2333,6 +2341,16 @@ entities@^1.1.1, entities@~1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/entities/-/entities-1.1.1.tgz#6e5c2d0a5621b5dadaecef80b90edfb5cd7772f0" +enzyme-adapter-react-15@^1.0.5: + version "1.0.5" + resolved "https://registry.yarnpkg.com/enzyme-adapter-react-15/-/enzyme-adapter-react-15-1.0.5.tgz#99f9a03ff2c2303e517342935798a6bdfbb75fac" + dependencies: + enzyme-adapter-utils "^1.1.0" + lodash "^4.17.4" + object.assign "^4.0.4" + object.values "^1.0.4" + prop-types "^15.5.10" + enzyme-adapter-react-16@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/enzyme-adapter-react-16/-/enzyme-adapter-react-16-1.1.1.tgz#a8f4278b47e082fbca14f5bfb1ee50ee650717b4" @@ -2345,7 +2363,7 @@ enzyme-adapter-react-16@^1.1.1: react-reconciler "^0.7.0" react-test-renderer "^16.0.0-0" -enzyme-adapter-utils@^1.3.0: +enzyme-adapter-utils@^1.1.0, enzyme-adapter-utils@^1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/enzyme-adapter-utils/-/enzyme-adapter-utils-1.3.0.tgz#d6c85756826c257a8544d362cc7a67e97ea698c7" dependencies: @@ -2791,7 +2809,7 @@ faye-websocket@~0.11.0: dependencies: websocket-driver ">=0.5.1" -fbjs@^0.8.16: +fbjs@^0.8.16, fbjs@^0.8.9: version "0.8.16" resolved "https://registry.yarnpkg.com/fbjs/-/fbjs-0.8.16.tgz#5e67432f550dc41b572bf55847b8aca64e5337db" dependencies: @@ -6258,6 +6276,14 @@ promise@^7.1.1: dependencies: asap "~2.0.3" +prop-types@^15.5.10: + version "15.6.1" + resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.6.1.tgz#36644453564255ddda391191fb3a125cbdf654ca" + dependencies: + fbjs "^0.8.16" + loose-envify "^1.3.1" + object-assign "^4.1.1" + prop-types@^15.5.8, prop-types@^15.6.0: version "15.6.0" resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.6.0.tgz#ceaf083022fc46b4a35f69e13ef75aed0d639856" @@ -6455,6 +6481,15 @@ react-day-picker@^7.0.7: object-assign "^4.1.1" prop-types "^15.6.0" +react-dom@^15.6.0: + version "15.6.2" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-15.6.2.tgz#41cfadf693b757faf2708443a1d1fd5a02bef730" + dependencies: + fbjs "^0.8.9" + loose-envify "^1.1.0" + object-assign "^4.1.0" + prop-types "^15.5.10" + react-dom@^16.2.0: version "16.2.0" resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.2.0.tgz#69003178601c0ca19b709b33a83369fe6124c044" @@ -6480,6 +6515,13 @@ react-reconciler@^0.7.0: object-assign "^4.1.1" prop-types "^15.6.0" +react-test-renderer@^15.6.0: + version "15.6.2" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-15.6.2.tgz#d0333434fc2c438092696ca770da5ed48037efa8" + dependencies: + fbjs "^0.8.9" + object-assign "^4.1.0" + react-test-renderer@^16.0.0-0, react-test-renderer@^16.2.0: version "16.2.0" resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.2.0.tgz#bddf259a6b8fcd8555f012afc8eacc238872a211" @@ -6499,6 +6541,16 @@ react-transition-group@^2.2.1: prop-types "^15.5.8" warning "^3.0.0" +react@^15.6.0: + version "15.6.2" + resolved "https://registry.yarnpkg.com/react/-/react-15.6.2.tgz#dba0434ab439cfe82f108f0f511663908179aa72" + dependencies: + create-react-class "^15.6.0" + fbjs "^0.8.9" + loose-envify "^1.1.0" + object-assign "^4.1.0" + prop-types "^15.5.10" + react@^16.2.0: version "16.2.0" resolved "https://registry.yarnpkg.com/react/-/react-16.2.0.tgz#a31bd2dab89bff65d42134fa187f24d054c273ba" From 0bd01004d7a6d974a52b13adafcaba88a3ccb08e Mon Sep 17 00:00:00 2001 From: Gilad Gray Date: Mon, 23 Apr 2018 16:49:48 -0700 Subject: [PATCH 02/22] remove isomorphic from main test-commons import because it only supports React 16 --- packages/core/test/isotest.js | 5 ++--- packages/datetime/test/isotest.js | 2 +- packages/select/test/isotest.js | 2 +- packages/table/test/isotest.js | 2 +- packages/test-commons/isomorphic.js | 8 ++++++++ packages/test-commons/src/index.ts | 1 - packages/timezone/test/isotest.js | 2 +- 7 files changed, 14 insertions(+), 8 deletions(-) create mode 100644 packages/test-commons/isomorphic.js diff --git a/packages/core/test/isotest.js b/packages/core/test/isotest.js index 6e0ee4b150..2753dcb646 100644 --- a/packages/core/test/isotest.js +++ b/packages/core/test/isotest.js @@ -4,15 +4,14 @@ */ // @ts-check -const { generateIsomorphicTests } = require("@blueprintjs/test-commons"); +const { generateIsomorphicTests } = require("@blueprintjs/test-commons/isomorphic"); 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/datetime/test/isotest.js b/packages/datetime/test/isotest.js index e5618f6a6c..d3dfff668a 100644 --- a/packages/datetime/test/isotest.js +++ b/packages/datetime/test/isotest.js @@ -3,7 +3,7 @@ * Licensed under the terms of the LICENSE file distributed with this project. */ -const { generateIsomorphicTests } = require("@blueprintjs/test-commons"); +const { generateIsomorphicTests } = require("@blueprintjs/test-commons/isomorphic"); const React = require("react"); const DateTime = require("../lib/cjs"); diff --git a/packages/select/test/isotest.js b/packages/select/test/isotest.js index f376cdebf9..047084e917 100644 --- a/packages/select/test/isotest.js +++ b/packages/select/test/isotest.js @@ -4,7 +4,7 @@ */ // @ts-check -const { generateIsomorphicTests } = require("@blueprintjs/test-commons"); +const { generateIsomorphicTests } = require("@blueprintjs/test-commons/isomorphic"); const React = require("react"); const Select = require("../lib/cjs"); diff --git a/packages/table/test/isotest.js b/packages/table/test/isotest.js index 99d00b8790..2e815364eb 100644 --- a/packages/table/test/isotest.js +++ b/packages/table/test/isotest.js @@ -4,7 +4,7 @@ */ // @ts-check -const { generateIsomorphicTests } = require("@blueprintjs/test-commons"); +const { generateIsomorphicTests } = require("@blueprintjs/test-commons/isomorphic"); const React = require("react"); const Table = require("../lib/cjs"); diff --git a/packages/test-commons/isomorphic.js b/packages/test-commons/isomorphic.js new file mode 100644 index 0000000000..e505bc8507 --- /dev/null +++ b/packages/test-commons/isomorphic.js @@ -0,0 +1,8 @@ +/* + * Copyright 2017 Palantir Technologies, Inc. All rights reserved. + * + * This module has its own root so it is not included when tests import this package, + * as this module only uses React 16 -- no support for React 15 isotests. + */ + +module.exports = require("./lib/cjs/generateIsomorphicTests"); diff --git a/packages/test-commons/src/index.ts b/packages/test-commons/src/index.ts index c82150e8d3..7fbaa204ec 100644 --- a/packages/test-commons/src/index.ts +++ b/packages/test-commons/src/index.ts @@ -3,6 +3,5 @@ * Licensed under the terms of the LICENSE file distributed with this project. */ -export * from "./generateIsomorphicTests"; export * from "./testErrorBoundary"; export * from "./utils"; diff --git a/packages/timezone/test/isotest.js b/packages/timezone/test/isotest.js index 1a48a7b24f..dc557a3e2e 100644 --- a/packages/timezone/test/isotest.js +++ b/packages/timezone/test/isotest.js @@ -4,7 +4,7 @@ */ // @ts-check -const { generateIsomorphicTests } = require("@blueprintjs/test-commons"); +const { generateIsomorphicTests } = require("@blueprintjs/test-commons/isomorphic"); const React = require("react"); const Timezone = require("../lib/cjs"); From 4ae639682d4362eb0bad287bce9677c6d139a1e8 Mon Sep 17 00:00:00 2001 From: Gilad Gray Date: Mon, 23 Apr 2018 17:10:03 -0700 Subject: [PATCH 03/22] fix message --- packages/test-commons/bootstrap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/test-commons/bootstrap.js b/packages/test-commons/bootstrap.js index d1ed4dfe4a..92ad15e196 100644 --- a/packages/test-commons/bootstrap.js +++ b/packages/test-commons/bootstrap.js @@ -10,4 +10,4 @@ const Adapter = require(`enzyme-adapter-react-${process.env.REACT}`); Enzyme.configure({ adapter: new Adapter() }); // tslint:disable-next-line:no-console -console.info(`Enzyme configured with ${adapter.name}.`); +console.info(`Enzyme configured with ${Adapter.name}.`); From a1d4e08b4298a882c8bd5045145a74f0b754f5d9 Mon Sep 17 00:00:00 2001 From: Gilad Gray Date: Mon, 23 Apr 2018 17:10:16 -0700 Subject: [PATCH 04/22] fix core tests --- .../src/components/button/abstractButton.tsx | 24 +++++++-------- .../core/src/components/portal/portal.tsx | 30 +++++++++++++++++-- packages/core/test/alert/alertTests.tsx | 5 ++-- packages/core/test/buttons/buttonTests.tsx | 4 +-- .../test/editable-text/editableTextTests.tsx | 12 +++----- packages/core/test/menu/menuItemTests.tsx | 2 +- packages/core/test/overlay/overlayTests.tsx | 7 +++-- packages/core/test/popover/popoverTests.tsx | 4 +-- packages/core/test/utils.ts | 24 +++++++++++++++ 9 files changed, 78 insertions(+), 34 deletions(-) create mode 100644 packages/core/test/utils.ts diff --git a/packages/core/src/components/button/abstractButton.tsx b/packages/core/src/components/button/abstractButton.tsx index c917adf25f..ae901b3d01 100644 --- a/packages/core/src/components/button/abstractButton.tsx +++ b/packages/core/src/components/button/abstractButton.tsx @@ -144,19 +144,17 @@ export abstract class AbstractButton> extend protected renderChildren(): React.ReactNode { const { children, icon, loading, rightIcon, text } = this.props; - return ( - <> - {loading && } - - {(text || children) && ( - - {text} - {children} - - )} - - - ); + return [ + loading && , + , + (text || children) && ( + + {text} + {children} + + ), + , + ]; } } diff --git a/packages/core/src/components/portal/portal.tsx b/packages/core/src/components/portal/portal.tsx index 52deed5ec5..24c95bcb24 100644 --- a/packages/core/src/components/portal/portal.tsx +++ b/packages/core/src/components/portal/portal.tsx @@ -9,9 +9,11 @@ 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"; -export interface IPortalProps extends IProps, HTMLDivProps { +const isReact15 = typeof ReactDOM.createPortal === "undefined"; + +export interface IPortalProps extends IProps { /** * Callback invoked when the children of this `Portal` have been added to the DOM. */ @@ -54,17 +56,25 @@ export class Portal extends React.Component { // 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 (isReact15 || typeof document === "undefined" || !this.state.hasMounted) { return null; } else { + console.log("Creating portal"); return ReactDOM.createPortal(this.props.children, this.portalElement); } } + public componentDidCatch(e: any) { + console.log(e); + } + public componentDidMount() { this.portalElement = this.createContainerElement(); document.body.appendChild(this.portalElement); this.setState({ hasMounted: true }, this.props.onChildrenMount); + if (isReact15) { + this.renderReact15(); + } } public componentDidUpdate(prevProps: IPortalProps) { @@ -73,10 +83,16 @@ export class Portal extends React.Component { this.portalElement.classList.remove(prevProps.className); maybeAddClass(this.portalElement.classList, this.props.className); } + if (isReact15) { + this.renderReact15(); + } } public componentWillUnmount() { if (this.portalElement != null) { + if (isReact15) { + ReactDOM.unmountComponentAtNode(this.portalElement); + } this.portalElement.remove(); } } @@ -90,6 +106,14 @@ export class Portal extends React.Component { } return container; } + + private renderReact15() { + ReactDOM.unstable_renderSubtreeIntoContainer( + /* parentComponent */ this, +
{this.props.children}
, + this.portalElement, + ); + } } function maybeAddClass(classList: DOMTokenList, className?: string) { diff --git a/packages/core/test/alert/alertTests.tsx b/packages/core/test/alert/alertTests.tsx index 5a3bc545bb..795382c26d 100644 --- 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("", () => { it("renders its content correctly", () => { @@ -136,7 +137,7 @@ describe("", () => {

There is no going back.

, ); - 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("", () => {

There is no going back.

, ); - 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 index ed2b03a319..e2ac748e02 100644 --- a/packages/core/test/buttons/buttonTests.tsx +++ b/packages/core/test/buttons/buttonTests.tsx @@ -28,8 +28,8 @@ function buttonTestSuite(component: React.ComponentClass, 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 index deb732e143..f89b88a465 100644 --- a/packages/core/test/editable-text/editableTextTests.tsx +++ b/packages/core/test/editable-text/editableTextTests.tsx @@ -61,14 +61,10 @@ describe("", () => { it("calls onChange when escape key pressed and value is unconfirmed", () => { const changeSpy = spy(); - const input = mount( - , - ).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() + .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/menu/menuItemTests.tsx b/packages/core/test/menu/menuItemTests.tsx index e96d1f3792..0889df4191 100644 --- 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 = ; const wrapper = mount( - +