Update dependencies and cleanup repo #30

cody-dot-js · 2018-12-01T02:42:21Z

Summary

Fixes #29

Update dependencies
- Update babelrc
- Update eslint config and ignore
- Use .nvmrc lts/carbon (lts node v8.x)
Use yarn
Cleanup README
Fix broken tests from dependency bumps
Lint/cleanup code

Additional Info

Comparison of output: Webpack v2 vs Webpack v4 + Terser

cc: @cerner/xfc @nickpith @mjhenkes @ikottman

… update tests

emilyrohrbough · 2019-02-13T17:04:47Z

webpack.config.js

@@ -26,4 +25,18 @@ module.exports = {
    ],
  },
  devtool: 'source-map',
+  optimization: {


By default, the TerserPlugin in used by webpack 4 for optimization. This webpack configuration is pretty simple. Is there a reason we can't use the default?

This is just copy-pasted from terra-toolkit: https://github.com/cerner/terra-toolkit/blob/master/config/webpack/webpack.config.js#L125-L138

emilyrohrbough · 2019-02-13T17:06:16Z

webpack.config.js

@@ -1,6 +1,5 @@
 const path = require('path');
-const webpack = require('webpack');
-const pkg = require('./package.json');
+const TerserPlugin = require('terser-webpack-plugin');



With upgrading to webpack v4, we need to add the mode key to this configuration https://webpack.js.org/concepts/mode/

emilyrohrbough · 2019-02-13T17:07:32Z

webpack.config.js

@@ -26,4 +25,18 @@ module.exports = {
    ],
  },
  devtool: 'source-map',


We should not be using source-map for production mode.

@nickpith @alex-bezek @mhemesath I believe the source maps are still wanted for prod?

yes? don't you always want source maps in production mode because thats when its minified and everything and we wantt to be able to debug things?
https://webpack.js.org/configuration/devtool/#devtool

It does have yes for production.
It does say this FYI

emilyrohrbough · 2019-02-13T17:10:43Z

webpack.config.dev.js

@@ -1,6 +1,4 @@
 const path = require('path');
-const webpack = require('webpack');
-const pkg = require('./package.json');


Lets remove disableHostCheck: true, from the devServer options. This is not recommended configuration

Also, the only different between this config and the non-dev configuration should be the mode, output.path, devServer and devTool config. IMO we should combine these files.

emilyrohrbough · 2019-02-13T17:12:30Z

package.json

+    "docs": "jsdoc  --package package.json -d doc ./src/**/*.js",
+    "lint": "eslint './src/**/*.js'",
+    "prepublish": "yarn run build",
+    "prebuild": "yarn run clean && yarn run lint && yarn run test",


Lets keep the npm references and npm-package.lock file

Why have you changed / remove scripts that haven't been replaced? I might be missing them.

emilyrohrbough · 2019-02-13T17:12:38Z

README.md

