Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📦 React 15 support #2426

Merged
merged 24 commits into from
May 1, 2018
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,30 @@ jobs:
- packages/*/dist
- packages/*/lib

test:
test-react-15:
docker:
- image: circleci/node:8.10.0-browsers
environment:
CHROME_BIN: "/usr/bin/google-chrome"
environment:
REACT: 15 # use React 15 for this job
parallelism: 4
steps:
- checkout
- attach_workspace:
at: '.'
- restore_cache:
key: v7-dependency-cache-{{ checksum "yarn.lock" }}
- run:
command: |
case $CIRCLE_NODE_INDEX in \
0) yarn lerna run test --scope '@blueprintjs/core' ;; \
1) yarn lerna run test --scope '@blueprintjs/{datetime,timezone}' ;; \
2) yarn lerna run test --scope '@blueprintjs/table' ;; \
3) yarn lerna run test --scope '@blueprintjs/{icons,labs,select}' ;; \
esac

test-react-16:
docker:
- image: circleci/node:8.10.0-browsers
environment:
Expand Down Expand Up @@ -134,7 +157,9 @@ workflows:
requires: [install-dependencies]
- dist:
requires: [compile]
- test:
- test-react-15:
requires: [compile]
- test-react-16:
requires: [compile]
- deploy-preview:
requires: [dist]
Expand Down
4 changes: 2 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
24 changes: 11 additions & 13 deletions packages/core/src/components/button/abstractButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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} />,
];
}
}

Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/components/portal/portal.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you want to use React.createContext() or some other new React feature in the future? Supporting both versions might seem like a few hundred LOC right now, but it could end up being pretty limiting.

Copy link
Contributor Author

@giladgray giladgray Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree, it is quite limiting, but it's the quickest path to adoption.

shim/ponyfill packages are available, such as create-react-context, which is actually used in the latest version of react-popper to support multiple versions. so we could go down that route, like we did with pure-render-decorator.

</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.
27 changes: 24 additions & 3 deletions packages/core/src/components/portal/portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -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();
}
}
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions packages/core/test/alert/alertTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/buttons/buttonTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
12 changes: 4 additions & 8 deletions packages/core/test/editable-text/editableTextTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]}"`);
});
Expand Down
3 changes: 1 addition & 2 deletions packages/core/test/isotest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/menu/menuItemTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
);
Expand Down
7 changes: 4 additions & 3 deletions packages/core/test/overlay/overlayTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;

Expand Down Expand Up @@ -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);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/popover/popoverTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
24 changes: 24 additions & 0 deletions packages/core/test/utils.ts
Original file line number Diff line number Diff line change
@@ -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);
}
18 changes: 9 additions & 9 deletions packages/docs-app/src/getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

@### NPM packages

Blueprint is available as a collection of NPM packages under the `@blueprintjs` scope. The full
package list and their latest versions appear under the _Releases_ dropdown above.
Blueprint is available as a collection of NPM packages under the `@blueprintjs` scope.
Each package appears at the top level of the sidebar, along with its current version.

Each package contains a CSS file and a collection of CommonJS modules exposing React components.
The `main` module exports all symbols from all modules so you don't have to import individual files
Expand All @@ -19,7 +19,7 @@ The `main` module exports all symbols from all modules so you don't have to impo
yarn add @blueprintjs/core
```

1. If you see `UNMET PEER DEPENDENCY` errors, you should manually install React:
1. If you see `UNMET PEER DEPENDENCY` errors, you should manually install React (v15.3 or greater):

```sh
yarn add react react-dom react-transition-group
Expand Down Expand Up @@ -70,8 +70,8 @@ Blueprint supports the venerable [unpkg CDN](https://unpkg.com). Each package pr
library on the `Blueprint` global variable: `Blueprint.Core`, `Blueprint.Datetime`, etc.

These bundles _do not include_ external dependencies; your application will need to ensure that
`normalize.css`, `react`, `react-dom`, `react-transition-group`, `classnames`, `popper.js`, and
`react-popper`are available at runtime.
`normalize.css`, `classnames`, `dom4`, `react`, `react-dom`, `react-transition-group`, `popper.js`, and
`react-popper` are available at runtime.

```html
<!DOCTYPE html>
Expand All @@ -98,7 +98,7 @@ These bundles _do not include_ external dependencies; your application will need
<div id="btn"></div>
<script>
const button = React.createElement(Blueprint.Core.Button, {
icon: "predictive-analysis",
icon: "cloud",
text: "CDN Blueprint is go!",
});
ReactDOM.render(button, document.querySelector("#btn"));
Expand Down Expand Up @@ -139,8 +139,8 @@ install typings for Blueprint's dependencies before you can consume it:
# required for all @blueprintjs packages:
npm install --save @types/react @types/react-dom @types/react-transition-group

# @blueprintjs/datetime requires:
npm install --save @types/moment
# @blueprintjs/timezone requires:
npm install --save @types/moment-timezone
```

Blueprint's declaration files require **TypeScript 2.3+** for default generic parameter arguments: `<P = {}>`.
Expand Down Expand Up @@ -181,7 +181,7 @@ ReactDOM.unmountComponentAtNode(myContainerElement);

Check out the [React API docs](https://facebook.github.io/react/docs/react-api.html) for more details.

You'll need to install **React 16.2+** alongside Blueprint.
You'll need to install React v15.3 or greater alongside Blueprint.

```sh
npm install --save @blueprintjs/core react react-dom react-transition-group
Expand Down
11 changes: 8 additions & 3 deletions packages/test-commons/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@
* Copyright 2017 Palantir Technologies, Inc. All rights reserved.
*/

import "./polyfill";
// Note: using CommonJS syntax here so this can be used in the isomorphic tests, which must run in a server environment.
require("./polyfill");
Copy link
Contributor

@themadcreator themadcreator Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it necessary to switch to commonjs for this test code? shouldn't the node version that runs it support es6 modules?

Copy link
Contributor Author

@giladgray giladgray Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

necessary because isotest uses this now and that suite explicitly runs in a node environment (for server-side rendering). added code comment.


import * as Enzyme from "enzyme";
import * as Adapter from "enzyme-adapter-react-16";
const Enzyme = require("enzyme");
// test against React 15 with REACT=15 env variable.
const Adapter = require(`enzyme-adapter-react-${process.env.REACT || 16}`);

Enzyme.configure({ adapter: new Adapter() });

// tslint:disable-next-line:no-console
console.info(`Enzyme configured with \x1b[35m${Adapter.name}\x1b[0m`);
Loading