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

Upgrade to Node v22 #1047

Merged
merged 17 commits into from
Dec 10, 2024
Merged

Upgrade to Node v22 #1047

merged 17 commits into from
Dec 10, 2024

Conversation

meissadia
Copy link
Contributor

@meissadia meissadia commented Nov 6, 2024

Close #1040

Changes

  • [e2e] Update tests for View Institution Profile that were missed as part of [View Institution profile] Update language #1013
  • [fix] [View Institution Profile] Display alert when sbl_institution_types is null or []
  • Updated .nvmrc to use v22
  • Updated package.json -> engines to use Node v22
  • Update Github actions to use Node v22
  • Updated Dockerfile to use new node-js-alpine:3.20 base image

How to test this PR

  1. Pull branch
  2. nvm use
  3. yarn start to easily launch all backend services
  4. yarn docker-build-and-run to run the container version of the Frontend
  5. Update your .env file to run tests against the container version
    SBL_PLAYWRIGHT_TEST_TARGET="http://localhost:8085"
    
  6. yarn playwright test --workers 4
  7. Verify all tests pass

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

Awesome!

@billhimmelsbach billhimmelsbach self-requested a review November 8, 2024 17:10
Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

Actually, could you update the package.json required version as well?

EDIT: Hmm, we'll also want to update the Dockerfile as well.

EDIT 2: We'll also want to update the node version in the image. I'll give ya a call about that one.

@meissadia
Copy link
Contributor Author

meissadia commented Nov 8, 2024

Hmm, we'll also want to update the Dockerfile as well.

We'll also want to update the node version in the image.

@billhimmelsbach

  • Update package.json
  • Update Dockerfile
    • Changing this to anything other than node18 (node20, node21, node22) fails.
    • I've searched for other examples of this type of transformation, searched for a list of valid values for this -t <VERSION> argument, but I'm not finding anything that helps clarify my confusion.
    • Screenshot 2024-11-08 at 12 59 12 PM
  • Update image
    • I'm guessing this would be in the source image this container is based on? So will try to find that once I regain access.

@meissadia
Copy link
Contributor Author

meissadia commented Nov 13, 2024

@billhimmelsbach I still don't know what to do about the build of import-meta-env but things seem to be working as-is.

It looks like the latest version of Node supported by pkg is v18 and the source repo is archived as of the start of 2024 so no further updates.
https://www.npmjs.com/package/pkg-fetch#binary-compatibility

tanner-ricks
tanner-ricks previously approved these changes Dec 6, 2024
@meissadia meissadia enabled auto-merge (squash) December 10, 2024 18:14
@meissadia meissadia merged commit 1143bfe into main Dec 10, 2024
10 of 11 checks passed
@meissadia meissadia deleted the 1040-node-22 branch December 10, 2024 18:26
billhimmelsbach added a commit that referenced this pull request Dec 10, 2024
billhimmelsbach added a commit that referenced this pull request Dec 10, 2024
## Changes

- Reverts [update to Node
22](#1047) which was causing
this error:
<img width="1092" alt="Screenshot 2024-12-10 at 11 11 11 AM"
src="https://github.com/user-attachments/assets/41a25d50-19a7-4045-be20-2b15928a0fac">


## How to test this PR

1. Are the e2e tests passing again?

## Screenshots
<img width="1012" alt="Screenshot 2024-12-10 at 11 12 32 AM"
src="https://github.com/user-attachments/assets/8ef31484-4e52-45ec-97e2-50b1047dbe45">
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.

Upgrade to Node 22
3 participants