codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
24 stars 24 forks source link

[No Issue] update node version docs #532

Closed timbot1789 closed 7 months ago

timbot1789 commented 7 months ago

Scott found an issue with our unit tests running node version 16.4. We don't want to be supporting several different versions of node. However, our docs say we support any version of Node 16. We should instead peg our project to the most recent LTS release of Node 16, which is node 16.20.2. I've updated docs to show that.


andycwilliams commented 7 months ago

Just for my own understanding, do points 2 and 3 pose significant issues going forward? Is the plan just to stick with Node 16 and hope Inrupt updates?

Frankly, I'm not sure whether committing to an older version of Node is a hindrance or not.

timbot1789 commented 7 months ago

Just for my own understanding, do points 2 and 3 pose significant issues going forward? Is the plan just to stick with Node 16 and hope Inrupt updates?

Frankly, I'm not sure whether committing to an older version of Node is a hindrance or not.

For a volunteer project that doesn't have a production release, and is using node simply to deploy a react app, it's not a big problem. The main concern in using an unsupported node version is a data breach. But our user data isn't on the node server, it's on the solid server, so there's not much to breach.

If this had a production release, the unsupported node server would be an issue.

leekahung commented 7 months ago

Just checked in a local branch for Development with node 18 (18.18.2) and node 20 (20.9.0).

While I'm probably stabbing in the dark here, but I've ran npm run test with the newer node versions just to see if it'll break any of our limited tests. With v16 ran all tests just fine. With v18 and v20 breaks tests related to webcrypto which is imported from a 'crypto' module.

The major different between the v16 and newer versions is that "oidc-provider@7.10.6" is unsupported.

Update: Just checked they seem unrelated. But it's interesting to see the different node versions running the same unit tests with v20 breaking the unit tests related to crypto:

Screen Shot 2023-11-19 at 19 16 11

and v18 and v16 passes all the tests:

Screen Shot 2023-11-19 at 19 18 34
timbot1789 commented 7 months ago

Just checked in a local branch for Development with node 18 (18.18.2) and node 20 (20.9.0).

~While I'm probably stabbing in the dark here, but I've ran npm run test with the newer node versions just to see if it'll break any of our limited tests. With v16 ran all tests just fine. With v18 and v20 breaks tests related to webcrypto which is imported from a 'crypto' module.~

The major different between the v16 and newer versions is that "oidc-provider@7.10.6" is unsupported.

Update: Just checked they seem unrelated. But it's interesting to see the different node versions running the same unit tests with v20 breaking the unit tests related to crypto:

Screen Shot 2023-11-19 at 19 16 11

and v18 and v16 passes all the tests:

Screen Shot 2023-11-19 at 19 18 34

Yeah oidc-provider isn't related to webcrypto. Odd that webcrypto breaks support though. That's a node library (not a dependency). That's dead code anyway, though. I'll remove it.

And I think you're right that we're probably fine running the app in a higher version of node. The warning message is just annoying.