-
Notifications
You must be signed in to change notification settings - Fork 2k
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 node version from 12.18.0 to 12.18.4 across the whole repo #45721
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
Blocked by #45722 |
48d9c41
to
3e7c450
Compare
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.
Looks good to me 👍 Thank you!
Do we need to also update the engines
entry in package.json
?
Good catch! We don't have to, but I guess it is more consistent. |
@@ -176,7 +175,8 @@ object RunAllUnitTests : BuildType({ | |||
export NODE_ENV="test" | |||
|
|||
# Update node | |||
. "${'$'}NVM_DIR/nvm.sh" | |||
. "${'$'}NVM_DIR/nvm.sh" --no-use |
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.
I'm not 100% sure why, but the previous code was not picking the new node version. I tested this config using TeamCity custom builds + a patch, and it seems to work.
Doing this as suggested by @scinos to make sure all the changes in this branch build under the this version, which is what master has been using since eea8ef6 (#45721). This is an extra caution step, but a useful one nonetheless. Given the merge strategy for PRs in Calypso at the time of writing is "Squash and merge" this change won't even make it into `master`, as it's already there. Normally, I would rebase the branch on top of the base branch to work around this kind of issues, but given this is a release branch it would not be appropriate.
Background
A high severity issue (CVE-2020-8201) was found in all Node version, Node released patched versions for v10,x, v12.x and v14.x lines
https://nodejs.org/en/blog/vulnerability/september-2020-security-releases/
Changes
Update Node from 12.18.0 to 12.18.4 across the whole repo.
Testing instructions
Verify all tests pass