Upgrade build tools to latest versions #1350

malonecj · 2018-10-30T11:25:41Z

Description

Build tools upgrade

Upgrade to Webpack 4
Upgrade to Babel 7
- Auto upgrade using Babel upgrade tool
- Added browserlist file to specify targetted browsers
- Replace babelrc file with babel.config.js file to work with monorepo structure
- Replace es2015 and stage2 presets with preset-env
Upgrade to Karma 3
Remove rewire from tests after upgrading to Babel 7
Fix tests

amanmahajan7

I did not do an extensive review but looks good in general. A few comments and questions

amanmahajan7 · 2018-11-06T03:35:47Z

packages/react-data-grid-addons/src/editors/__tests__/AutoCompleteEditor.spec.js

        expect(value).toBe('autocomplete');
      });
    });
-
-    describe('With different props', () => {


Do these tests add any value or can they be safely deleted?

I think can be deleted

amanmahajan7 · 2018-11-06T03:43:58Z

babel.config.js

+  plugins: [
+    '@babel/plugin-proposal-class-properties',
+    '@babel/plugin-transform-property-literals',
+    '@babel/plugin-transform-member-expression-literals'


Do we need @babel/plugin-transform-runtime?

Added that in now

amanmahajan7 · 2018-11-06T03:47:01Z

config/karma.js

+            ]
+          },
+          {
+            test: /\.css$/,


Should the CSS file be extracted using https://github.com/webpack-contrib/mini-css-extract-plugin?

amanmahajan7 · 2018-11-06T03:49:25Z

browserslist

@@ -0,0 +1,6 @@
+ie 11


Does the examples page load fine on IE11 and do the polyfills load correctly?

amanmahajan7 · 2018-11-06T03:51:08Z

packages/react-data-grid-addons/src/editors/__tests__/ContainerEditorWrapper.spec.js

@@ -23,45 +23,12 @@ describe('ContainerEditorWrapper', () => {
    it('should create a new ContainerEditorWrapper instance wrapping the passed in component', () => {
      // ACT
      let ConnectedContainerEditorWrapper = ContainerEditorWrapper(FakeContainer);
-      const renderedComp = Enzyme.mount(<ConnectedContainerEditorWrapper />);
+      const renderedComp = mount(<ConnectedContainerEditorWrapper />);

      // ASSERT
      expect(renderedComp).toBeDefined();


expect(renderedComp).toBeDefined(); can deleted as it will always be true

amanmahajan7 · 2018-11-06T03:54:07Z

packages/react-data-grid/src/__tests__/ColumnMetrics.spec.js

@@ -1,6 +1,5 @@
 import getScrollbarSize from '../getScrollbarSize';
-const rewire = require('rewire');
-const ColumnMetrics = rewire('../ColumnMetrics');
+const ColumnMetrics = require('../ColumnMetrics');
 const Immutable = window.Immutable = require('immutable');


I think const Immutable = window.Immutable = require('immutable'); can be deleted because of this change.

amanmahajan7 · 2018-11-06T03:58:45Z

packages/react-data-grid/src/masks/InteractionMasks.js

@@ -352,7 +352,7 @@ class InteractionMasks extends React.Component {
    const next = this.getNextSelectedCellPositionForKeyNavAction(keyNavAction, currentPosition, cellNavigationMode);
    this.checkIsAtGridBoundary(keyNavAction, next);

-    const { changeRowOrColumn, ...nextPos } = next;
+    const { ...nextPos } = next;


Is this needed? If this needs to be cloned then this.selectCell({ ...next}) be used directly

amanmahajan7 · 2018-11-06T04:01:21Z

packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js

-          expect(selectedRange.bottomRight).toEqual({ idx: 3, rowIdx: 2 });
-          expect(selectedRange.cursorCell).toEqual({ idx: 3, rowIdx: 2 });
-          expect(selectedRange.startCell).toEqual({ idx: 2, rowIdx: 2 });
+          expect(selectedRange.topLeft).toEqual(objectMatching({ idx: 2, rowIdx: 2 }));


Is the object being tested changed? Just figuring out the need for objectMatching

It now has a changeRowOrColumn property that it is expecting. Not sure why the tests didnt fail before. Since I'm only interested in the cell position I used objectContaining

amanmahajan7 · 2018-11-06T04:01:43Z

packages/react-data-grid/src/masks/InteractionMasks.js

@@ -363,7 +363,7 @@ class InteractionMasks extends React.Component {
    const next = this.getNextSelectedCellPositionForKeyNavAction(keyNavAction, currentPosition, cellNavigationMode);
    this.checkIsAtGridBoundary(keyNavAction, next);

-    const { changeRowOrColumn, ...nextPos } = next;
+    const { ...nextPos } = next;


amanmahajan7 · 2018-11-06T04:03:07Z

website/yarn.lock

@@ -0,0 +1,10202 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.


Does this file need to be checked it?

No. Committed by mistake. Have removed now

amanmahajan7

Looks good 👍 Just needs catchup. Once this is merged we can look into migrating to ES modules so we can set modules: false so webpack can treeshake. One more thing I noticed, when debug is set to true in babel env, I get the following warning message

`import '@babel/polyfill'` was not found.

Not sure if it needs to be imported or not but we can work on it in a separate PR

amanmahajan7 · 2018-11-15T04:09:34Z

babel.config.js

+module.exports = {
+  presets: [
+    ['@babel/env', {
+      useBuiltIns: 'entry'}],


The default behavior of modules property is auto. I did not find a good explanation but seems like auto defaults to false https://github.com/babel/website/pull/1856/files

I think in our case it defaults to CJS as we still have commonjs code along with ES code. When it is set to false then webpack complains so we are good it seems but something to be aware of

amanmahajan7 · 2018-11-15T13:55:28Z

config/webpack.config.prod.js

-    'react-data-grid/dist/react-data-grid.min': ['./packages/react-data-grid/src'],
-    'react-data-grid-addons/dist/react-data-grid-addons.min': ['./packages/react-data-grid-addons/src'],
-    'react-data-grid-examples/dist/index': './packages/react-data-grid-examples/src'
+    'react-data-grid/dist/react-data-grid': [path.resolve('packages/react-data-grid/src')],


It seems like both react-data-grid and react-data-grid.min.js are minified.

* Babel 7 upgrade * Fix lint error * fix lint error * Upgrade to Webpack 4 * Fix tests after Webpack and karma upgrade * Fix tests failing in production mode * import local * Fix failing IE tests * Fix lint errors * Add babel polyfill to tests * Add babel ployfill to dependencies * corejs dependency * Adress PR comments * Fix Data.Selectors to export correct module name * remove package.json * Commit package0lock.json

malonecj added 5 commits October 30, 2018 11:22

Babel 7 upgrade

6af36dc

Fix lint error

23430d6

fix lint error

650ae4f

Upgrade to Webpack 4

a25eaef

Fix tests after Webpack and karma upgrade

cc4b7d2

malonecj changed the title ~~Babel 7 upgrade~~ Upgrade build tools to latest versions Oct 31, 2018

amanmahajan7 added the WIP label Oct 31, 2018

malonecj added 8 commits November 2, 2018 14:45

Fix tests failing in production mode

b1dedfb

Catchup with master

c1c29da

import local

3e99bed

Fix failing IE tests

6307f3f

Fix lint errors

1be7ffb

Add babel polyfill to tests

7003882

Add babel ployfill to dependencies

cb8eba0

corejs dependency

ece42ba

amanmahajan7 requested review from nstepien and amanmahajan7 November 2, 2018 17:57

Catchup with master

6e8bac2

malonecj removed the WIP label Nov 5, 2018

amanmahajan7 requested changes Nov 6, 2018

View reviewed changes

malonecj changed the base branch from master to next November 13, 2018 10:45

malonecj added 2 commits November 13, 2018 11:58

Adress PR comments

9409bbd

Fix Data.Selectors to export correct module name

d4f6bb2

malonecj changed the base branch from next to master November 14, 2018 14:47

malonecj force-pushed the cm-babel-7 branch from d4c6ce1 to d4f6bb2 Compare November 15, 2018 11:59

remove package.json

5f15a61

amanmahajan7 approved these changes Nov 15, 2018

View reviewed changes

malonecj added 2 commits November 15, 2018 14:28

Catch up with master

ff97207

Commit package0lock.json

767abc7

amanmahajan7 merged commit d9a4cb9 into master Nov 15, 2018

amanmahajan7 deleted the cm-babel-7 branch November 15, 2018 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade build tools to latest versions #1350

Upgrade build tools to latest versions #1350

malonecj commented Oct 30, 2018 •

edited

Loading

amanmahajan7 left a comment

amanmahajan7 Nov 6, 2018

malonecj Nov 13, 2018

amanmahajan7 Nov 6, 2018

malonecj Nov 13, 2018

amanmahajan7 Nov 6, 2018

amanmahajan7 Nov 6, 2018

malonecj Nov 14, 2018

amanmahajan7 Nov 6, 2018 •

edited

Loading

amanmahajan7 Nov 6, 2018

amanmahajan7 Nov 6, 2018

amanmahajan7 Nov 6, 2018

malonecj Nov 13, 2018

amanmahajan7 Nov 6, 2018

amanmahajan7 Nov 6, 2018

malonecj Nov 13, 2018

amanmahajan7 left a comment •

edited

Loading

amanmahajan7 Nov 15, 2018

amanmahajan7 Nov 15, 2018

		@@ -0,0 +1,10202 @@
		# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

Upgrade build tools to latest versions #1350

Upgrade build tools to latest versions #1350

Conversation

malonecj commented Oct 30, 2018 • edited Loading

Description

Build tools upgrade

amanmahajan7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanmahajan7 Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanmahajan7 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malonecj commented Oct 30, 2018 •

edited

Loading

amanmahajan7 Nov 6, 2018 •

edited

Loading

amanmahajan7 left a comment •

edited

Loading