Skip to content

Update from node v14 to v18.6.0 - #4778

Merged
mnaamani merged 17 commits into
Joystream:masterfrom
mnaamani:update-to-node-18
Jul 24, 2023
Merged

mnaamani merged 17 commits into
Joystream:masterfrom
mnaamani:update-to-node-18

Conversation

@mnaamani

@mnaamani mnaamani commented Jun 1, 2023 •

Copy link
Copy Markdown
Member

Updated all volta sections in repo package.json files to use node 18, as well is in github workflows and docker images.
I did not change the min version in engine sections to prevent breaking any deployments where volta is not used or forcing users to upgrade. But as long as volta is used, it will trigger it to update and use new version.

@vercel

vercel Bot commented Jun 1, 2023 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
pioneer-testnet ⬜️ Ignored (Inspect) Jul 20, 2023 8:21pm

@mnaamani
mnaamani marked this pull request as ready for review July 6, 2023 13:26
@mnaamani
mnaamani requested a review from zeeshanakram3 July 16, 2023 00:22
Comment thread package.json Outdated
},
"engines": {
"node": ">=14.0.0",
"node": ">=14.6.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If Volta is not installed, then this engine requirement could break things. I got the following error while installing dependencies using node 14.7.0.

[1/6] Validating package.json...
[2/6] Resolving packages...
[3/6] Fetching packages...
error file-type@17.1.1: The engine "node" is incompatible with this module. Expected version "^12.20.0 || ^14.13.1 || >=16.0.0". Got "14.7.0"
error Found incompatible module.

So the engine min version needs to be increased. Things seem to work at the minimum of 14.18.0 (version previously defined in "volta" section)

Comment thread README.md Outdated

- [Rust](https://www.rust-lang.org/tools/install) toolchain - _required_
- [nodejs](https://nodejs.org/) v14.x - _required_
- [nodejs](https://nodejs.org/) >= v18.6 - _required_

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't be v14.x.x i.e. the minimum node version defined in the engine section?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will fix, I think I changed that originally when I was forcing all engine filed to be >=18

@mnaamani

Copy link
Copy Markdown
Member Author

@zeeshanakram3 Applied your feedback, I also update Readme to document the fact that old version of linux will not support node v18, so I include a workaround.

@zeeshanakram3

Copy link
Copy Markdown
Contributor

Looks good, I think we should use the same nodes engine requirement (>=14.18.0) in all package.json files in monorepo.

@mnaamani
mnaamani requested a review from zeeshanakram3 July 20, 2023 20:25
@mnaamani

Copy link
Copy Markdown
Member Author

@zeeshanakram3 Applied your feedback, I also update Readme to document the fact that old version of linux will not support node v18, so I include a workaround.

done in 4a985be

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
mnaamani and others added 3 commits July 22, 2023 08:00
Co-authored-by: Zeeshan Akram <37098720+zeeshanakram3@users.noreply.github.com>
Co-authored-by: Zeeshan Akram <37098720+zeeshanakram3@users.noreply.github.com>

@zeeshanakram3 zeeshanakram3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mnaamani
mnaamani merged commit 6ca3cbf into Joystream:master Jul 24, 2023
@mnaamani
mnaamani deleted the update-to-node-18 branch July 29, 2023 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants