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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Node.js 16 #122

Merged
merged 2 commits into from
Jun 23, 2023
Merged

Upgrade to Node.js 16 #122

merged 2 commits into from
Jun 23, 2023

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Dec 20, 2022 •
edited
Loading

Description of proposed changes

Upgrade to Node.js 16 and remove a redundant Auspice build that comes with the upgrade.

Pre-merge tasks

  • Merge #98 which is the target branch of this PR. Otherwise without cross-compilation, Auspice installs/builds are much slower.

Related issue(s)

Testing

  • Checks pass

@victorlin victorlin self-assigned this Dec 20, 2022
@jameshadfield
Copy link
Member

Any reason not to jump straight to node v18 (which is also LTS)?

With regards to possibly removing the redundant auspice build step (#98, which indicates it may be rolled into this PR) I would favour the more explicit approach of npm install --ignore-scripts ... and a later npm run build, especially if the latter will be followed by linking.

@victorlin
Copy link
Member Author

@jameshadfield

Any reason not to jump straight to node v18 (which is also LTS)?

Auspice doesn't officially support v18 (but will soon). Also, v16 is what's currently used on nextstrain.org and auspice.us.

After nextstrain/auspice#1560 is released, we can coordinate an upgrade of v16 → v18 for this Docker image, the two deployed apps, and individual dev environments.

@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 4 times, most recently from 5478afc to 0ff5492 Compare March 15, 2023 18:04
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 0ff5492 to 21f8283 Compare March 21, 2023 21:22
@victorlin victorlin mentioned this pull request Jun 13, 2023
Merged
@jameshadfield jameshadfield mentioned this pull request Jun 13, 2023
Merged
1 task
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 284629e to 7ccea6e Compare June 13, 2023 22:13
@jameshadfield
Copy link
Member

P.S. Node 16 reaches end-of-life in September this year so I'd suggest going straight to node 18 in this PR, as auspice supports it even though we still use v16 on nextstrain.org & auspice.us.

@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 3 times, most recently from 43f57d7 to d6f39af Compare June 22, 2023 23:23
Base automatically changed from victorlin/arm64-cross-compilation to master June 23, 2023 00:12
@victorlin
Upgrade Node.js to v16
626ca5e 
Node.js v16 should run without emulation on the build platform, which
results in faster build times. This was the main reason to avoid
upgrading in a multi-platform setting.

Node.js v16 should also come with performance improvements during run
time.
@victorlin
Remove redundant Auspice build
efb8dcf 
Auspice has a prepare script that runs the build script automatically
after npm install. This means the explicit call to run the build script
is redundant and can be removed to improve build times.
@victorlin
Copy link
Member Author

I'm going to leave this PR as 14→16 and make another PR for 16→18. Just in case there are issues with 14→18, it'll be easier to figure out where the issue comes from.

I plan to merge this once CI passes and I verify ability to run auspice view on the test image.

@victorlin victorlin mentioned this pull request Jun 23, 2023
Merged
1 task
@victorlin victorlin marked this pull request as ready for review June 23, 2023 17:41
@victorlin victorlin merged commit ce72f41 into master Jun 23, 2023
33 checks passed
@victorlin victorlin deleted the victorlin/use-nodejs-16 branch June 23, 2023 17:41
@victorlin victorlin mentioned this pull request Jun 30, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@victorlin victorlin

Labels
None yet
Projects
No open projects
Nextstrain planning (archived)
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@victorlin @jameshadfield