-npm run dev
+
+```bash
+yarn install


Lets keep the npm references

emilyrohrbough · 2019-02-13T17:12:56Z

NOTICE

@@ -1,4 +1,4 @@
-Copyright 2017 Cerner Innovation, Inc.
+Copyright 2018 Cerner Innovation, Inc.


this needs to be 2017-2019

emilyrohrbough · 2019-02-13T17:15:08Z

.babelrc

+      "env",
+      {
+        "targets": {
+          "browsers": [ "last 2 versions", "iOS >= 10", "ie >= 10" ]


Not sure on this list, but this might need to align with the terra-supported browserlist. Likely an issue for another PR. https://github.com/cerner/browserslist-config-terra

emilyrohrbough · 2019-02-13T17:15:25Z

.babelrc

    "transform-runtime",
+    "transform-object-rest-spread",
+    "transform-class-properties"


Why are we adding this plugin?

emilyrohrbough · 2019-02-13T17:16:54Z

package.json

+    "babel-preset-env": "^1.7.0",
+    "babel-preset-react": "^6.24.1",
+    "chai": "^4.2.0",
+    "eslint": "^5.9.0",


I would prefer if we aligned with our sharable eslint configuration: https://github.com/cerner/eslint-config-terra/blob/master/package.json

emilyrohrbough · 2019-02-13T17:20:04Z

package.json

@@ -10,42 +10,52 @@
    "src"
  ],
  "scripts": {


Lets add the following since you've opened up the node version

"engines": { "node": ">=8.9.2" },

emilyrohrbough · 2019-02-13T17:23:21Z

package.json

-    "babel-loader": "^6.2.4",
+    "babel-cli": "^6.26.0",
+    "babel-core": "^6.26.3",
+    "babel-eslint": "10.0.1",


Why is this locked down?

emilyrohrbough · 2019-02-13T17:24:23Z

src/consumer/frame.js

          return Promise.resolve();
        },

        authorized(detail = {}) {
          self.wrapper.setAttribute('data-status', 'authorized');
-          self.emit('xfc.authorized', detail);
+          self.emit(events.authorized, detail);


Can you explain why you've changed this? How have you tested this?

This is just essentially an enum

mhemesath · 2019-02-25T19:03:00Z

package.json

-    "babel-cli": "^6.7.5",
-    "babel-core": "^6.7.7",
-    "babel-loader": "^6.2.4",
+    "babel-cli": "^6.26.0",


if we're upgrading dependencies.. should just update to babel 7

mjhenkes · 2019-05-30T12:04:07Z

Is this PR still active? I'd like to see xfc upgraded to babel 7, namely the babel-runtime dependency. Which will probably require a major version bump.

alex-bezek · 2019-05-31T00:37:13Z

Is this PR still active? I'd like to see xfc upgraded to babel 7, namely the babel-runtime dependency. Which will probably require a major version bump.

It seems this is very old. It also has lots of changes all grouped together. IMO if you want, update just the babel stuff without all the lint fixes and do a couple of small changes. I'll leave it up to @dev-cprice to say if he is done with this or not, but I would err on making a new PR with just the babel updates and he can always merge those changes into his branch

cody-dot-js · 2020-04-22T14:59:33Z

Closing this since it's wayyyy old and I don't know if I'll ever have time to get around to it anytime soon.

Cody Price added 4 commits November 30, 2018 20:12

Update dependencies, use yarn and node lts/carbon, lint & clean code,…

c37ae37

… update tests

Add self to CONTRIBUTORS

eedb428

Update travis config

7ca5762

Cleanup code, fix babel config

0e7ea81

cody-dot-js force-pushed the cleanup-repo-bump-deps branch from a8b8c17 to 0e7ea81 Compare December 7, 2018 22:58

Cody Price added 2 commits December 7, 2018 17:36

Fix eslint config, update webpack to use terser

56e8148

Add CODEOWNERS: default * for @cerner/xfc

0b384af

emilyrohrbough reviewed Feb 13, 2019

View reviewed changes

README.md

npm run dev

```bash

yarn install

Copy link

emilyrohrbough Feb 13, 2019

Choose a reason for hiding this comment

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

Lets keep the npm references

emilyrohrbough reviewed Feb 13, 2019

View reviewed changes

NOTICE

@@ -1,4 +1,4 @@

Copyright 2017 Cerner Innovation, Inc.

Copyright 2018 Cerner Innovation, Inc.

Copy link

emilyrohrbough Feb 13, 2019

Choose a reason for hiding this comment

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

this needs to be 2017-2019

emilyrohrbough reviewed Feb 13, 2019

View reviewed changes

.babelrc

"transform-runtime",

"transform-object-rest-spread",

"transform-class-properties"

Copy link

emilyrohrbough Feb 13, 2019

Choose a reason for hiding this comment

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

Why are we adding this plugin?

emilyrohrbough reviewed Feb 13, 2019

View reviewed changes

mhemesath approved these changes Feb 22, 2019

View reviewed changes

mhemesath reviewed Feb 25, 2019

View reviewed changes

cody-dot-js closed this Apr 22, 2020

cody-dot-js deleted the cleanup-repo-bump-deps branch April 22, 2020 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies and cleanup repo #30

Update dependencies and cleanup repo #30

cody-dot-js commented Dec 1, 2018 •

edited

Loading

emilyrohrbough Feb 13, 2019

cody-dot-js Feb 28, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

cody-dot-js Feb 28, 2019

alex-bezek May 31, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

emilyrohrbough Feb 13, 2019

cody-dot-js Feb 28, 2019

mhemesath Feb 25, 2019

mjhenkes commented May 30, 2019

alex-bezek commented May 31, 2019

cody-dot-js commented Apr 22, 2020

		@@ -1,4 +1,4 @@
		Copyright 2017 Cerner Innovation, Inc.
		Copyright 2018 Cerner Innovation, Inc.

Update dependencies and cleanup repo #30

Update dependencies and cleanup repo #30

Conversation

cody-dot-js commented Dec 1, 2018 • edited Loading

Summary

Additional Info

Comparison of output: Webpack v2 vs Webpack v4 + Terser

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

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

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

mjhenkes commented May 30, 2019

alex-bezek commented May 31, 2019

cody-dot-js commented Apr 22, 2020

cody-dot-js commented Dec 1, 2018 •

edited

Loading