Update create-react-app to 2.1.2 (#302)

* WIP upgrade to create-react-app v2.1.2 Requires changes from PR #346 which are published as react-app-rewired@2.0.2-next.0 timarney/react-app-rewired#346 Signed-off-by: Joe Farro <joef@uber.com> * Use node 8, less liberal browser support, fix test Signed-off-by: Joe Farro <joef@uber.com> * Use eslintrc, fix CI build, fix pre-commit hook Make sure the ./packages/jaeger-ui uses the ./.eslintrc. Make sure all tests are run in pre-commit hook. CRA is now failing builds in CI if there are any webpack build warnings: https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184 Signed-off-by: Joe Farro <joef@uber.com> * Skip react-vis.css format check, fail-fast in CI Signed-off-by: Joe Farro <joef@uber.com> * Don't collect coverage from dev proxy setup Signed-off-by: Joe Farro <joef@uber.com> * Upgrade react-app-rewired to 2.0.1 Avoid issue yarnpkg/yarn#6300 Signed-off-by: Joe Farro <joef@uber.com> * Cleanup npm packages in packages/jaeger-ui Signed-off-by: Joe Farro <joef@uber.com>
jaegertracing · Jan 9, 2019 · 7a6190e · 7a6190e
1 parent f23eb48
commit 7a6190e
Show file tree

Hide file tree

Showing 25 changed files with 4,831 additions and 3,048 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -27,7 +27,7 @@
     "jsx-a11y/interactive-supports-focus": 0,
 
     /* react */
-    "react/jsx-curly-brace-presence": [2, 'never'],
+    "react/jsx-curly-brace-presence": ["error", "never"],
     "react/jsx-filename-extension": 0,
     "react/forbid-prop-types": 1,
     "react/require-default-props": 1,
@@ -50,14 +50,6 @@
     /* import */
     "import/prefer-default-export": 1,
     "import/no-named-default": 0,
-    "import/extensions": 0,
-
-    "new-cap": [2, {
-      "capIsNewExceptions": [
-        "Immutable.Map",
-        "Immutable.List",
-        "Immutable.Set"
-      ]
-    }]
+    "import/extensions": 0
   }
 }
diff --git a/.nvmrc b/.nvmrc
@@ -1 +1 @@
-6
+8
diff --git a/.travis.yml b/.travis.yml
@@ -1,11 +1,13 @@
 language: node_js
 node_js:
-  - '6'
+  - '8'
 cache: yarn
 env:
   global:
     # so the yarn installed in before_install will be used
     - PATH=$HOME/.yarn/bin:$PATH
+matrix:
+  fast_finish: true
 before_install:
   # update yarn - travis is using an old version
   # https://yarnpkg.com/en/docs/install#alternatives-stable

diff --git a/package.json b/package.json
@@ -2,8 +2,8 @@
   "private": true,
   "license": "Apache-2.0",
   "devDependencies": {
-    "babel-eslint": "^7.2.3",
-    "eslint": "^4.5.0",
+    "babel-eslint": "9.x.x",
+    "eslint": "5.6.0",
     "eslint-config-airbnb": "^15.1.0",
     "eslint-config-prettier": "^2.3.0",
     "eslint-config-react-app": "^2.0.0",
@@ -13,10 +13,10 @@
     "eslint-plugin-react": "^7.12.2",
     "flow-bin": "^0.71.0",
     "glow": "^1.2.2",
-    "husky": "^0.14.3",
+    "husky": "1.3.1",
     "lerna": "^2.10.2",
-    "lint-staged": "^7.0.4",
-    "prettier": "^1.10.2"
+    "prettier": "^1.10.2",
+    "rxjs-compat": "6.3.3"
   },
   "workspaces": ["packages/*"],
   "scripts": {
@@ -25,11 +25,13 @@
     "coverage": "lerna run coverage",
     "eslint": "eslint 'scripts/*.js' 'packages/*/src/**/*.js' 'packages/*/*.js'",
     "flow": "glow",
-    "lint": "yarn run eslint && yarn run prettier && yarn run flow && yarn run check-license",
-    "precommit": "lint-staged",
+    "lint": "yarn run prettier-lint && yarn run eslint && yarn run flow && yarn run check-license",
     "prepare": "lerna run --stream --sort prepublishOnly",
+    "prettier-comment": "https://github.com/yarnpkg/yarn/issues/6300",
     "prettier":
-      "prettier --write '{.,scripts}/*.{js,json,md}' 'packages/*/{src,demo/src}/**/*.{css,js,json,md}' 'packages/*/*.{css,js,json,md}'",
+      "./node_modules/prettier/bin-prettier.js --write '{.,scripts}/*.{js,json,md}' 'packages/*/{src,demo/src}/**/!(react-vis).{css,js,json,md}' 'packages/*/*.{css,js,json,md}'",
+    "prettier-lint":
+      "./node_modules/prettier/bin-prettier.js --list-different '{.,scripts}/*.{js,json,md}' 'packages/*/{src,demo/src}/**/!(react-vis).{css,js,json,md}' 'packages/*/*.{css,js,json,md}'",
     "test": "lerna run test",
     "start": "cd packages/jaeger-ui && yarn start"
   },
