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

Update to NodeJS v20.x (using a workaround) #688

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Dec 7, 2023

Context

  • Sort-of resolves Update app to run on Node 18 or newer (NodeJS 16 has reached End of Support) #682, with a workaround.
    • (Proper compatibility with NodeJS 17+, without resorting to a workaround, is still To-Do.)
  • Use the workaround flag --openssl-legacy-provider, via the NODE_OPTIONS env var, so we can bump to newer NodeJS (Node v20.x, specifically).
  • Note: NodeJS 18 was tested and working as well, but bumping to NodeJS 20 is more desirable, since it will be supported for longer.

Summary of Changes

  • Set the NODE_OPTIONS=--openssl-legacy-provider env var for development (Dockerfile) and production (app.json).
  • Bump to NodeJS 20 in development (Dockerfile) and production (engines field of package.json).

Checklist

  • Tested Mobile Responsiveness (and that app generally works in manual testing)
  • Added Unit Tests
  • CI Passes
  • Deploys to Heroku on test Correctly (Maintainers will handle)
  • Added Documentation (Service and Code when required)

Workaround for @rails/webpacker v5.x on NodeJS 17 or newer,
due to the OpenSSL upgrade in these newer versions of NodeJS,
along with webpacker using a deprecated hashing function.
With the "NODE_OPTIONS=--openssl-legacy-provider" workaround,
we are able to run on NodeJS 18 or 20 now.

Bump to the newest LTS we are compatible with.
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Dec 7, 2023

Other than the one known failure from before, CI is passing.

Manual testing worked fine, even for the functionality tested by the "failing" test case. Searching from the splash page (AKA homepage) "just works". Still not sure why that's broken in testing, since it does work in manual testing. But I'm inclined to ignore that apparently test-only failure if it allows getting us on newer dependencies/runtime versions.

And, for what it's worth, upgrading all outdated ruby gems with bundle update did not change the test results. All passing but the one failure that works in real life outside of the test suite.

Tried tweaking the tests slightly to see if it's an issue with the test logic, but I've personally had no luck with that yet, either.

@ice1080 ice1080 mentioned this pull request Dec 9, 2023
5 tasks
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Dec 11, 2023

If we can test this on a dyno on the main Heroku account for the project, that would add confidence in the Node 17+ workaround "just working" as I have it set in app.json without any fiddling with the Heroku dashboard.

If other maintainers can't help out with this, I'll be looking at paying $5 for an "eco dyno" on my personal Heroku account to see how it goes.

Would love to avoid paying Heroku money just for the privilege of testing the app, but that may just be how it is.

Will also let me run the Rails 7 upgrade PR through some paces on Heroku as well if I do that, though I'll note I haven't set up a postgresql DB in Heroku before, I don't think I'd know how, and that costs even more, so anything to do with actually hitting the DB won't work in my test dyno. That limits what I can really test with it, but at least it's something.

At minimum, I can confirm whether this workaround works. Success criterion for testing: The app's JS compiles successfully. (i.e. the app deploys to the dyno and doesn't go up in smoke, basically works.) The env var is only needed by Webpacker to compile, apparently. Or so I have heard. It may be possible to unset the env var after compiling, even. Anyway, this remains to be seen.

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.

Update app to run on Node 18 or newer (NodeJS 16 has reached End of Support)
1 participant