engine versions not correctly identified

[marcus-j-davies]

We see this.

Despite having this.
https://github.com/zwave-js/node-red-contrib-zwave-js/blob/9f251b1fe5fc26f1d4e17a55fc5d8cbc02c10794/package.json#L20

Output.

[sammachin]

The test is functioning correctly the package is requiring a version greater than the minimum supported node-red version so it highlights this to users as a warning.

However the text on the webpage should be improved not juts to default to "missing"

[knolleary]

Aside from how it's presented on the scorecard webpage, I think we should modify the wording of P07. I think saying its minimum version is 'not compatible' doesn't quite express what we want it to.

Something like:

Module requires newer version of Node.js than minimum supported by Node-RED

[marcus-j-davies]

Right,

Yes, having it be marked as missing, is quite different from having a version mismatch.
I would also ask the question if this is really the best way.

nminversion = semver.minVersion(response.data.versions[response.data["dist-tags"].latest].engines.node)
if  (semver.satisfies(nminversion, package.engines.node)){
     ...
}

IMO, wouldn't this be better?
Surely a module that is capable of running on a lower node version that Node RED can is more of a problem?

nminversion = semver.minVersion(response.data.versions[response.data["dist-tags"].latest].engines.node)
if  (semver.gte(package.engines.node, nminversion)){
     ...
}

not really sure of the right answer here 😅

[knolleary]

Surely a module that is capable of running on a lower node version that Node RED can is more of a problem?

I disagree. If a Node can run on 8.x or later and the user has Node 14.x then there is no issue.

But if a Node requires 16.x and the User has Node 12.x then there is an issue.

This is something we know we need to police better in the palette manager, but the purpose of the test is good - we just need to fix how it is presented.

[marcus-j-davies]

Ah I see now.
The fact Node RED is installed and running, means they would have settled the min node version for Node RED 🤦‍♂️

[sammachin]

I've made a partial fix for this issue in the above PR, we will now display the version if its available, however it will still show a warning ! if the requirements of the node are greater than the current Min node-red version.

Ideally there is some more work to do as we should show a Fail X if the node requires a version less than the current node-red version, eg if its pinned to 10.x or something.
However this will require more thought so leaving this issue open to revisit.

[marcus-j-davies]

Yup,

we should show a Fail X if the node requires a version less than the current node-red version

I think this is what I was trying to get at with my suggestion above - just didn't articulate it well.