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

Upgrade to Node.js 16, NPM 7/8, lockfile v2 #635

Merged
merged 7 commits into from
Dec 15, 2022

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Nov 29, 2022

Description of proposed changes

Changes to support a newer Node.js version and the latest NPM version.

Note: this requires a coordinated dev environment upgrade for all contributors. While lockfile v2 is backwards-compatible with older NPM versions, all developers should use NPM 7/8 which is shipped with Node.js 16.

Related issue(s)

Tasks

  • Fix TODOs to replace promisify(): 1, 2

Testing

  • Checks pass
  • Manually click around Heroku preview app for this PR

@victorlin victorlin self-assigned this Nov 29, 2022
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--gurwj9 November 29, 2022 00:05 Inactive
@victorlin victorlin changed the title Upgrade to Node.js 16 and NPM 7/8 Upgrade to Node.js 16, NPM 7/8, lockfile v2 Nov 29, 2022
@victorlin victorlin mentioned this pull request Nov 29, 2022
4 tasks
@nextstrain-bot nextstrain-bot had a problem deploying to nextstrain-s-victorlin--gurwj9 November 29, 2022 01:32 Failure
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--gurwj9 November 29, 2022 01:40 Inactive
@victorlin victorlin marked this pull request as ready for review November 29, 2022 01:57
@victorlin victorlin requested a review from a team November 29, 2022 01:58
@@ -11,8 +11,8 @@
"license": "MIT",
"main": "n/a",
"engines": {
"node": "^14",
"npm": "^6.14"
"node": "^16",
Copy link
Member

Choose a reason for hiding this comment

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

Using this line in the code to get a 🧵, but the error isn't related to node-16 AFAIK.

One of the benefits of this was to allow native (arm64) installs of node, as eariler versions weren't available on conda for M1 architectures. However I can't seem to build the static-site under a M1 architecture as sharp (used in the static site via gatsby-sharp-plugin) doesn't seem to be useable on M1 - see lovell/sharp#2533.

Initially I got the following stack trace:

Stack trace of build failure related to gatsby-sharp-plugin
bash build.sh static
Building the static site (./static-site/public/)
npm WARN deprecated @types/[email protected]: This is a stub types definition. vfile-message provides its own type definitions, so you do not need this installed.
npm WARN deprecated [email protected]: Please update to ini >=1.3.6 to avoid a prototype pollution issue
npm WARN deprecated [email protected]: No longer maintained. Use [lru-cache](http://npm.im/lru-cache) version 7.6 or higher, and provide an asynchronous `fetchMethod` option.
npm WARN deprecated [email protected]: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated [email protected]: flatten is deprecated in favor of utility frameworks such as lodash.
npm WARN deprecated @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
npm WARN deprecated @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
npm WARN deprecated [email protected]: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated [email protected]: this library is no longer supported
npm WARN deprecated [email protected]: This loader has been deprecated. Please use eslint-webpack-plugin
npm WARN deprecated [email protected]: “Please update to latest v2.3 or v2.2”
npm WARN deprecated [email protected]: https://github.com/lydell/resolve-url#deprecated
npm WARN deprecated [email protected]: Browserslist 2 could fail on reading Browserslist >3.0 config used in other tools.
npm WARN deprecated [email protected]: Browserslist 2 could fail on reading Browserslist >3.0 config used in other tools.
npm WARN deprecated [email protected]: Browserslist 2 could fail on reading Browserslist >3.0 config used in other tools.
npm WARN deprecated [email protected]: See https://github.com/lydell/source-map-resolve#deprecated
npm WARN deprecated [email protected]: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
npm WARN deprecated [email protected]: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
npm WARN deprecated [email protected]: Browserslist 2 could fail on reading Browserslist >3.0 config used in other tools.
npm WARN deprecated [email protected]: See https://github.com/lydell/source-map-url#deprecated
npm WARN deprecated [email protected]: CircularJSON is in maintenance only, flatted is its successor.
npm WARN deprecated [email protected]: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.
npm WARN deprecated [email protected]: Chokidar 2 does not receive security updates since 2019. Upgrade to chokidar 3 with 15x fewer dependencies
npm WARN deprecated [email protected]: Chokidar 2 does not receive security updates since 2019. Upgrade to chokidar 3 with 15x fewer dependencies
npm WARN deprecated [email protected]: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
npm WARN deprecated [email protected]: babel-eslint is now @babel/eslint-parser. This package will no longer receive updates.
npm WARN deprecated @hapi/[email protected]: Moved to ‘npm install @sideway/address’
npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
npm WARN deprecated [email protected]: The `subscriptions-transport-ws` package is no longer maintained. We recommend you use `graphql-ws` instead. For help migrating Apollo software to `graphql-ws`, see https://www.apollographql.com/docs/apollo-server/data/subscriptions/#switching-from-subscriptions-transport-ws    For general help using `graphql-ws`, see https://github.com/enisdenjo/graphql-ws/blob/master/README.md
npm WARN deprecated @hapi/[email protected]: Switch to ‘npm install joi’
npm WARN deprecated [email protected]: This SVGO version is no longer supported. Upgrade to v2.x.x.
npm WARN deprecated [email protected]: gatsby-recipes has been removed from gatsby/gatsby-cli >=4.5.0. Update to gatsby@latest/gatsby-cli@latest to use versions without gatsby-recipes. This package will no longer receive updates.
npm WARN deprecated [email protected]: core-js@<3.23.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Some versions have web compatibility issues. Please, upgrade your dependencies to the actual version of core-js.
npm WARN deprecated [email protected]: core-js@<3.23.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Some versions have web compatibility issues. Please, upgrade your dependencies to the actual version of core-js.
npm ERR! code 1
npm ERR! path /Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp
npm ERR! command failed
npm ERR! command sh /var/folders/7w/rvnm5fgd7kb83fw8lg882x680000gn/T/install-190b74fb.sh
npm ERR! CC(target) Release/obj.target/nothing/../node-addon-api/src/nothing.o
npm ERR!   LIBTOOL-STATIC Release/nothing.a
npm ERR!   TOUCH Release/obj.target/libvips-cpp.stamp
npm ERR!   CXX(target) Release/obj.target/sharp/src/common.o
npm ERR! info sharp Downloading https://github.com/lovell/sharp-libvips/releases/download/v8.9.1/libvips-8.9.1-darwin-arm64v8.tar.gz
npm ERR! ERR! sharp Prebuilt libvips 8.9.1 binaries are not yet available for darwin-arm64v8
npm ERR! info sharp Attempting to build from source via node-gyp but this may fail due to the above error
npm ERR! info sharp Please see https://sharp.pixelplumbing.com/install for required dependencies
npm ERR! gyp info it worked if it ends with ok
npm ERR! gyp info using [email protected]
npm ERR! gyp info using [email protected] | darwin | arm64
npm ERR! gyp info find Python using Python version 3.10.7 found at “/usr/local/opt/[email protected]/bin/python3.10”
npm ERR! gyp info spawn /usr/local/opt/[email protected]/bin/python3.10
npm ERR! gyp info spawn args [
npm ERR! gyp info spawn args   ‘/Users/enigma/miniconda3/envs/node16/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py’,
npm ERR! gyp info spawn args   ‘binding.gyp’,
npm ERR! gyp info spawn args   ‘-f’,
npm ERR! gyp info spawn args   ‘make’,
npm ERR! gyp info spawn args   ‘-I’,
npm ERR! gyp info spawn args   ‘/Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp/build/config.gypi',
npm ERR! gyp info spawn args   ‘-I’,
npm ERR! gyp info spawn args   ‘/Users/enigma/miniconda3/envs/node16/lib/node_modules/npm/node_modules/node-gyp/addon.gypi’,
npm ERR! gyp info spawn args   ‘-I’,
npm ERR! gyp info spawn args   ‘/Users/enigma/Library/Caches/node-gyp/16.17.1/include/node/common.gypi’,
npm ERR! gyp info spawn args   ‘-Dlibrary=shared_library’,
npm ERR! gyp info spawn args   ‘-Dvisibility=default’,
npm ERR! gyp info spawn args   ‘-Dnode_root_dir=/Users/enigma/Library/Caches/node-gyp/16.17.1’,
npm ERR! gyp info spawn args   ‘-Dnode_gyp_dir=/Users/enigma/miniconda3/envs/node16/lib/node_modules/npm/node_modules/node-gyp’,
npm ERR! gyp info spawn args   ‘-Dnode_lib_file=/Users/enigma/Library/Caches/node-gyp/16.17.1/<(target_arch)/node.lib’,
npm ERR! gyp info spawn args   ‘-Dmodule_root_dir=/Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp',
npm ERR! gyp info spawn args   ‘-Dnode_engine=v8’,
npm ERR! gyp info spawn args   ‘—depth=.’,
npm ERR! gyp info spawn args   ‘—no-parallel’,
npm ERR! gyp info spawn args   ‘—generator-output’,
npm ERR! gyp info spawn args   ‘build’,
npm ERR! gyp info spawn args   ‘-Goutput_dir=.’
npm ERR! gyp info spawn args ]
npm ERR! gyp info spawn make
npm ERR! gyp info spawn args [ ‘BUILDTYPE=Release’, ‘-C’, ‘build’ ]
npm ERR! warning: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: archive library: Release/nothing.a the table of contents is empty (no object file members in the library define global symbols)
npm ERR! ../src/common.cc:23:10: fatal error: ‘vips/vips8’ file not found
npm ERR! #include <vips/vips8>
npm ERR!          ^~~~~~~~~~~~
npm ERR! 1 error generated.
npm ERR! make: *** [Release/obj.target/sharp/src/common.o] Error 1
npm ERR! gyp ERR! build error 
npm ERR! gyp ERR! stack Error: `make` failed with exit code: 2
npm ERR! gyp ERR! stack     at ChildProcess.onExit (/Users/enigma/miniconda3/envs/node16/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:194:23)
npm ERR! gyp ERR! stack     at ChildProcess.emit (node:events:513:28)
npm ERR! gyp ERR! stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)
npm ERR! gyp ERR! System Darwin 20.6.0
npm ERR! gyp ERR! command “/Users/enigma/miniconda3/envs/node16/bin/node” “/Users/enigma/miniconda3/envs/node16/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js” “rebuild”
npm ERR! gyp ERR! cwd /Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp
npm ERR! gyp ERR! node -v v16.17.1
npm ERR! gyp ERR! node-gyp -v v9.0.0
npm ERR! gyp ERR! not ok

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/enigma/.npm/_logs/2022-11-30T23_30_18_685Z-debug-0.log

Build failed at build.sh line 37

You are responsible for clean up (sorry!)

I overcame this by running brew install vips (from this stackoverflow answer). However now the static-site can't build:

  Error: Something went wrong installing the "sharp" module
  dlopen(/Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp/build/Release/sharp.node, 1): Symbol not found: __ZTVN4vips7VOptionE
    Referenced from: /Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp/build/Release/sharp.node
    Expected in: flat namespace
   in /Users/enigma/projects/nextstrain/nextstrain.org/static-site/node_modules/sharp/build/Release/sharp.node
  - Remove the "node_modules/sharp" directory then run
    "npm install --ignore-scripts=false --verbose" and look for errors
  - Consult the installation documentation at https://sharp.pixelplumbing.com/install
  - Search for this error at https://github.com/lovell/sharp/issues

I'll try creating a rosetta environment with node16 & see if that works...

Copy link
Member Author

Choose a reason for hiding this comment

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

I also did brew install vips (which took forever!!!) but don't remember encountering issues afterwards. I can try again, though a better solution here might be to upgrade to a newer version of gatsby-plugin-sharp that uses sharp v0.28.0 (based on lovell/sharp#2460 (comment)).

Copy link
Member

Choose a reason for hiding this comment

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

I'll try creating a rosetta environment with node16 & see if that works...

Can confirm this works, and so the above is related to M1 (which won't be encountered by CI)

node -p "process.arch"
x64

Copy link
Member

Choose a reason for hiding this comment

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

I presume brew is arch specific (it's gotta be...) and so I've probably got a osx-64 brew? This is not fun stuff & I don't like using brew because it's not scoped to the (conda) environment

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I'm running into a different build error (webpack/webpack#14532) since Webpack 4 is used in static-site (looks like that's the resolved Webpack version for the installed version of gatsby):

static-site % npm list webpack
[email protected]
├─┬ [email protected]
│ ├─┬ @pieh/[email protected]
│ │ └── [email protected] deduped
│ ├─┬ @pmmmwh/[email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ └─┬ [email protected]
│   └─┬ [email protected]
│     └── [email protected] deduped
└─┬ [email protected]
  └── [email protected] deduped

In any case, like you mentioned, removing Gatsby would solve things.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll try creating a rosetta environment with node16 & see if that works...

I just ran into the same issue, but this didn't work for me 😕

@@ -58,6 +58,22 @@ rules:
react/no-danger: off # gatsby uses this a lot
no-use-before-define: ["error", { "functions": false }]
no-return-await: off
# <<< These were enabled when upgrading eslint-config-airbnb 16→19. Disabling for now.
Copy link
Member

Choose a reason for hiding this comment

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

Historical context -- we started with the eslint-config-airbnb a long time ago, and I remember that the seemed pretty sensible at the time. We haven't regularly updated these (it's a chore) and from time-to-time we would disable rules in this file when we found them to be frustrating. The goal of linting (in my eyes) is largely to help avoid errors / JS gotchas. While stylistic consistency is nice, and linting helps here, certain rules (e.g. operator-linebreak) tend to be controversial and not actually improve the code quality all that much so I've tended to turn them off.

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense. I think it's still good to keep eslint-config-airbnb around (or if not, copy over all the rules that are actively being used). And some of these new rules might be useful in avoiding errors / JS gotchas, but I see that as a separate task to go through rules one-by-one and evaluate usefulness.

Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Everything's working for me under the following steps:

Node 16.17.1, npm 8.15.0, arch=osx-64 (i.e. rosetta emulation on M1)

I’m using the following branch https://github.com/nextstrain/nextstrain.org/tree/james/test-victors-prs which is:

  1. Checkout this PR
  2. The single commit from PR auspice-client: Append extension on ESM imports #634 cherry-picked (helped with testing a suite of PRs together)
  3. npm ci
  4. Auspice upgraded via npm install nextstrain/auspice#7be662dc4554319a3d3f02fc5d75cdf8ea5366d8 (not needed to test this PR, but helped me test a suite of PRs together)
  5. bash build.sh
  6. npm run server

It's not working (for me) in a arm-64 environment - see above. I suggest we accept this for now and work on migrating off gatsby rather than fixing that particular issue.

@jameshadfield
Copy link
Member

This PR branch is about to be rebased onto master, #634 merged into it, and a commit added bumping auspice etc. Assuming all tests pass (and I expect they will) this PR will be merged into master triggering a deployment to canary.

Node.js 16 provides a faster developer experience, especially for those
using Apple silicon since it is the earliest version that can be
installed natively¹.

NPM 7 is the earliest major version that is installed alongside Node.js
16, and comes with a new lockfile version. NPM 8 is also supported since
it uses the same lockfile version and is currently shipped alongside
Node.js 16.

The lockfile was upgraded from a fresh environment (node v16.17.1, npm
8.15.0) using `npm ci && npm install`.

¹ https://nodejs.org/en/blog/release/v16.0.0/
Now that we use NPM 7 which tries to install peer dependencies, having
an old version of eslint-config-airbnb alongside a newer version of
eslint will not work with `npm install` as it used to.

Version 19 is the earliest version to support the currently installed
version of eslint.

Command used:

    npm install \
        eslint-config-airbnb@19 \
        eslint-plugin-jsx-a11y@6
The upgrade from version 16 to 19 introduced some new rules. Disabling
them for now.

Command used:

    npx eslint --ext .js,.jsx . --format json \
        | jq -r '.[] | .messages[] | .ruleId' \
        | sort | uniq
This is necessary for Webpack 5, which resolves paths as fully
specified.

Something similar was done in the CJS→ESM migration
(56b50c5), independent of
`auspice-client`.
@victorlin victorlin force-pushed the victorlin/nodejs-16 branch from 44d9e70 to 7d0b0ca Compare December 9, 2022 20:22
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--odlbya December 9, 2022 20:22 Inactive
@victorlin victorlin force-pushed the victorlin/nodejs-16 branch from 7d0b0ca to 8b0c656 Compare December 9, 2022 21:39
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--odlbya December 9, 2022 21:39 Inactive
The new version of Auspice (which uses Webpack v5) uses a built-in Asset
Module (asset/resource) to handle files that were previously handled
with file-loader¹.

file-loader in Webpack v4 is documented to handle both import and
require()². Based on docs which suggest that asset/resource directly
replaces file-loader³, I'd expect it to also support require(). It seems
like that is not the case, so this is a workaround.

¹ nextstrain/auspice@7892c59
² https://v4.webpack.js.org/loaders/file-loader/
³ https://webpack.js.org/guides/asset-modules/
This release includes the webpack v4 upgrade
@jameshadfield
Copy link
Member

This ended up being a lot of work with many twists and turns. Thanks @victorlin for seeing it through with me ⭐

Merging now which will deploy auspice 2.42.0 to canary

@jameshadfield jameshadfield merged commit f04cfd1 into master Dec 15, 2022
@jameshadfield jameshadfield deleted the victorlin/nodejs-16 branch December 15, 2022 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

CI: Check lockfile version and fail if it is v2
3 participants