Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies #557

Merged
merged 16 commits into from
Jul 19, 2018
Merged

Update dependencies #557

merged 16 commits into from
Jul 19, 2018

Conversation

36degrees
Copy link
Contributor

Updates all dependencies reported as outdated by running npm outdated:

$ npm outdated
Package                    Current  Wanted  Latest  Location
basic-auth                   1.1.0   1.1.0   2.0.0  express-prototype
cross-spawn                  5.1.0   5.1.0   6.0.5  express-prototype
dotenv                       4.0.0   4.0.0   6.0.0  express-prototype
express                     4.15.2  4.15.2  4.16.3  express-prototype
gulp-clean                   0.3.2   0.3.2   0.4.0  express-prototype
gulp-mocha                   4.3.1   4.3.1   6.0.0  express-prototype
gulp-sass                    3.1.0   3.1.0   4.0.1  express-prototype
marked                      0.3.19  0.3.19   0.4.0  express-prototype
notifications-node-client    3.5.0   3.5.0   4.1.0  express-prototype
readdir                     0.0.13  0.0.13   0.1.0  express-prototype
require-dir                  0.3.2   0.3.2   1.0.0  express-prototype
run-sequence                 1.2.2   1.2.2   2.2.1  express-prototype
standard                    10.0.3  10.0.3  11.0.1  express-prototype
sync-request                 4.1.0   4.1.0   6.0.0  express-prototype

(readdir has been removed as it was not being used)

Individual commits have details for how each change was tested.

https://trello.com/c/Vaizr1wg/1164-2-update-dependencies-in-prototype-kit

@NickColley
Copy link
Contributor

In your commits you mention that we support Node.js 8 and above, given that the data we have suggests lots of 6.x usage is there any reason why we'd not want to support 6.x and above?

@joelanman
Copy link
Contributor

The commit messages don't seem to require 8? They seem to drop support for older Node, but all older than 6

@joelanman joelanman self-requested a review July 19, 2018 10:20
@joelanman
Copy link
Contributor

Not blocking, just to record, I get this output when installing with node 6.10.3:

npm WARN deprecated minimatch@2.0.10: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated minimatch@0.2.14: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated graceful-fs@1.2.3: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.

> fsevents@1.2.4 install /Users/joelanman/projects/govuk-prototype-kit/node_modules/fsevents
> node install

[fsevents] Success: "/Users/joelanman/projects/govuk-prototype-kit/node_modules/fsevents/lib/binding/Release/node-v48-darwin-x64/fse.node" already installed
Pass --update-binary to reinstall or --build-from-source to recompile

> node-sass@4.9.2 install /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass
> node scripts/install.js

Downloading binary from https://github.com/sass/node-sass/releases/download/v4.9.2/darwin-x64-48_binding.node
Download complete  ⸩ ⠋ :
Binary saved to /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass/vendor/darwin-x64-48/binding.node
Caching binary to /Users/joelanman/.npm/node-sass/4.9.2/darwin-x64-48_binding.node

> node-sass@4.9.2 postinstall /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass
> node scripts/build.js

Binary found at /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass/vendor/darwin-x64-48/binding.node
Testing binary
Binary is fine

> nodemon@1.18.3 postinstall /Users/joelanman/projects/govuk-prototype-kit/node_modules/nodemon
> node bin/postinstall || exit 0


> nunjucks@3.1.3 postinstall /Users/joelanman/projects/govuk-prototype-kit/node_modules/nunjucks
> node postinstall-build.js src

npm WARN ajv-keywords@2.1.1 requires a peer of ajv@^5.0.0 but none is installed. You must install peer dependencies yourself.

@joelanman
Copy link
Contributor

On node 8.9.4 - I think it's identical

npm WARN deprecated minimatch@2.0.10: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated minimatch@0.2.14: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated graceful-fs@1.2.3: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.

> fsevents@1.2.4 install /Users/joelanman/projects/govuk-prototype-kit/node_modules/fsevents
> node install

[fsevents] Success: "/Users/joelanman/projects/govuk-prototype-kit/node_modules/fsevents/lib/binding/Release/node-v57-darwin-x64/fse.node" already installed
Pass --update-binary to reinstall or --build-from-source to recompile

