-
Notifications
You must be signed in to change notification settings - Fork 130
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
Node update for compatibility with lts versions 18 and 20 #2402
Conversation
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.
Tested locally using Node v18.19.1 and it works correctly.
To do:
- Rebase and fix conflict
- Update the reference to WET-BOEW in package.json to the latest version when it is released.
Equivalent change was merged in wet-boew, please rebase and update the build configuration as intended on the wet-boew tag is created. |
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.
The version of wet-boew that are running with Node 18-20 is now released. You can update your PR to align all the dependencies as expected.
c6a5ac5
to
ec5b660
Compare
Well, after this change we will need to look for alternative for node-sass at our next upgrade of NodeJS to build GCWeb
|
@Ricokola FYI, please review the step bellow and let's include those in our release note under "Building GCWeb developer note" sub-title. Here my step that I took to upgrade my dev build environment on Window 10:
Then, it will be as usual step to build
There is an error in wet-boew, we need to change how we enforce the npm dependencies
We should change for: To workaround locally, I needed to run Next step to fix that issue:
Note to reinstate from that above error: You will need either to reset the package-lock.json or simply delete it to get it regenerated. |
Here are the steps for testing locally:
|
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.
Please remove the merge commit
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 have tested this PR and everything works well locally after the cherry-pick of @EricDunsworth 's PR. However, the merge commit will need to be removed.
Update node for compatibility with lts versions 18 and 20.