Skip to content

Add support to latest NodeJS LTS version#328

Merged
gildub merged 7 commits intoguacsec:mainfrom
gildub:bump-nodejs
Mar 3, 2025
Merged

Add support to latest NodeJS LTS version#328
gildub merged 7 commits intoguacsec:mainfrom
gildub:bump-nodejs

Conversation

@gildub
Copy link
Copy Markdown
Contributor

@gildub gildub commented Jan 29, 2025

NodeJS 22 is LTS

  • Using UBI9/nodejs-22 for Dockerfiles
  • Updated github workflows accordingly
  • Bump @hey-api clients to support nodejs 22
  • Bump few @storybook packages for proper support of nodejs 22 types

Fix #327

@gildub gildub requested a review from carlosthe19916 January 29, 2025 21:20
@gildub gildub force-pushed the bump-nodejs branch 14 times, most recently from 5d14ac8 to 75b86de Compare January 30, 2025 17:23
Copy link
Copy Markdown
Collaborator

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

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

The test are failing. And it is due to the changes in this PR. See this other test PR I created #330 , the tests are passing but they fail here. https://github.com/trustification/trustify-ui-tests has quite a simple test so there is something wrong with this PR. Please clone that repo and make sure this PR does not break that basic test... on a side note, I am happy that we have the e2e tests even at a basic level, we can capture these kind of things, that hopefully those test will get even better.

On the other hand. If I execute:

npm run start:dev

In a clean environment I get the error:

Cannot find module '@app/client' or its corresponding type declarations

That is caused because we are not including client/src/app/client/ anymore. While I see the reason behind ignoring that folder, seems like it is not actually solving a problem but creating one.

@carlosthe19916
Copy link
Copy Markdown
Collaborator

@gildub here is the PR I mentioned internally before #372
As mentioned the only dependency that required to be upgraded was @storybook/addon-webpack5-compiler-swc

Feel free to close my PR and take it just as an example. I don't want to step of your toes, just to share what I have discovered while playing with it.

Copy link
Copy Markdown
Collaborator

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

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

@gildub This is my environment

NodeJS v22.14.0
NPM v10.9.2

I checkout this PR. Then

npm ci --ignore-scripts

Then

npm run start:dev
Screencast.From.2025-02-25.15-38-23.mp4
  • The dev mode is broken
  • Clearly the ci pipeline fails for a reason

@gildub
Copy link
Copy Markdown
Contributor Author

gildub commented Feb 26, 2025

@carlosthe19916, this is ready to go, please confirm.

@carlosthe19916
Copy link
Copy Markdown
Collaborator

@carlosthe19916, this is ready to go, please confirm.

Give me some time please.

Copy link
Copy Markdown
Collaborator

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

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

LGTM!
We need to synchronize this important change here https://gitlab.cee.redhat.com/trustification/rhtpa-konflux-release.0.2.z/-/blob/main/Dockerfile.server?ref_type=heads#L4

@gildub could you take care of it please?

  • First the downstream image needs to be changed.
  • Only after it this PR can be merged otherwise we will destroy the downstream pipeline

@gildub gildub merged commit 6d4d5fb into guacsec:main Mar 3, 2025
8 checks passed
@gildub gildub deleted the bump-nodejs branch March 3, 2025 10:41
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.

Support lates nodejs LTS version 22

2 participants