Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

review dependencies #3771

Closed
clouserw opened this issue Aug 3, 2018 · 10 comments
Closed

review dependencies #3771

clouserw opened this issue Aug 3, 2018 · 10 comments
Labels
size:L < 3 days

Comments

@clouserw
Copy link
Member

clouserw commented Aug 3, 2018

We have a lot of them! https://github.com/mozilla/testpilot/network/dependencies

@clouserw clouserw changed the title review depedencies review dependencies Aug 3, 2018
@pdehaan
Copy link
Contributor

pdehaan commented Aug 3, 2018

Here's the always mildly-suspicious output from depcheck:

➜  testpilot git:(master) $ git rev-parse --short HEAD # a8682f5b

➜  testpilot git:(master) $ npx depcheck@latest

Unused dependencies
* domready
* legal-docs

Unused devDependencies
* autoprefixer
* babel-eslint
* babel-register
* cross-spawn
* cssnano
* empty
* eslint-config-react
* eslint-plugin-jsx-a11y
* flow-typed
* gulp-cache
* gulp-if
* gulp-multi-dest
* gulp-rename
* gulp-uglify
* merge-stream
* mocha-junit-reporter
* node-normalize-scss
* photon-colors
* react-addons-test-utils
* react-test-renderer
* require-globify
* url-loader
* vinyl-source-stream

Missing dependencies
* eslint-config-plugin:import
* eslint-config-plugin:flowtype
* eslint-config-plugin:react
* eslint-config-plugin:mozilla
* @storybook/addon-links
* globby
* l20n
* redux-actions
* isomorphic-fetch
* react-router-redux
* history
* redux-logger
* redux-thunk
* fbjs
* object-assign
* _process
* select
* lodash.isplainobject
* delegate
* warning
* invariant
* deep-equal
* query-string
* lodash._basefor
* lodash.isarguments
* lodash.keysin
* lodash.isarray
* strict-uri-encode
* json-stringify-safe
* lodash
* hoist-non-react-statics
* reduce-reducers
* deep-diff
* symbol-observable
* tiny-emitter
* good-listener
* flux-standard-action

@meandavejustice
Copy link
Contributor

Here is output for npm outdated:

