HomeUniteUs icon indicating copy to clipboard operation
HomeUniteUs copied to clipboard

Upgrade node2023

Open mira-kine opened this issue 1 year ago • 2 comments

Closes #588

What changes did you make?

  • Followed the general solutions plan on the issues page, except I upgraded to Node 20.x instead of Node 18.x.
  • I created a pre-install check to enforce a minimum node version (20.x), set in the engine's section of package.json. When a new dev installs the node packages, a script will run to check the node version first per check-node-version.mjs. If the minimum node version is not met, the installation will fail and a console log error will show up requiring the current node version to be satisfied.
  • Added that minimum node version is enforced and how it works in README

Rationale behind the changes?

  • The docker image currently runs on the latest version of node (as of October 18th, 2023, that is Node 20.x), so there was not as much to change there. The github work flow script is set to the lts of node instead of being specified as 20.x because we are always aiming to use the lts.
  • For enforcing the minimum node version, I set it to >=20.0.0 because Node 20.x active support ends in 11 months and security support ends in 2 years according to this schedule. Therefore I chose the lowest LTS version that will give us the longest time before security support ends.
  • As for setting esModuleInterop in tsconfig.json as "true": EsNext module does not currently have a way to support package.json exports, which was necessary for my pre-install check file check-node-version.mjs. To make the least amount of changes, I chose to set esModuleInterop to true and change the file extension to mjs in order for the json import to work. I also used Node's createRequire to mimic CommonJS "require" statement, which allows package.json imports.

Testing done for these changes

  • I ran the build + ran the tests + manually tested the frontend before and after the upgrade
  • I changed the node version to a ridiculous number aka "node": ">=50.0.0" and ran npm i to see if I would receive the expected error

What did you learn or can share that is new?(optional)

  • I learned a lot about modules and Typescript compiler options regarding modules. I learned that TS needs to check that: 1. files are compiled into valid output module formats, 2. ensure that imports in the outputs (aka what is compiled out to runtime) will resolve successfully, and 3. what the type of the imports are. Other ways the package.json import could have worked was to change the moduleResolution to nodenext instead as a typescript compiler option, as NodeNext makes CJS and ESM coexistable, where as simple Node is meant only for cjs. Since we are using ESM however, the clash of interests made the package.json import harder than expected. I also could have used ts-node but I didn't want to keep adding dependencies for only one file.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals after changes are applied, testing node minimum version

image

mira-kine avatar Nov 08 '23 00:11 mira-kine

Hi @mira-kine ! This looks all good. I wrote some comments/questions - take a look!

Aside from that, just merge the main branch into your branch and resolve the merge conflicts, then see if it starts up ok in your environment.

app starts up just fine btw!

image

agosmou avatar Nov 22 '23 06:11 agosmou

@mira-kine It looks like eslint-plugin-prettier got installed at the root of the project, creating a package.json, package-lock.json, and node_modules folder. Maybe you accidentally npm installed at the root instead of the /app directory?

There are also acouple eslint disables comments. Can these be resolved? I'd like to avoid putting these in the codebase as much as possible.

erikguntner avatar Feb 06 '24 22:02 erikguntner