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

Bump Node.js version 8.9.4 -> 10.15.2 #785

Merged
merged 6 commits into from
Jul 9, 2019
Merged

Bump Node.js version 8.9.4 -> 10.15.2 #785

merged 6 commits into from
Jul 9, 2019

Conversation

msorens
Copy link
Contributor

@msorens msorens commented Jul 8, 2019

🔩 Description

Continuing work towards the latest Angular upgrade, it is necessary to move to Node.Js version 10. Though Node.js is up to v12, v10 is the appropriate version for production applications, which should use either "Active" or "Maintenance" phase versions, and the state of the world today is shown here (from https://nodejs.org/en/about/releases/):

image

(So in a couple months it would be appropriate to bump the version further to v12, but not now.)

The latest release of node 10/Active is designated with codename "Dubnium" only, as far as I can tell, because it begins with "D", and is the follow-on to node 8/Active "Carbon" beginning with a "C", and so forth. You can see that the latest Dubnium release is quite recent (https://nodejs.org/en/download/releases/).

For those interested, see What's New In Node 10.

Also included in this PR are two other changes relevant to the impending Angular upgrade:

  1. Needed to upgrade codelyzer, which is used for a small subset of linting rules.

  2. Some time ago, we had already migrated from @angular/http to @angular/common/http per the Angular preparation instructions... but the actual package @angular/http -- now unused -- was still in the package.json file, and it was impeding the Angular upgrade.

⚠️ In case I am not the one merging this, it should be accompanied by a notice in multiple Slack channels that if you build the ui locally you need to (a) upgrade your own version of node and (b) npm install.

Update:
At the time of writing, the latest Dubnium version is 10.16.0. But we are limited by the packages that habitat knows about, too, and the latest there is 10.15.2 (thanks to @srenatus and @afiune!). The available habitat packages are listed at https://bldr.habitat.sh/#/pkgs/core/node10.

👍 Definition of Done

UI still works.

👟 Demo Script / Repro Steps

  1. Upgrade your local version of node.
nvm use 10.15.2
  1. Update your packages corresponding to the node version change (order matters!).
cd automate/components/chef-ui-library
npm install
cd components/automate-ui
npm install
  1. Restart your automate-ui server
make serve

⛓️ Related Resources

✅ Checklist

  • Necessary tests added/updated?
  • Necessary docs added/updated?
  • Code actually executed?
  • Vetting performed (unit tests, lint, etc.)?

As detailed here (https://nodejs.org/en/about/releases/)
production applications should only use "active" or "maintenance"
releases, which means v10.

This requires a complete overhaul of the node_modules folder,
as those are Node.js version specific.

After `npm install` on both chef-ui-lib and automate-ui,
confirmed that the UI builds properly.
Spot checking the UI in the browser,
 I encountered no runtime errors or warnings.

Signed-off-by: michael sorens <msorens@chef.io>
Codelyzer was an incompatibility for the Angular upgrade.
This updates from 4.4.4 to 4.5.0, which meets the prereqs.

Signed-off-by: michael sorens <msorens@chef.io>
We had converted from @angular/http to @angular/common/http
some time ago but apparently the actual package had never been removed.
The presence of the package is an impediment for
the impending Angular 8 upgrade.
`ng update` complained about an incompatible peer dependency for it,
which I suppose is true, but it was not really pointing me towards
the correct issue.

Signed-off-by: michael sorens <msorens@chef.io>
@msorens msorens requested review from a team July 8, 2019 02:21
@msorens msorens self-assigned this Jul 8, 2019
Copy link

@afiune afiune left a comment

Choose a reason for hiding this comment

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

The nodejs version 10.16.0 doesn't exist in habitat world.

You can check the available versions at: https://bldr.habitat.sh/#/pkgs/core/node

@srenatus
Copy link
Contributor

srenatus commented Jul 8, 2019

💭 If we bump, we might as well bump to v12, I guess?

should use either "Active" or "Maintenance" phase versions

looks like the sorce says

Production applications should only use Active LTS or Maintenance LTS releases.

...and these are v8 and v10 respectively, at the moment.

So yeah let's bump this. I suppose we could look into getting the latest v10 into core-plans, then?

(the latest version of the v10 series we currently have in core-plans)

Signed-off-by: Stephan Renatus <srenatus@chef.io>
Except for automate-workflow-ui, which is still using 5.6.0.

Signed-off-by: Stephan Renatus <srenatus@chef.io>
@srenatus srenatus changed the title Bump Node.js version 8.9.4 -> 10.16.0 Bump Node.js version 8.9.4 -> 10.15.2 Jul 8, 2019
@srenatus
Copy link
Contributor

srenatus commented Jul 8, 2019

Fixed the title. To bump to 10.16.0, we'll need some action on https://github.com/habitat-sh/core-plans.

@@ -73,7 +72,7 @@
"ajv": "^6.5.3",
"angular2-template-loader": "^0.6.2",
"axe-webdriverjs": "^2.0.1",
"codelyzer": "^4.4.4",
"codelyzer": "^4.5.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes to these deps feels very minor, it might be worth putting them in their own PR to make this one smaller.

Copy link
Contributor

@tarablack01 tarablack01 left a comment

Choose a reason for hiding this comment

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

Looks good :)

Signed-off-by: michael sorens <msorens@chef.io>
@srenatus
Copy link
Contributor

srenatus commented Jul 9, 2019

@phiggins I'm with you. I'll drop that commit and merge this when green.

Update: should have checked first -- it's not that consequential; the changed lines are many without the codelyzer change, too. I'll merge this incuding that commit now.

@srenatus srenatus force-pushed the ms/ui-node-update branch 2 times, most recently from 3691fef to 563bd70 Compare July 9, 2019 08:39
@srenatus srenatus merged commit 4bbd5de into master Jul 9, 2019
@chef-ci chef-ci deleted the ms/ui-node-update branch July 9, 2019 09:06
@srenatus
Copy link
Contributor

srenatus commented Jul 9, 2019

In case I am not the one merging this, it should be accompanied by a notice in multiple Slack channels that if you build the ui locally you need to (a) upgrade your own version of node and (b) npm install.

✔️

@tduffield
Copy link
Contributor

In the future, please also coordinate with release engineering when making major version upgrades of software languages. We'll need to make sure our pipelines are ready to receive such an upgrade :) cc/ @sdelano @vjeffrey

@msorens
Copy link
Contributor Author

msorens commented Jul 11, 2019

Thanks for the note, @tduffield. I guess I had thought of this as a library update, like we've done for Go and TypeScript casually, but I can see that this is a bigger thing.

msorens added a commit that referenced this pull request Dec 23, 2019
Two previous PRs (#785 and #2267) updated plan.sh
in a couple projects to be specific to a node.js major version.
I now believe that was an unnecessary, and in fact, brittle change.
A node.js package newly installed in core-plans makes its way
to the "main branch" (https://bldr.habitat.sh/#/pkgs/core/node).
References should remain invariant to that location
so they do not need updating when node.js major versions change.

Signed-off-by: michael sorens <msorens@chef.io>
msorens added a commit that referenced this pull request Jan 3, 2020
* Reverting reference back to main path

Two previous PRs (#785 and #2267) updated plan.sh
in a couple projects to be specific to a node.js major version.
I now believe that was an unnecessary, and in fact, brittle change.
A node.js package newly installed in core-plans makes its way
to the "main branch" (https://bldr.habitat.sh/#/pkgs/core/node).
References should remain invariant to that location
so they do not need updating when node.js major versions change.

Signed-off-by: michael sorens <msorens@chef.io>

* automate-ui: Regenerate with new node version

Only change: added `optional` property to many packages.

$ npm install

> automate-ui@0.0.0 install /Users/msorens/code/go/src/github.com/chef/automate/components/automate-ui
> echo 'use `npm run install:ui-library` for old behaviour'; npm run copy-ui-lib

use `npm run install:ui-library` for old behaviour

> automate-ui@0.0.0 copy-ui-lib /Users/msorens/code/go/src/github.com/chef/automate/components/automate-ui
> npm run copy-ui-lib:library && npm run copy-ui-lib:fonts

> automate-ui@0.0.0 copy-ui-lib:library /Users/msorens/code/go/src/github.com/chef/automate/components/automate-ui
> cp -r ../chef-ui-library/dist/ src/assets/chef-ui-library

> automate-ui@0.0.0 copy-ui-lib:fonts /Users/msorens/code/go/src/github.com/chef/automate/components/automate-ui
> cp -r ../chef-ui-library/dist/collection/assets/fonts/ src/assets/fonts

audited 18030 packages in 44.608s
found 0 vulnerabilities

Signed-off-by: michael sorens <msorens@chef.io>

* chef-ui-library: Regenerate with new node version

Only change: added `optional` property to many packages.

$ npm install
audited 1748734 packages in 9.775s
found 16 vulnerabilities (3 moderate, 13 high)
  run `npm audit fix` to fix them, or `npm audit` for details

Signed-off-by: michael sorens <msorens@chef.io>

* automate-workflow-web: Regenerate with new node version

Only change: added `optional` property to many packages.

$ npm install

> fsevents@1.2.9 install /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/fsevents
> node install

node-pre-gyp WARN Using request for node-pre-gyp https download
[fsevents] Success: "/Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/fsevents/lib/binding/Release/node-v72-darwin-x64/fse.node" is installed via remote

> node-zopfli@2.1.3 install /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/node-zopfli
> prebuild-install --runtime napi || prebuild-install || node-gyp rebuild

> phantomjs-prebuilt@2.1.16 install /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/phantomjs-prebuilt
> node install.js

PhantomJS not found on PATH
Downloading https://github.com/Medium/phantomjs/releases/download/v2.1.1/phantomjs-2.1.1-macosx.zip
Saving to /var/folders/c0/dxv4vgb11ts_cvznblv5pvtm0000gn/T/phantomjs/phantomjs-2.1.1-macosx.zip
Receiving...
  [=======================================-] 98%
Received 16746K total.
Extracting zip contents
Removing /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/phantomjs-prebuilt/lib/phantom
Copying extracted folder /var/folders/c0/dxv4vgb11ts_cvznblv5pvtm0000gn/T/phantomjs/phantomjs-2.1.1-macosx.zip-extract-1577140037455/phantomjs-2.1.1-macosx -> /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/phantomjs-prebuilt/lib/phantom
Writing location.js file
Done. Phantomjs binary available at /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/phantomjs-prebuilt/lib/phantom/bin/phantomjs

> node-sass@4.13.0 install /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/node-sass
> node scripts/install.js

Downloading binary from https://github.com/sass/node-sass/releases/download/v4.13.0/darwin-x64-72_binding.node
Download complete
Binary saved to /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/node-sass/vendor/darwin-x64-72/binding.node
Caching binary to /Users/msorens/.npm/node-sass/4.13.0/darwin-x64-72_binding.node

> node-sass@4.13.0 postinstall /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/node-sass
> node scripts/build.js

Binary found at /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/node-sass/vendor/darwin-x64-72/binding.node
Testing binary
Binary is fine

> sauce-connect-launcher@0.12.0 postinstall /Users/msorens/code/go/src/github.com/chef/automate/components/automate-workflow-web/node_modules/sauce-connect-launcher
> node scripts/install.js

npm WARN The package font-awesome is included as both a dev and production dependency.

added 1166 packages from 1337 contributors and audited 6940 packages in 189.377s
found 66 vulnerabilities (22 low, 9 moderate, 35 high)
  run `npm audit fix` to fix them, or `npm audit` for details

Signed-off-by: michael sorens <msorens@chef.io>

* automate-ui: bump .nvmrc

Signed-off-by: michael sorens <msorens@chef.io>

* automate-workflow-web: bump .nvmrc

Signed-off-by: michael sorens <msorens@chef.io>

* chef-ui-library: bump .nvmrc

Signed-off-by: michael sorens <msorens@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants