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 Node Version #68

Merged
merged 5 commits into from
Feb 1, 2023
Merged

Upgrade Node Version #68

merged 5 commits into from
Feb 1, 2023

Conversation

confused-Techie
Copy link
Member

Requirements

  • Filling out the template is required.

  • All new code requires tests to ensure against regressions.

    • However, if your PR contains zero code changes, feel free to select the checkmark below to indicate so.
  • Have you ran tests against this code?

  • This PR contains zero code changes.

Description of the Change

In this PR I've upgraded to the latest version of Node Supported by Google App Engine (Which is the service we run the backend on).

After upgrading my local Node to Node v18.13.0 I went ahead and ran all test suites of the code, and amazingly all tests pass without any modification.

Where we saw updates:

  • GitHub Actions have all added 18.x as a Node Version to run tests on. The other versions are still listed out of curiosity if we can support any other versions. But may be removed in the future if needed. Where we can officially say that version is no longer supported.
  • The app.yaml is now set to using Node 18. As well as the app.example.yaml
  • I've updated the engines field in the package.json
  • Lastly I've added .nvmrc so there no longer needs to be any confusion as to what version of Node the backend runs. The version will always be available in this file, and will be kept up to date as needed.

@Digitalone1 take a look and see if we can merge this into #65 or #66

@confused-Techie
Copy link
Member Author

Alright, as you can see I played around with seeing the difficulty of supporting multiple versions of NodeJS.

So with this the engines field in our package.json is 100% accurate into the exact versions of NodeJS we support. And can be used for future reference.

I know you originally wanted this to work with the newest features, so if you need to on your PR feel free to remove support for NodeJS 16 if needed. Since we control the environment this runs in, it's not the biggest deal if we only support a single version, just thought it'd be cool to support more.

@Digitalone1
Copy link

@confused-Techie sorry for the late reply, we use structuredClone which was introduced in Node v17, so the minimum version should be 17.

@Digitalone1
Copy link

@confused-Techie you can add v17 and v19. I tested on 19 locally, all passed.

@confused-Techie
Copy link
Member Author

@confused-Techie you can add v17 and v19. I tested on 19 locally, all passed.

Noted, I'll fix that. Although will have to check if GitHub Actions supports those other Node versions. If it doesn't I'd rather only support what we are able to test on each PR just to make sure our engines is accurate.

But will do some additional testing.

@confused-Techie
Copy link
Member Author

@Digitalone1 Alright, I've gone ahead and updated this one. You can pull it into your PR if you'd like, but otherwise this ones good to go.

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