> node-sass@4.9.2 install /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass
> node scripts/install.js

Cached binary found at /Users/joelanman/.npm/node-sass/4.9.2/darwin-x64-57_binding.node

> node-sass@4.9.2 postinstall /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass
> node scripts/build.js

Binary found at /Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass/vendor/darwin-x64-57/binding.node
Testing binary
Binary is fine

> nodemon@1.18.3 postinstall /Users/joelanman/projects/govuk-prototype-kit/node_modules/nodemon
> node bin/postinstall || exit 0


> nunjucks@3.1.3 postinstall /Users/joelanman/projects/govuk-prototype-kit/node_modules/nunjucks
> node postinstall-build.js src

npm WARN ajv-keywords@2.1.1 requires a peer of ajv@^5.0.0 but none is installed. You must install peer dependencies yourself.

@36degrees
Copy link
Contributor Author

I based my commit messages off of the fact that we say we require node 8.x.x in the install docs:

https://govuk-prototype-kit.herokuapp.com/docs/install/developer-install-instructions
https://govuk-prototype-kit.herokuapp.com/docs/install/requirements

Copy link
Contributor

@joelanman joelanman left a comment

Choose a reason for hiding this comment

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

looks great thanks for this work!

Can we update changelog and rebase it?

basic-auth 2.0.0 drops support for Node.js below 0.8, but we require Node 8.0.It also removes support for passing the context directly to the auth function, but that's ok because we pass the request object.

Tested by adding the following to the .env file and ensuring that basic auth still functions:

USERNAME=foo
PASSWORD=bar
NODE_ENV=production
USE_HTTPS=false
cross-spawn 6.0 removes support for older node versions, only node >= 4 is supported. This is OK because we already require node >= 8.

Tested by making sure that the Prototype Kit still runs without error.
dotenv 6.0 drops support for node v4, but this is fine because we already require node v6.

Other breaking changes are:
- default path is now path.resolve(process.cwd(), '.env')
- does not write over keys already in process.env if the key has a falsy value

Neither of these should affect the kit.

Tested by adding `FOO=bar` to the `.env` file, and ensuring that it is available within the app by adding a `console.log(process.env.FOO)` and checking for 'bar' in the console output.
Only bugfixes, features and dependency updates.
Tested by running `gulp clean` and checking that the public directory is removed.
There are no release notes or changelog for this module, but looking at sindresorhus/gulp-mocha@v4.0.0...v6.0.0 v6.0 drops support for anything below Node 6. This should be fine, as we require Node 8 or above anyway.

Once updated, the tests started 'hanging' – updating the mocha gulp task to use the example from their readme (passing exit: true) solved this.
gulp-sass 4.0 drops support for Node < 4, but this is fine because we require node 8 anyway.

Tested by running `gulp clean` to remove the public directory, then running `npm start` and ensuring that the stylesheet is re-generated and the prototype kit app looks correct.
Fixes a number of security vulnerabilities, fixes a load of bugs and adds a number of new features.

Some of these are listed as breaking changes, but only because they fix bugs so the output would be different.

Tested by going through the documentation (which is written in markdown) and checking that it looks OK.
This package isn't actually require'd or used anywhere in the application – it's pre-installed to make it easier for prototype kit users to get setup with Notify.
This is no longer used as of 6ff3b6b.
Tested by ensuring that gulp tasks are still included from the gulp directory (the only place that require-dir is used is in gulpfile.js, where it includes the tasks from ./gulp.
Tested by ensuring that the gulp tasks in gulp/tasks.js that use run-sequence still run without error.
Tested by ensuring that the release url on the `/docs/install` page is still generated correctly (sync-request is only used by the getLatestRelease in lib/utils, which is used on the /docs/install page to automatically link to the latest release from GitHub)
@36degrees
Copy link
Contributor Author

Done 👍

@36degrees 36degrees merged commit 9953d9d into master Jul 19, 2018
@36degrees 36degrees deleted the update-dependencies branch July 19, 2018 15:50
@joelanman joelanman mentioned this pull request Jul 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants