Upgrade to React==16.4.1 & Enzyme==3.3.0 (#5359)

* Bumping react==16.4.1 & enzyme==3.3.0 The upgrade was pretty smooth except for a cryptic message coming out of react-select around running multiple copies of React. It turns out the `common` bundle had React and was conflicting with explore and dashboard apps, only in 16.x. This somehow wasn't a problem in 15.x outside of the wasted resources. Running 16.4 should bring in all sorts of perf improvements and features we've all been waiting for. https://reactjs.org/blog/2017/09/26/react-v16.0.html TODO: react-bootstrap-datetimepicker isn't compatible with React 16 * Trying to deprecate react-bootstrap-datetime * Moving forward * Reintroducing tests
apache · Sep 10, 2018 · 73db918 · 73db918
1 parent e35bfba
commit 73db918
Show file tree

Hide file tree

Showing 20 changed files with 516 additions and 559 deletions.
diff --git a/superset/assets/package.json b/superset/assets/package.json
@@ -8,9 +8,9 @@
     "test": "spec"
   },
   "scripts": {
-    "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js 'spec/**/*_spec.*'",
-    "test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js",
-    "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --compilers babel-core/register --require spec/helpers/browser.js --require ignore-styles 'spec/**/*_spec.*'",
+    "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*'",
+    "test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js",
+    "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --compilers babel-core/register --require spec/helpers/shim.js --require ignore-styles 'spec/**/*_spec.*'",
     "dev": "webpack --mode=development --colors --progress --debug --watch",
     "dev-server": "webpack-dev-server --mode=development --progress",
     "prod": "node --max_old_space_size=4096 webpack --mode=production --colors --progress",
@@ -86,7 +86,7 @@
     "nvd3": "1.8.6",
     "prop-types": "^15.6.0",
     "re-resizable": "^4.3.1",
-    "react": "^15.6.2",
+    "react": "^16.4.1",
     "react-ace": "^5.10.0",
     "react-addons-css-transition-group": "^15.6.0",
     "react-addons-shallow-compare": "^15.4.2",
@@ -98,7 +98,7 @@
     "react-datetime": "^2.14.0",
     "react-dnd": "^2.5.4",
     "react-dnd-html5-backend": "^2.5.4",
-    "react-dom": "^15.6.2",
+    "react-dom": "^16.4.1",
     "react-gravatar": "^2.6.1",
     "react-hot-loader": "^4.3.6",
     "react-map-gl": "^3.0.4",
@@ -113,7 +113,7 @@
     "react-sticky": "^6.0.2",
     "react-syntax-highlighter": "^7.0.4",
     "react-virtualized": "9.19.1",
-    "react-virtualized-select": "^2.4.0",
+    "react-virtualized-select": "^3.1.3",
     "reactable": "1.0.2",
     "redux": "^3.5.2",
     "redux-localstorage": "^0.4.1",
@@ -138,11 +138,13 @@
     "babel-plugin-syntax-dynamic-import": "^6.18.0",
     "babel-polyfill": "^6.23.0",
     "babel-preset-airbnb": "^2.1.1",
+    "babel-preset-env": "^1.7.0",
     "chai": "^4.0.2",
     "clean-webpack-plugin": "^0.1.19",
     "css-loader": "^0.28.0",
     "cypress": "^3.0.3",
-    "enzyme": "^2.0.0",
+    "enzyme": "^3.3.0",
+    "enzyme-adapter-react-16": "^1.1.1",
     "eslint": "^4.19.0",
     "eslint-config-airbnb": "^15.0.1",
     "eslint-config-prettier": "^2.9.0",
@@ -152,7 +154,7 @@
     "eslint-plugin-react": "^7.0.1",
     "exports-loader": "^0.7.0",
     "file-loader": "^1.1.11",
-    "github-changes": "^1.0.4",
+    "gl": "^4.0.4",
     "ignore-styles": "^5.0.1",
     "imports-loader": "^0.7.1",
     "istanbul": "^1.0.0-alpha",
@@ -163,7 +165,7 @@
     "mini-css-extract-plugin": "^0.4.0",
     "minimist": "^1.2.0",
     "mocha": "^3.5.3",
-    "npm-check-updates": "^2.14.0",
+    "npm-check-updates": "^2.14.2",
     "optimize-css-assets-webpack-plugin": "^5.0.1",
     "po2json": "^0.4.5",
     "prettier": "^1.12.1",
@@ -175,7 +177,7 @@
     "transform-loader": "^0.2.3",
     "uglifyjs-webpack-plugin": "^1.1.0",
     "url-loader": "^1.0.1",
-    "webpack": "^4.17.1",
+    "webpack": "^4.18.0",
     "webpack-assets-manifest": "^3.0.1",
     "webpack-cli": "^2.0.10",
     "webpack-dev-server": "^3.1.7",

diff --git a/superset/assets/spec/helpers/browser.js → superset/assets/spec/helpers/shim.js b/superset/assets/spec/helpers/browser.js → superset/assets/spec/helpers/shim.js
@@ -2,6 +2,10 @@
 import 'babel-polyfill';
 import chai from 'chai';
 import jsdom from 'jsdom';
+import { configure } from 'enzyme';
+import Adapter from 'enzyme-adapter-react-16';
+
+configure({ adapter: new Adapter() });
 
 require('babel-register')({
   // NOTE: If `dynamic-import-node` is in .babelrc alongside
@@ -51,3 +55,4 @@ global.window.XMLHttpRequest = global.XMLHttpRequest;
 global.window.location = { href: 'about:blank' };
 global.window.performance = { now: () => new Date().getTime() };
 global.$ = require('jquery')(global.window);
+
diff --git a/superset/assets/spec/javascripts/chart/Chart_spec.jsx b/superset/assets/spec/javascripts/chart/Chart_spec.jsx
@@ -66,12 +66,10 @@ describe('Chart', () => {
     });
 
     it('should call after resize', () => {
-      const prevProp = wrapper.props();
       wrapper.setProps({
         chartStatus: 'rendered',
         height: 100,
       });
-      wrapper.instance().componentDidUpdate(prevProp);
       expect(stub.callCount).to.equals(1);
     });
   });

diff --git a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx
@@ -63,7 +63,6 @@ describe('AsyncSelect', () => {
         <AsyncSelect {...mockedProps} autoSelect />,
       );
       const spy = sinon.spy(wrapper.instance(), 'onChange');
-      wrapper.instance().fetchOptions();
       server.respond();
 
       expect(spy.callCount).to.equal(1);

diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
@@ -152,7 +152,6 @@ describe('Dashboard', () => {
     it('should call refresh if a filter is added', () => {
       const wrapper = setup({ dashboardState: overrideDashboardState });
       const refreshExceptSpy = sinon.spy(wrapper.instance(), 'refreshExcept');
-      const prevProps = wrapper.instance().props;
       wrapper.setProps({
         dashboardState: {
           ...overrideDashboardState,
@@ -162,30 +161,26 @@ describe('Dashboard', () => {
           },
         },
       });
-      wrapper.instance().componentDidUpdate(prevProps);
       refreshExceptSpy.restore();
       expect(refreshExceptSpy.callCount).to.equal(1);
     });
 
     it('should call refresh if a filter is removed', () => {
       const wrapper = setup({ dashboardState: overrideDashboardState });
       const refreshExceptSpy = sinon.spy(wrapper.instance(), 'refreshExcept');
-      const prevProps = wrapper.instance().props;
       wrapper.setProps({
         dashboardState: {
           ...overrideDashboardState,
           filters: {},
         },
       });
-      wrapper.instance().componentDidUpdate(prevProps);
       refreshExceptSpy.restore();
       expect(refreshExceptSpy.callCount).to.equal(1);
     });
 
     it('should call refresh if a filter is changed', () => {
       const wrapper = setup({ dashboardState: overrideDashboardState });
       const refreshExceptSpy = sinon.spy(wrapper.instance(), 'refreshExcept');
-      const prevProps = wrapper.instance().props;
       wrapper.setProps({
         dashboardState: {
           ...overrideDashboardState,
@@ -195,15 +190,13 @@ describe('Dashboard', () => {
           },
         },
       });
-      wrapper.instance().componentDidUpdate(prevProps);
       refreshExceptSpy.restore();
       expect(refreshExceptSpy.callCount).to.equal(1);
     });
 
     it('should not call refresh if filters change and refresh is false', () => {
       const wrapper = setup({ dashboardState: overrideDashboardState });
       const refreshExceptSpy = sinon.spy(wrapper.instance(), 'refreshExcept');
-      const prevProps = wrapper.instance().props;
       wrapper.setProps({
         dashboardState: {
           ...overrideDashboardState,
@@ -214,7 +207,6 @@ describe('Dashboard', () => {
           refresh: false,
         },
       });
-      wrapper.instance().componentDidUpdate(prevProps);
       refreshExceptSpy.restore();
       expect(refreshExceptSpy.callCount).to.equal(0);
     });

diff --git a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Markdown_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Markdown_spec.jsx
@@ -113,9 +113,10 @@ describe('Markdown', () => {
     // the mode dropdown onchange instead
     const dropdown = wrapper.find(MarkdownModeDropdown);
     dropdown.prop('onChange')('preview');
+    wrapper.update();
 
-    expect(wrapper.find(AceEditor)).to.have.length(0);
     expect(wrapper.find(ReactMarkdown)).to.have.length(1);
+    expect(wrapper.find(AceEditor)).to.have.length(0);
   });
 
   it('should call updateComponents when editMode changes from edit => preview, and there are markdownSource changes', () => {

diff --git a/superset/assets/spec/javascripts/explore/components/AdhocFilterEditPopover_spec.jsx b/superset/assets/spec/javascripts/explore/components/AdhocFilterEditPopover_spec.jsx
@@ -106,7 +106,7 @@ describe('AdhocFilterEditPopover', () => {
 
     expect(wrapper.find('i.glyphicon-resize-full')).to.have.lengthOf(1);
     expect(wrapper.instance().onDragDown.calledOnce).to.be.false;
-    wrapper.find('i.glyphicon-resize-full').simulate('mouseDown');
+    wrapper.find('i.glyphicon-resize-full').simulate('mouseDown', {});
     expect(wrapper.instance().onDragDown.calledOnce).to.be.true;
   });
 });
diff --git a/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopoverTitle_spec.jsx b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopoverTitle_spec.jsx
@@ -36,7 +36,7 @@ describe('AdhocMetricEditPopoverTitle', () => {
   it('renders an OverlayTrigger wrapper with the title', () => {
     const { wrapper } = setup();
     expect(wrapper.find(OverlayTrigger)).to.have.lengthOf(1);
-    expect(wrapper.find(OverlayTrigger).dive().text()).to.equal('My Metric\xa0');
+    expect(wrapper.find(OverlayTrigger).find('span').text()).to.equal('My Metric\xa0');
   });
 
   it('transfers to edit mode when clicked', () => {

diff --git a/superset/assets/spec/javascripts/explore/components/DateFilterControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/DateFilterControl_spec.jsx
@@ -4,7 +4,7 @@ import sinon from 'sinon';
 import { expect } from 'chai';
 import { describe, it, beforeEach } from 'mocha';
 import { shallow } from 'enzyme';
-import { Button } from 'react-bootstrap';
+import { Button, Label } from 'react-bootstrap';
 
 import DateFilterControl from '../../../../src/explore/components/controls/DateFilterControl';
 import ControlHeader from '../../../../src/explore/components/ControlHeader';
@@ -29,36 +29,28 @@ describe('DateFilterControl', () => {
     expect(controlHeader).to.have.lengthOf(1);
   });
   it('renders 3 Buttons', () => {
-    const label = wrapper.find('.label').first();
+    const label = wrapper.find(Label).first();
     label.simulate('click');
     setTimeout(() => {
       expect(wrapper.find(Button)).to.have.length(3);
     }, 10);
   });
   it('loads the right state', () => {
-    const label = wrapper.find('.label').first();
+    const label = wrapper.find(Label).first();
     label.simulate('click');
     setTimeout(() => {
       expect(wrapper.state().num).to.equal('90');
     }, 10);
   });
-  it('sets now and closes', () => {
-    const label = wrapper.find('.now').first();
-    label.simulate('click');
-    setTimeout(() => {
-      expect(wrapper.state().free).to.equal('now');
-      expect(wrapper.find('.popover')).to.have.length(0);
-    }, 10);
-  });
   it('renders 2 dimmed sections', () => {
-    const label = wrapper.find('.label').first();
+    const label = wrapper.find(Label).first();
     label.simulate('click');
     setTimeout(() => {
       expect(wrapper.find(Button)).to.have.length(3);
     }, 10);
   });
   it('opens and closes', () => {
-    const label = wrapper.find('.label').first();
+    const label = wrapper.find(Label).first();
     label.simulate('click');
     setTimeout(() => {
       expect(wrapper.find('.popover')).to.have.length(1);

diff --git a/superset/assets/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx b/superset/assets/spec/javascripts/explore/components/DisplayQueryButton_spec.jsx
@@ -2,7 +2,6 @@ import React from 'react';
 import { expect } from 'chai';
 import { describe, it } from 'mocha';
 import { mount } from 'enzyme';
-import { Modal } from 'react-bootstrap';
 import ModalTrigger from './../../../../src/components/ModalTrigger';
 
 import DisplayQueryButton from '../../../../src/explore/components/DisplayQueryButton';
@@ -27,6 +26,5 @@ describe('DisplayQueryButton', () => {
   it('renders a dropdown', () => {
     const wrapper = mount(<DisplayQueryButton {...defaultProps} />);
     expect(wrapper.find(ModalTrigger)).to.have.lengthOf(2);
-    expect(wrapper.find(Modal)).to.have.lengthOf(2);
   });
 });
diff --git a/superset/assets/spec/javascripts/sqllab/TableElement_spec.jsx b/superset/assets/spec/javascripts/sqllab/TableElement_spec.jsx
@@ -36,7 +36,7 @@ describe('TableElement', () => {
     mount(<TableElement {...mockedProps} />);
   });
   it('sorts columns', () => {
-    const wrapper = mount(<TableElement {...mockedProps} />);
+    const wrapper = shallow(<TableElement {...mockedProps} />);
     expect(wrapper.state().sortColumns).to.equal(false);
     expect(wrapper.find(ColumnElement).first().props().column.name).to.equal('id');
     wrapper.find('.sort-cols').simulate('click');
@@ -50,7 +50,7 @@ describe('TableElement', () => {
     expect(mockedActions.collapseTable.called).to.equal(true);
   });
   it('removes the table', () => {
-    const wrapper = mount(<TableElement {...mockedProps} />);
+    const wrapper = shallow(<TableElement {...mockedProps} />);
     expect(wrapper.state().expanded).to.equal(true);
     wrapper.find('.table-remove').simulate('click');
     expect(wrapper.state().expanded).to.equal(false);

diff --git a/superset/assets/spec/javascripts/visualizations/table_spec.jsx b/superset/assets/spec/javascripts/visualizations/table_spec.jsx
@@ -1,7 +1,7 @@
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 import $ from 'jquery';
-import '../../helpers/browser';
+import '../../helpers/shim';
 import tableVis from '../../../src/visualizations/table';
 
 describe('table viz', () => {

diff --git a/superset/assets/src/SqlLab/actions.js b/superset/assets/src/SqlLab/actions.js
@@ -9,7 +9,7 @@ import {
   addDangerToast as addDangerToastAction,
   addInfoToast as addInfoToastAction,
 } from '../messageToasts/actions';
-import { COMMON_ERR_MESSAGES } from '../common';
+import { COMMON_ERR_MESSAGES } from '../utils/common';
 
 export const RESET_STATE = 'RESET_STATE';
 export const ADD_QUERY_EDITOR = 'ADD_QUERY_EDITOR';
@@ -441,7 +441,6 @@ export function popDatasourceQuery(datasourceKey, sql) {
     });
   };
 }
-
 export function createDatasourceStarted() {
   return { type: CREATE_DATASOURCE_STARTED };
 }

diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx
@@ -89,7 +89,7 @@ class SqlEditor extends React.PureComponent {
       height,
     });
 
-    if (this.refs.ace.clientHeight) {
+    if (this.refs.ace && this.refs.ace.clientHeight) {
       this.props.actions.persistEditorHeight(this.props.queryEditor, this.refs.ace.clientHeight);
     }
   }

diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js
@@ -3,7 +3,7 @@ import URI from 'urijs';
 import { getExploreUrlAndPayload, getAnnotationJsonUrl } from '../explore/exploreUtils';
 import { requiresQuery, ANNOTATION_SOURCE_TYPES } from '../modules/AnnotationTypes';
 import { Logger, LOG_ACTIONS_LOAD_CHART } from '../logger';
-import { COMMON_ERR_MESSAGES } from '../common';
+import { COMMON_ERR_MESSAGES } from '../utils/common';
 import { t } from '../locales';
 
 const $ = (window.$ = require('jquery'));

diff --git a/superset/assets/src/common.js b/superset/assets/src/common.js
@@ -1,6 +1,7 @@
 /* eslint-disable global-require */
 import $ from 'jquery';
-import { t } from './locales';
+// Everything imported in this file ends up in the common entry file
+// be mindful of double-imports
 
 const utils = require('./modules/utils');
 
@@ -31,8 +32,3 @@ export function appSetup() {
   window.jQuery = $;
   require('bootstrap');
 }
-
-// Error messages used in many places across applications
-export const COMMON_ERR_MESSAGES = {
-  SESSION_TIMED_OUT: t('Your session timed out, please refresh your page and try again.'),
-};
diff --git a/superset/assets/src/explore/components/AdhocMetricEditPopover.jsx b/superset/assets/src/explore/components/AdhocMetricEditPopover.jsx
@@ -86,10 +86,12 @@ export default class AdhocMetricEditPopover extends React.Component {
   }
 
   onColumnChange(column) {
-    this.setState({ adhocMetric: this.state.adhocMetric.duplicateWith({
-      column,
-      expressionType: EXPRESSION_TYPES.SIMPLE,
-    }) });
+    this.setState({
+      adhocMetric: this.state.adhocMetric.duplicateWith({
+        column,
+        expressionType: EXPRESSION_TYPES.SIMPLE,
+      }),
+    });
   }
 
   onAggregateChange(aggregate) {

diff --git a/superset/assets/src/explore/components/controls/SpatialControl.jsx b/superset/assets/src/explore/components/controls/SpatialControl.jsx
@@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
 import {
   Row, Col, Button, Label, OverlayTrigger, Popover,
 } from 'react-bootstrap';
-import 'react-datetime/css/react-datetime.css';
 
 import ControlHeader from '../ControlHeader';
 import SelectControl from './SelectControl';

diff --git a/superset/assets/src/utils/common.js b/superset/assets/src/utils/common.js
@@ -1,5 +1,6 @@
 /* eslint global-require: 0 */
 import $ from 'jquery';
+import { t } from '../locales';
 
 const d3 = require('d3');
 
@@ -132,3 +133,8 @@ export function optionFromValue(opt) {
   // From a list of options, handles special values & labels
   return { value: optionValue(opt), label: optionLabel(opt) };
 }
+
+// Error messages used in many places across applications
+export const COMMON_ERR_MESSAGES = {
+  SESSION_TIMED_OUT: t('Your session timed out, please refresh your page and try again.'),
+};
-Original file line number
+Diff line change
@@ Expand Up / @@ -89,7 +89,7 @@ class SqlEditor extends React.PureComponent { @@
           height,
         });
-        if (this.refs.ace.clientHeight) {
+        if (this.refs.ace && this.refs.ace.clientHeight) {
           this.props.actions.persistEditorHeight(this.props.queryEditor, this.refs.ace.clientHeight);
         }
       }
@@ Expand Down @@