-
Notifications
You must be signed in to change notification settings - Fork 11
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
Upgrade to Node 14 #637
Merged
Merged
Upgrade to Node 14 #637
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update webapp babel (via manual lockfile deletion & reinstall) due to babel/babel#11216 Update webapp node-sass for Node 14 support Update webapp element-ui due to ElemeFE/element#19389 Update graphql bcrypt to hopefully fix rebuild-on-node-version-change behavior Update graphql knex/pg to fix knex/knex#3836 Update graphql winston to fix winstonjs/winston#1797 Update graphql ts-node-dev to fix wclr/ts-node-dev#143 Pin version of graphql graphql-binding because I remember newer versions not being compatible Remove hacks needed for node 8 (asyncIterator polyfill, incorrect date format in ElapsedTime.spec.ts) Updated FilterPanel snapshot. It seems the reason it was reordered was because Array.prototype.sort started using a stable sorting algorithm in Node 11, and `sortFilterKeys` in store/mutations.js sorts against the same value for most items.
@@ -1,14 +1,15 @@ | |||
FROM node:8.16.2 | |||
FROM node:14.5.0-alpine3.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for Alpine
richardgoater
approved these changes
Jul 15, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be great to modernise things a bit, thanks for picking up all those issues.
intsco
approved these changes
Jul 16, 2020
ansible/roles/sm_cluster_autostart_app/templates/sm-cluster-autostart-daemon.supervisor.j2
Outdated
Show resolved
Hide resolved
# Conflicts: # metaspace/graphql/package.json # metaspace/webapp/src/modules/Filters/__snapshots__/FilterPanel.spec.ts.snap
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds nvm for managing node versions, which would allow webapp/graphql to have their own independent versions if necessary. Unfortunately nvm did not integrate well with ansible / supervisor, so the commands to launch node/yarn are a bit hacky in places.
The main issue is that nvm isn't a script - it's a shell function that gets installed when you
source
the~/.nvm/nvm.sh
script, which usually is done automatically via.bashrc
. To get past this, I had to ensurebash
was running whenever nvm was needed, and had to prefix commands with. ~/.nvm/nvm.sh
to install the shell function andnvm use
to switch to the working directory's specified version of node. Neither Ansible nor Supervisord have any non-hacky ways to source.profile
/.bashrc
/etc. before running their commands.Node 14 is not LTS yet - it's due to become LTS in 3 months. I like new shiny things though, and Node 14 has native ES Module support. I pinned it to 14.5.* because I've had dev environment issues in the past when my local copy of node has had a different minor version than my docker copy.
@richardgoater Please review the JS changes
@intsco Please review the ansible & docker changes
Related: #552
Code changes
sortFilterKeys
in store/mutations.js sorts against the same value for most items.Some dependencies needed to be updated for Node 14 support, or to prevent console warnings:
babel
(via manual lockfile deletion & reinstall) due to No "exports" main resolved in @babel/helper-compilation-targets/package.json babel/babel#11216node-sass
for Node 14 binary supportbcrypt
to fix the rebuild-on-node-version-change behavior, as it would previously keep the node 8 binaries even after ayarn install
with node 14knex
andpg
due to Node 14 - Knex PG can't connect knex/knex#3836winston
due to incompatible issue: Node 14 unsupported warning winstonjs/winston#1797ts-node-dev
due to Package is broken with node 14 : fs.watch with recursive is throwing on incompatible os wclr/ts-node-dev#143I also snuck in a couple unrelated changes:
element-ui
due to [Bug Report] Autocomplete broken "Cannot read property 'value' of undefined" ElemeFE/element#19389 (this benign bug has happened >200 times on Sentry)graphql-binding
because I remember newer versions not being compatibleAnsible changes
python
topython3
were overwritten when provisioning from scratch, so I had to make several fixes to prevent python2 from overwriting the symlink.{{ var }}
templates in thewhen
clause in the ElasticSearch taskautorestart
for the supervisor services.Docker changes
Testing
For ansible, I provisioned & deployed to a new dev server to test that the ansible changes all worked.
For webapp, I'd expect any issues would surface during build/test. There are no new errors/warnings as far as I can see.
For graphql:
Deployment
I don't plan to merge this until after the current batch of changes to prod are deployed, as the pending deployment already has a lot of manual steps.
Before deployment the following should be run to install nvm: