[HOLD for payment 2023-11-29] Bump E/App and Onyx to use NodeJS 20.9.0 LTS

[fabioh8010]

Problem

Both E/App and Onyx repos currently enforces developers to use NodeJS 16.15.1. As of September, NodeJS 16.x will reach its End-of-life (EOL), meaning there will be no more updates, patches, or security fixes. Additionally, the future React Native 0.73 version will require using NodeJS 18.x.

Using an outdated version of NodeJS not only exposes the application to potential security vulnerabilities, compatibility issues, and prevents developers from leveraging the latest features, optimizations, and improvements available in newer versions of NodeJS, but it will also block us from updating the App to RN 0.73 or higher versions.

Solution

Upgrade both repos to require NodeJS 18.17.1 (or 18.x) and NPM 9.6.7. This will ensure that the application remains secure, compliant, in line with best practices for software development and will allows us to continue updating our app to latest RN versions.

Plan

For react-native-onyx:

1. Change package.json to require NodeJS 18.17.1 and NPM 9.6.7.
   1. Alternatively, since this is a open-source library we could relax the version enforcement to only 18.x, in this way developers/companies who uses Onyx don't need to use a strict version, just the LTS release.
2. Change .nvmrc to require 18.17.1 (or v18).
3. Change publish.yml, test.yml and lint.yml to use the node version from .nvmrc file instead of hardcoded version.

For E/App:

1. Change package.json to require NodeJS 18.17.1 and NPM 9.6.7.
2. Change .nvmrc to require 18.17.1.
3. Change TestSpec.yml to use .nvmrc (if possible) or 18.17.1.
4. Fix build-desktop.sh to drop npm bin usage since it was removed in NPM 9.x, and use npx instead.

Tests

I forked Onyx to use 18.17.1 and used my own release in E/APP in order to test the changes and I was able to install the dependencies and build for all platforms. When running tests I got some dependency errors about node version mismatched in installed packages, was able to resolve the issues by running npm rebuild (probably just removing node_modules and reinstalling would work as well).

I also didn't have any issues with dependencies auto-updating or changing package-lock.json.

[melvin-bot]

Triggered auto assignment to @kevinksullivan (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[fabioh8010]

@AndrewGable @roryabraham Based on this idea posted here: https://expensify.slack.com/archives/C01GTK53T8Q/p1692277812800739

[AndrewGable]

Looks great!

[Tony-MK]

Hey, I would love to work on the issue. Can I get assigned? Do I need to post proposal?

[AndrewGable]

@fabioh8010 is assigned and will be working on this one. 👍

[fabioh8010]

@AndrewGable

@pac-guerreiro Will take the issue over! Could you assign him?

[roryabraham]

On HOLD until after the merge freeze

[AndrewGable]

@fabioh8010 - Can you have him comment here? Thanks.

[pac-guerreiro]

Hi @AndrewGable, I can take this 😄

[roryabraham]

BTW, I think this issue is a bit lower urgency, so due of the backlog of issues that we have been HOLDing the last couple weeks, I suggest that we wait until a few days after the merge freeze is lifted to take this off HOLD. That way we don't have a huge amount of PRs merged all on the same day and end up with a massive release that's very hard to ship

[kevinksullivan]

What's the latest here @pac-guerreiro ?

[pac-guerreiro]

@kevinksullivan still waiting for my PR to be merged with Onyx. After it, it should be safe to merge my PR to Expensify App 😄

[pac-guerreiro]

Onyx is now using Node 18!

I'm going to create a PR to bump E/App to Node 18!

[pac-guerreiro]

@dylanexpensify @roryabraham The PR for Onyx is now open! The one for the app will remain in draft until the Onyx one is merged because until this happens, the app will throw compatibility errors with Onyx because of node version

[pac-guerreiro]

@DylanDylann @roryabraham

I addressed the feedback on the PR for Onyx! Let me know if there is anything else that needs to be taken care of and sorry for the time this has been taking to be merged.

[roryabraham]

This is done now, just awaiting deploy to production (even though it mostly affects dev/test environments)

[AndrewGable]

@roryabraham - Can you send an action required email to internal engineers on this?

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.1-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• chore(node): bump node version to 20.9.0 #30645
• [No QA] chore(node): bump version to 20.9.0 #31460
• [No QA] Use Node 20 for all JavaScript GitHub Actions #31501

If no regressions arise, payment will be issued on 2023-11-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @pac-guerreiro does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@pac-guerreiro] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@kevinksullivan] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[roryabraham]

Looks like none of the PRs underwent C+ review, so we can close this.

[roryabraham]

No regression tests needed

[roryabraham]

Can you send an action required email to internal engineers on this?

I think @arosiclair sent one