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

Node update for compatibility with lts versions 18 and 20 #2402

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

Ricokola
Copy link
Contributor

Update node for compatibility with lts versions 18 and 20.

@Ricokola Ricokola changed the title Node update to v20 Node update for compatibility with lts versions 18 and 20 Jul 18, 2024
Garneauma
Garneauma previously approved these changes Aug 15, 2024
Copy link
Contributor

@Garneauma Garneauma left a 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.

@duboisp duboisp added this to the v15.6.0 milestone Aug 19, 2024
@duboisp
Copy link
Member

duboisp commented Aug 19, 2024

Equivalent change was merged in wet-boew, please rebase and update the build configuration as intended on the wet-boew tag is created.

Copy link
Member

@duboisp duboisp left a 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.

package.json Outdated Show resolved Hide resolved
@duboisp
Copy link
Member

duboisp commented Aug 22, 2024

Well, after this change we will need to look for alternative for node-sass at our next upgrade of NodeJS to build GCWeb

image

npm warn deprecated [email protected]: Node Sass is no longer supported. Please use `sass` or `sass-embedded` instead.

@duboisp
Copy link
Member

duboisp commented Aug 23, 2024

@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:

  1. Uninstall existing NodeJS version if installed OS wide
  2. Get NodeJS LTS recent version
    a. Preferably - Install NVM - Node Version Manager and use Node 20 LTS
    b. Or, install Node 20 LTS OS wide
  3. Delete the folder "node-sass" under the "node_modules"
  4. Run "script/setup" which would reinstall bower and install all the NodeJS libraries

Then, it will be as usual step to build

  1. If wet-boew dependency is updated > npm install
  2. Build GCWeb > grunt
  3. Run Jekyll local website > docker-compose up
  4. Test the webiste + code by navigating locally: http://localhost:4000/

There is an error in wet-boew, we need to change how we enforce the npm dependencies

npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '[email protected]',
npm warn EBADENGINE   required: { npm: '~10.7.0' },
npm warn EBADENGINE   current: { node: 'v20.17.0', npm: '10.8.2' }
npm warn EBADENGINE }

We should change for: required: { npm: '>=10.7.0' } (please check the syntax first) in the wet-boew package.json file.

To workaround locally, I needed to run > npm install [email protected] -g to force the use of npm v10.7.0

Next step to fix that issue:

  • Update wet-boew and ensure the version indicator is "greater" version
  • Add the npm requirement in GCWeb or remove it in wet-boew
  • Optional: Only specify the NodeJS requirement in the package.json file

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.

@Ricokola
Copy link
Contributor Author

@Garneauma @duboisp

Here are the steps for testing locally:

  1. Reset hard on this PR latest commit: 716b3f9
  2. Ensure you run node 20 (One solution is installing and using NVM with command: nvm use lts/iron).
  3. Execute a "cherry-pick" on the following commit from Eric Dunsworth: 7daac31
  4. Execute a "grunt test" command and then,
  5. Build locally using "docker compose up"

Copy link
Member

@duboisp duboisp left a 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

Garneauma
Garneauma previously approved these changes Aug 27, 2024
Copy link
Contributor

@Garneauma Garneauma left a 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.

@Garneauma Garneauma merged commit fb61643 into wet-boew:master Aug 27, 2024
1 check failed
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.

3 participants