meandave@LAPTOP-NA0Q0G86:/mnt/c/Users/Dave Justice/Code/testpilot$ npm outdated
Package                             Current         Wanted   Latest  Location
@storybook/addon-actions      3.4.0-alpha.1  3.4.0-alpha.1   3.4.10  testpilot
@storybook/addon-knobs        3.4.0-alpha.1  3.4.0-alpha.1   3.4.10  testpilot
@storybook/react              3.4.0-alpha.1  3.4.0-alpha.1   3.4.10  testpilot
autoprefixer                          8.1.0          8.6.5    9.1.0  testpilot
babel-core                           6.26.0         6.26.0   6.26.3  testpilot
babel-eslint                          8.2.1          8.2.1    8.2.6  testpilot
babel-loader                          7.1.2          7.1.2    7.1.5  testpilot
babel-plugin-istanbul                 4.1.5          4.1.5    4.1.6  testpilot
classnames                            2.2.5          2.2.5    2.2.6  testpilot
cldr-core                            32.0.0         32.0.0   33.0.0  testpilot
clipboard                             1.7.1          1.7.1    2.0.1  testpilot
copy-webpack-plugin                   4.5.1          4.5.2    4.5.2  testpilot
cross-env                             5.1.3          5.1.3    5.2.0  testpilot
cross-spawn                           5.1.0          5.1.0    6.0.5  testpilot
cssnano                              3.10.0         3.10.0    4.0.5  testpilot
doctoc                                1.3.0          1.3.0    1.3.1  testpilot
es6-promise                           4.2.2          4.2.2    4.2.4  testpilot
eslint                               4.15.0         4.15.0    5.3.0  testpilot
eslint-plugin-flowtype               2.30.0         2.30.0   2.50.0  testpilot
eslint-plugin-import                  2.8.0          2.8.0   2.13.0  testpilot
eslint-plugin-jsx-a11y                6.0.3          6.0.3    6.1.1  testpilot
eslint-plugin-mozilla                 0.5.0          0.5.0   0.15.0  testpilot
eslint-plugin-no-unsanitized          2.0.2          2.0.2    3.0.2  testpilot
eslint-plugin-react                   7.5.1          7.5.1   7.10.0  testpilot
express                              4.16.2         4.16.3   4.16.3  testpilot
feed                                  1.1.1          1.1.1    2.0.1  testpilot
fetch-mock                     6.0.0-beta.7   6.0.0-beta.7    6.5.2  testpilot
flow-bin                             0.47.0         0.47.0   0.78.0  testpilot
flow-coverage-report                  0.4.0          0.4.0    0.5.0  testpilot
flow-typed                            2.2.3          2.2.3    2.5.1  testpilot
fluent                                0.4.3          0.4.3    0.7.0  testpilot
fluent-react                          0.4.1          0.4.1    0.7.0  testpilot
gulp-multi-dest                       0.0.4          0.0.4    1.3.7  testpilot
gulp-rename                           1.2.2          1.2.2    1.4.0  testpilot
gulp-rev-all                          0.9.7          0.9.7    1.0.0  testpilot
gulp-uglify                           3.0.0          3.0.0    3.0.1  testpilot
html-react-parser                     0.4.1          0.4.1    0.4.6  testpilot
image-webpack-loader                  3.4.2          3.4.2    4.3.1  testpilot
jsdom                                11.5.1         11.5.1  11.12.0  testpilot
legal-docs                            1.0.0            git      git  testpilot
libphonenumber-js                    1.2.15          1.4.2    1.4.2  testpilot
mocha                                 4.1.0          4.1.0    5.2.0  testpilot
mocha-junit-reporter                 1.15.0         1.15.0   1.17.0  testpilot
moment                               2.20.1         2.20.1   2.22.2  testpilot
node-normalize-scss                   3.0.0          3.0.0    8.0.0  testpilot
node-sass                             4.7.2          4.7.2    4.9.2  testpilot
npm-run-all                           4.1.2          4.1.3    4.1.3  testpilot
nyc                                  11.4.1         11.4.1   12.0.2  testpilot
onchange                              3.3.0          3.3.0    4.1.0  testpilot
photon-colors                         1.3.1          1.3.1    3.3.1  testpilot
postcss-loader                        2.1.1          2.1.6    2.1.6  testpilot
raven-js                             3.21.0         3.21.0   3.26.4  testpilot
react                                16.2.0         16.2.0   16.4.2  testpilot
react-dom                            16.2.0         16.2.0   16.4.2  testpilot
react-markdown                        3.1.4          3.1.4    3.4.1  testpilot
react-redux                           5.0.6          5.0.6    5.0.7  testpilot
react-router                          4.2.0          4.3.1    4.3.1  testpilot
react-router-dom                      4.2.2          4.3.1    4.3.1  testpilot
react-test-renderer                  16.2.0         16.2.0   16.4.2  testpilot
redux                                 3.7.2          3.7.2    4.0.0  testpilot
redux-promise                         0.5.3          0.5.3    0.6.0  testpilot
sass-loader                           6.0.6          6.0.6    7.1.0  testpilot
sinon                                 4.1.4          4.1.4    6.1.4  testpilot
uglifyjs-webpack-plugin               1.1.6          1.1.6    1.2.7  testpilot
url-loader                            0.6.2          0.6.2    1.0.1  testpilot
webpack                              3.10.0         3.10.0   4.16.5  testpilot
webpack-bundle-analyzer               2.9.2          2.9.2   2.13.1  testpilot
webpack-dev-server                   2.10.1         2.11.2    3.1.5  testpilot
whatwg-fetch                          2.0.3          2.0.3    2.0.4  testpilot
write-file-webpack-plugin             4.2.0          4.3.2    4.3.2  testpilot

11 of our deps are in the red

Addon

All of our addon dependencies are needed at the moment. All of them are dev dependencies so we don't have the same level of security concern for these.

Package               Current  Wanted  Latest  Location
cross-env               5.0.1   5.0.1   5.2.0  testpilot-addon
eslint                 4.15.0  4.15.0   5.3.0  testpilot-addon
eslint-plugin-import    2.8.0   2.8.0  2.13.0  testpilot-addon
fs-extra                6.0.0   6.0.1   7.0.0  testpilot-addon
onchange                4.0.0   4.1.0   4.1.0  testpilot-addon
shx                     0.2.2   0.2.2   0.3.2  testpilot-addon
web-ext                 2.6.0   2.8.0   2.8.0  testpilot-addon

3 are in the red

@clouserw clouserw added this to the Things we should do 🇬🇫 milestone Aug 6, 2018
@clouserw clouserw added size:S < 4 hours size:L < 3 days and removed size:L < 3 days size:S < 4 hours labels Aug 6, 2018
@meandavejustice
Copy link
Contributor

Been working on this for a few days now,

  • Everything from depcheck has been addressed, no more unused or missing deps
  • Flow upgrade exposed a lot of Flow errors (around 250) those are now down to 29 as of this morning. PR should be open shortly after I finish fixing these
  • Everything is updated except package related to updating fluent (Update to a newer version of Fluent #3789) and webpack. Webpack upgrade is complex with larger sweeping changes in our tasks, it should probably be done as a part of finishing up Move everything to webpack, remove gulp #3111.
  • eslint issues addressed
  • new flow-typed definitions added/updated where necessary
  • Random minor compatibility issues addressed from package upgrades
  • removed some dead code in some components found with new flow version
  • Found an metric that was being misreported with flow, fixed and updated ga.md docs to reflect changes

@stasm
Copy link

stasm commented Aug 17, 2018

I noticed that Test Pilot uses both uglifyjs-webpack-plugin and gulp-uglify. Are both required?

This might be relevant to the Fluent upgrade in #3789. Fluent's compat builds use some ES6+ features, which old versions of Uglify don't support. From what I was able to gather, there are three Uglify packages on npm:

  • uglify-js is the master branch and it only supports ES5.
  • uglify-es is built from the harmony branch of the same repo as uglify-js and it supports some ES6+. Fluent compat files minify fine with it. Unfortunately, uglify-es is not maintained any more.
  • terser is a fork of uglify-es and is actively maintained. Fluent's compat builds minify fine using terser.

While uglifyjs-webpack-plugin depends on uglify-es, gulp-uglify still uses the old uglify-js package. It looks like this is the main blocker right now to upgrading Fluent in Test Pilot. Two questions follow:

  1. Assuming they serve roughly the same purpose, can gulp-uglify be removed in favor of uglify-webpack-plugin?
  2. If not, would you consider replacing gulp-uglify with gulp-uglify-es which depends on terser? I haven't checked if it solves the Fluent upgrade problems, but I can try if you're open to it.

PS Always bet on JavaScript.

@lmorchard
Copy link
Contributor

I noticed that Test Pilot uses both uglifyjs-webpack-plugin and gulp-uglify. Are both required?

We're 5/7 of the way through removing Gulp from our build chain (it's been a slow process) - but we don't actually use gulp-uglify any more as far as I can tell. So, it's just uglifyjs-webpack-plugin. We can upgrade that, as long as it doesn't cause an upgrade cascade that we've been trying to avoid for the time being.

Also I think the larger issue is not just whether uglify fails - but whether we want to switch from transpiled ES5 to serving up straight ES6 code, right? I'm not sure we want to do that yet. Maybe it's okay with respect to our browser support goals? I don't know off the top of my head.

@stasm
Copy link

stasm commented Aug 20, 2018

we don't actually use gulp-uglify any more as far as I can tell. So, it's just uglifyjs-webpack-plugin.

The version of uglifyjs-webpack-plugin that you currently use depends on uglify-es. I can't say it's enough for all ES6 code out there, but I've verified that it works well with the recent releases of fluent and fluent-react.

Also I think the larger issue is not just whether uglify fails - but whether we want to switch from transpiled ES5 to serving up straight ES6 code, right? I'm not sure we want to do that yet. Maybe it's okay with respect to our browser support goals? I don't know off the top of my head.

Looking at Test Pilot's FAQ, I sense some enthusiasm about shipping ES6+ code :) FWIW new releases of Fluent packages are currently targeting the following browsers with its transpiled compat builds:

Firefox >= 52,
FirefoxAndroid >= 52,
Chrome >= 55,
ChromeAndroid >= 55,
Edge >= 15,
Safari >= 10.1,
iOS >= 10.3

fzzzy added a commit that referenced this issue Aug 22, 2018
This commit was authored by meandavejustice, but there was some git weirdness, so I am helping out by squashing it into a new commit on top of master.
@connorshea
Copy link

Have you considered using something like dependabot or greenkeeper to keep your npm dependencies up-to-date? I use dependabot and it works quite well for my projects :)

@lmorchard
Copy link
Contributor

We were using greenkeeper. But, we had a long run of it suggesting backwards-incompatible upgrades. The pull requests kind of piled up as tech debt chasing upstream API changes at a time when we were busy doing other things as a team (e.g. Test Pilot experiments). We disabled greenkeeper with the intent to get back to it (or something like it), but haven't quite done it yet.

@clouserw
Copy link
Member Author

I tried to enable renovate today, but couldn't figure out how. Not sure if that says more about me or GitHub's app system.... :-/

@clouserw
Copy link
Member Author

clouserw commented Sep 6, 2018

Alright renovate is enabled! For the record:

The Mozilla org had already approved Renovate for use in the org, however, a repository owner cannot turn it on. You have to be an org owner to add it to the repository... which is weird. Anyway...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size:L < 3 days
Projects
None yet
Development

No branches or pull requests

6 participants