@@ -39,8 +41,9 @@
     "singleQuote": true,
     "trailingComma": "es5"
   },
-  "lint-staged": {
-    "*.{css,js,json}": ["yarn run lint", "yarn run test", "git add"],
-    "*.md": ["yarn run prettier", "git add"]
+  "husky": {
+    "hooks": {
+      "pre-commit": "yarn run lint && yarn run test"
+    }
   }
 }
diff --git a/packages/jaeger-ui/config-overrides.js b/packages/jaeger-ui/config-overrides.js
@@ -13,27 +13,41 @@
 // limitations under the License.
 
 /* eslint-disable import/no-extraneous-dependencies */
-
-const path = require('path');
 const fs = require('fs');
-const { injectBabelPlugin } = require('react-app-rewired');
-const rewireLess = require('react-app-rewire-less');
+const { addBabelPlugin, addLessLoader } = require('customize-cra');
 const lessToJs = require('less-vars-to-js');
-const rewireBabelLoader = require('react-app-rewire-babel-loader');
 
-const appDirectory = fs.realpathSync(process.cwd());
-const resolveApp = relativePath => path.resolve(appDirectory, relativePath);
+function useEslintRc(config) {
+  const { rules } = config.module;
+  const preRule = rules.find(rule => rule.enforce === 'pre');
+  if (!preRule) {
+    throw new Error('Unable to find estlint rule, pre');
+  }
+  const use = Array.isArray(preRule.use) ? preRule.use[0] : null;
+  if (!use) {
+    throw new Error('Unable to find estlint rule, use');
+  }
+  const isEslintRule = /node_modules\/eslint-loader\//.test(use.loader);
+  if (!isEslintRule || !use.options) {
+    throw new Error('Unable to find estlint rule, eslint loader');
+  }
+  use.options.useEslintrc = true;
+  return config;
+}
 
-// Read the less file in as string
+// Convert less vars to JS
 const loadedVarOverrides = fs.readFileSync('config-overrides-antd-vars.less', 'utf8');
-
-// Pass in file contents
 const modifyVars = lessToJs(loadedVarOverrides);
 
-module.exports = function override(_config, env) {
+function webpack(_config) {
   let config = _config;
-  config = injectBabelPlugin(['import', { libraryName: 'antd', style: true }], config);
-  config = rewireLess.withLoaderOptions({ modifyVars })(config, env);
-  config = rewireBabelLoader.include(config, resolveApp('../../node_modules/drange'));
+  config = addLessLoader({
+    modifyVars,
+    javascriptEnabled: true,
+  })(config);
+  config = addBabelPlugin(['import', { libraryName: 'antd', style: true }])(config);
+  useEslintRc(config);
   return config;
-};
+}
+
+module.exports = { webpack };
diff --git a/packages/jaeger-ui/package.json b/packages/jaeger-ui/package.json
@@ -4,28 +4,20 @@
   "version": "0.0.1",
   "main": "src/index.js",
   "license": "Apache-2.0",
-  "proxy": {
-    "/api": {
-      "target": "http://localhost:16686",
-      "logLevel": "silent",
-      "secure": false,
-      "changeOrigin": true,
-      "ws": true,
-      "xfwd": true
-    }
-  },
   "homepage": ".",
   "devDependencies": {
-    "babel-plugin-import": "^1.6.3",
+    "babel-plugin-import": "1.11.0",
     "bluebird": "^3.5.0",
+    "customize-cra": "0.2.9",
     "enzyme": "^3.2.0",
     "enzyme-adapter-react-16": "^1.1.0",
     "enzyme-to-json": "^3.3.0",
+    "http-proxy-middleware": "^0.19.1",
+    "less": "3.9.0",
+    "less-loader": "4.1.0",
     "less-vars-to-js": "^1.2.1",
-    "react-app-rewire-babel-loader": "^0.1.1",
-    "react-app-rewire-less": "^2.1.0",
-    "react-app-rewired": "^1.4.0",
-    "react-scripts": "^1.0.11",
+    "react-app-rewired": "2.0.1",
+    "react-scripts": "2.1.2",
     "react-test-renderer": "^15.6.1",
     "sinon": "^3.2.1",
     "source-map-explorer": "^1.6.0"
@@ -47,7 +39,6 @@
     "history": "^4.6.3",
     "is-promise": "^2.1.0",
     "isomorphic-fetch": "^2.2.1",
-    "jest": "^21.2.1",
     "json-markup": "^1.1.0",
     "lodash": "^4.17.4",
     "logfmt": "^1.2.0",
@@ -93,9 +84,11 @@
   "jest": {
     "collectCoverageFrom": [
       "src/**/*.js",
+      "!src/setup*.js",
       "!src/utils/DraggableManager/demo/*.js",
       "!src/utils/test/**/*.js",
       "!src/demo/**/*.js"
     ]
-  }
+  },
+  "browserslist": [">0.5%", "not dead", "not ie <= 11", "not op_mini all"]
 }
diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItemTitle.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItemTitle.js
@@ -44,6 +44,7 @@ export default class ResultItemTitle extends React.PureComponent<Props> {
   props: Props;
 
   static defaultProps = {
+    disableComparision: false,
     durationPercent: 0,
     error: undefined,
     state: fetchedState.DONE,

diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js
@@ -84,6 +84,8 @@ export const sortFormSelector = formValueSelector('traceResultsSort');
 export default class SearchResults extends React.PureComponent<SearchResultsProps> {
   props: SearchResultsProps;
 
+  static defaultProps = { skipMessage: false };
+
   toggleComparison = (traceID: string, remove: boolean) => {
     const { cohortAddTrace, cohortRemoveTrace } = this.props;
     if (remove) {

diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.js b/packages/jaeger-ui/src/components/SearchTracePage/index.js
@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+/* eslint-disable react/require-default-props */
+
 import React, { Component } from 'react';
 import { Col, Row } from 'antd';
 import PropTypes from 'prop-types';
@@ -137,6 +139,7 @@ SearchTracePageImpl.propTypes = {
   isHomepage: PropTypes.bool,
   // eslint-disable-next-line react/forbid-prop-types
   traceResults: PropTypes.array,
+  // eslint-disable-next-line react/forbid-prop-types
   diffCohort: PropTypes.array,
   cohortAddTrace: PropTypes.func,
   cohortRemoveTrace: PropTypes.func,

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/TraceGraph.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/TraceGraph.js
@@ -326,7 +326,7 @@ export default class TraceGraph extends React.PureComponent<Props, State> {
                 title="Help"
                 bordered={false}
                 extra={
-                  <a onClick={this.closeSidebar}>
+                  <a onClick={this.closeSidebar} role="button">
                     <Icon type="close" />
                   </a>
                 }

diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/AltViewOptions.js b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/AltViewOptions.js
@@ -32,7 +32,9 @@ export default function AltViewOptions(props: Props) {
   const menu = (
     <Menu>
       <Menu.Item>
-        <a onClick={onTraceGraphViewClicked}>{traceGraphView ? 'Trace Timeline' : 'Trace Graph'}</a>
+        <a onClick={onTraceGraphViewClicked} role="button">
+          {traceGraphView ? 'Trace Timeline' : 'Trace Graph'}
+        </a>
       </Menu.Item>
       <Menu.Item>
         <Link

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/ListView/index.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/ListView/index.test.js
@@ -13,7 +13,6 @@
 // limitations under the License.
 
 import React from 'react';
-import Bluebird from 'bluebird';
 import { mount, shallow } from 'enzyme';
 
 import ListView from './index';
@@ -205,14 +204,16 @@ describe('<ListView>', () => {
         expect(eventListeners.scroll).toEqual([instance._onScroll]);
       });
 
-      it('calls _positionList when the document is scrolled', async () => {
+      it('calls _positionList when the document is scrolled', done => {
         const event = new Event('scroll');
         const fn = jest.spyOn(instance, '_positionList');
         expect(instance._isScrolledOrResized).toBe(false);
         window.dispatchEvent(event);
         expect(instance._isScrolledOrResized).toBe(true);
-        await Bluebird.resolve().delay(0);
-        expect(fn).toHaveBeenCalled();
+        window.requestAnimationFrame(() => {
+          expect(fn).toHaveBeenCalled();
+          done();
+        });
       });
 
       it('uses the root HTML element to determine if the view has changed', () => {

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js
@@ -39,6 +39,10 @@ export const LinkValue = (props: { href: string, title?: string, children: React
   </a>
 );
 
+LinkValue.defaultProps = {
+  title: '',
+};
+
 const linkValueList = (links: Link[]) => (
   <Menu>
     {links.map(({ text, url }, index) => (

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanTreeOffset.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanTreeOffset.test.js
@@ -101,7 +101,7 @@ describe('SpanTreeOffset', () => {
 
     it('does not call props.addHoverIndentGuideId on mouse enter if mouse came from a indentGuide with the same ancestorId', () => {
       const relatedTarget = document.createElement('span');
-      relatedTarget.dataset = { ancestorId: parentSpanID };
+      relatedTarget.dataset.ancestorId = parentSpanID;
       wrapper.find({ 'data-ancestor-id': parentSpanID }).simulate('mouseenter', {
         relatedTarget,
       });
@@ -116,7 +116,7 @@ describe('SpanTreeOffset', () => {
 
     it('does not call props.removeHoverIndentGuideId on mouse leave if mouse leaves to a indentGuide with the same ancestorId', () => {
       const relatedTarget = document.createElement('span');
-      relatedTarget.dataset = { ancestorId: parentSpanID };
+      relatedTarget.dataset.ancestorId = parentSpanID;
       wrapper.find({ 'data-ancestor-id': parentSpanID }).simulate('mouseleave', {
         relatedTarget,
       });

diff --git a/packages/jaeger-ui/src/model/search.js b/packages/jaeger-ui/src/model/search.js
@@ -32,6 +32,7 @@ const comparators = {
  * @param  {Trace[]} traces The `Trace` array to sort.
  * @param  {string} sortBy A sort specification, see ./order-by.js.
  */
+// eslint-disable-next-line import/prefer-default-export
 export function sortTraces(traces: Trace[], sortBy: string) {
   const comparator = comparators[sortBy] || comparators[LONGEST_FIRST];
   traces.sort(comparator);

diff --git a/packages/jaeger-ui/src/model/span.js b/packages/jaeger-ui/src/model/span.js
@@ -21,6 +21,7 @@ import type { Span } from '../types/trace';
  * @param  {Span} span The span whose parent is to be returned.
  * @return {Span|null} The parent span if there is one, null otherwise.
  */
+// eslint-disable-next-line import/prefer-default-export
 export function getParent(span: Span) {
   const parentRef = span.references ? span.references.find(ref => ref.refType === 'CHILD_OF') : null;
   return parentRef ? parentRef.span : null;

diff --git a/packages/jaeger-ui/src/model/trace-dag/DenseTrace.js b/packages/jaeger-ui/src/model/trace-dag/DenseTrace.js
@@ -37,6 +37,7 @@ function convSpans(spans: Span[]) {
     if (references && references.length) {
       const { refType, spanID } = references[0];
       if (refType !== 'CHILD_OF' && refType !== 'FOLLOWS_FROM') {
+        // eslint-disable-next-line no-console
         console.warn(`Unrecognized ref type: ${refType}`);
       } else {
         parentID = spanID;

diff --git a/packages/jaeger-ui/src/model/trace-dag/TraceDag.js b/packages/jaeger-ui/src/model/trace-dag/TraceDag.js
@@ -98,6 +98,7 @@ export default class TraceDag<T = void> {
   _addDenseSpan(spanID: string, parentNodeID?: ?NodeID, data: T) {
     const denseSpan = this.denseTrace && this.denseTrace.denseSpansMap.get(spanID);
     if (!denseSpan) {
+      // eslint-disable-next-line no-console
       console.warn(`Missing dense span: ${spanID}`);
       return;
     }

diff --git a/packages/jaeger-ui/src/setupProxy.js b/packages/jaeger-ui/src/setupProxy.js
@@ -0,0 +1,29 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// eslint-disable-next-line import/no-extraneous-dependencies
+const proxy = require('http-proxy-middleware');
+
+module.exports = function setupProxy(app) {
+  app.use(
+    proxy('/api', {
+      target: 'http://localhost:16686',
+      logLevel: 'silent',
+      secure: false,
+      changeOrigin: true,
+      ws: true,
+      xfwd: true,
+    })
+  );
+};
diff --git a/packages/jaeger-ui/src/utils/number.test.js b/packages/jaeger-ui/src/utils/number.test.js
@@ -27,9 +27,10 @@ it('toFloatPrecision() should work for greater-than-0 numbers', () => {
 
 it('toFloatPrecision() should work for less-than-0 numbers', () => {
   expect(numberUtils.toFloatPrecision(0.24, 1)).toBe(0.2);
-  expect(numberUtils.toFloatPrecision(-0.026, 1)).toBe(0);
   expect(numberUtils.toFloatPrecision(0.51, 1)).toBe(0.5);
   expect(numberUtils.toFloatPrecision(-0.307, 2)).toBe(-0.31);
+  // Had an issue with expect(-0).toBe(0) failing
+  expect(numberUtils.toFloatPrecision(-0.026, 1)).toBeCloseTo(0);
 });
 
 it('toFloatPrecision() should work for e-notation numbers', () => {