Describe required version of Node.js and NPM.

[adamretter]

One of the problems with JavaScript projects is when they don't document or enforce which version of Node.js or NPM is required to build them; without that it becomes a game of guessing and/or whack-a-mole.

This sets the version of Node.js and NPM in package.json to the same as that which was previously specified in the pom.xml - https://github.com/eXist-db/monex/blob/master/pom.xml#L58

The existing required version is of course very old. If it is possible to successfully build Monex with a newer version of Node.js and NPM, then I would suggest either:

1. Merging this first, and then sending a follow-up PR to update the version numbers, or
2. tell me the correct version numbers, and I will append a commit to this PR to update them too.

[marmoure]

One of the problems with JavaScript projects is when they don't document or enforce which version of Node.js or NPM is required to build them; without that it becomes a game of guessing and/or whack-a-mole.

Yes, we don't want this, and we should set the version

[duncdrum]

@adamretter as I have told you before you opened the issue, the next release of monex (targeting exist 7) will have a node version of 22. The version is set via maven, the engine stanza in package.json is unnecessary. As monex is not published as a node package and never will be.

[adamretter]

@duncdrum I'm sorry... but where did you tell me this before???

Also, I think you may be missing the point. If this supports Node 22, then we should describe that. The stanza is not about publishing, it's about helping devs build the package. @marmoure can you confirm if this works on 22?

[marmoure]

@adamretter yes I can confirm that it works on both v20 and v22.16.0

[adamretter]

@marmoure Okay, I have now appended a commit to document Node.js 22 as the required version.

[marmoure]

@adamretter Great , the versions in the range should work without any issues.

[reinhapa]

node version of 22. The version is set via maven, the engine stanza in package.json is unnecessary. As monex is not

@duncdrum the PR is changed accordingly, LGTM

[adamretter]

@reinhapa Would you mind also reviewing and approving if you are happy with it please?

[duncdrum]

@adamretter adding a node engine declaration to package.json that is not identical to the one in pom.xml increases the maintenance burden and introduces ambiguities.

Monex is not a node package, it is a pure consumer of node dependencies which are resolved and managed by maven. Any conflicts will appear as part of the maven build.

If we declare a compatibility with node 18.x we would need to test it, which this PR doesn't. Doing so would just increase overhead, for no benefit. The only node version of relevance is the one set in pom.xml.

If there is a scenario where the engine object would actually be helpful please let us know. Otherwise just take it out, and the PR is good to go.

[adamretter]

@duncdrum Thank you for the followup.

adding a node engine declaration to package.json that is not identical to the one in pom.xml increases the maintenance burden and introduces ambiguities.

Unfortunately, unless you want to set a single specific version of Node.js and NPM in the package.json it is technically not possible to make them identical as they serve two very different purposes. I am guessing you don't want to set a single specific version of Node.js in package.json. The two different purposes are:

1. The node.version and npm.version entries in pom.xml define the version of Node.js and NPM that should be installed solely for use by the frontend-maven-plugin. The default installation path is local and inside the Maven project, i.e.: ${basedir}/node. The plugin requires a single version number and cannot accept a range of versions, as it needs to know which exact version to download and install.

   From the documentation for frontend-maven-pluigin:

   1. Not meant to install Node for production uses.

   2. This plugin does not support already installed Node or npm versions

2. The engines.node and engines.npm properties in package.json define the version (or version ranges) of Node.js and NPM that your stuff works on. Because we have intentionally not set the [engine-strict](https://docs.npmjs.com/cli/v11/using-npm/config#engine-strict), this serves as documentation for both developers and users, and produces a friendly warning if they try and work with the project using a version of Node.js or NPM that is not known to work with the Monex code base.

   From the package.json specification:

   1. You can specify the version of node that your stuff works on:

   2. Unless the user has set the engine-strict config flag, this field is advisory only and will only produce warnings when your package is installed as a dependency.

Monex is not a node package, it is a pure consumer of node dependencies which are resolved and managed by maven

Yes, that is already clear to us. This PR is NOT about building a Node package from Monex.

If we declare a compatibility with node 18.x we would need to test it, which this PR doesn't

Prior to this PR you already had that explicitly, as frontend-maven-pluigin was configured to download and build Monex using Node v18.12.1, see: https://github.com/eXist-db/monex/blob/master/pom.xml#L58. It has been that way since December 2022 where you introduced that change (see: 93ac7e9), so I think we can all agree it was tested by you before you committed the change, and given the time that has passed, it can be considered thoroughly tested by now.

The only node version of relevance is the one set in pom.xml.

For users who already have a global Node.js and NPM (as many of us do), being able to correctly use local Node tools, without first having to spend time to divine which versions are required to work with Monex (without frontend-maven-pluigin) is hugely valuable. Documenting this in package.json enables this.

---

I hope this detailed explanation with links to docs and specs has now made the value of its changes clear. If you have any further questions please let us know...

[line-o]

I completely agree with @duncdrum review. The engine should not be in the package.json as it is not setting the version but the pom file sets it. Having it in two places will likely introduce an issue in the future.

[adamretter]

The engine should not be in the package.json as it is not setting the version but the pom file sets it

The pom.xml setting does not set the required version of the engine. @line-o As detailed above It serves an entirely different and separate purpose.

[line-o]

@adamretter yes both serve a different purpose. Here we only need the node version for building with maven. Thus, only the setting in the pom is relevant.

[adamretter]

Here we only need the node version for building with maven

What about when you want to manage and update Node.js dependencies with npm @line-o? You can't do that from Maven at the moment!

[duncdrum]

@adamretter you do so from the node version that is specified in the pom, as this is the node version required by the build.

[adamretter]

@duncdrum in practice that doesn't work, without the engine setting, you have no idea what version of node is required to perform tasks with Node (outside of Maven), e.g. finding compatible Node Module version updates

[line-o]

Dependabot does not care about the stated node versions. Which external tooling do you have in mind @adamretter?

[adamretter]

@duncdrum @line-o Here is a concrete example from today of why this PR is important:

1. You want to update Cypress in this project from version 9 to version 10.
2. You edit the package.json to change "cypress": "^9.7.0" to "cypress": "^10.11.0"
3. You some how need to install the new Node module for Cypress, you have two options:
   1. You run npm install, but this doesn't work as your system's installed version of NPM is not compatible with the older version implicitly expected in Monex. That would be fixed by this PR!
   2. Anyway, so instead, you run: mvn frontend:npm -Dfrontend.npm.arguments="install". Okay that works, we can keep going...
4. You try to run the integration tests (i.e. mvn verify), you get the error:

   [INFO] --- exec:3.5.1:exec (cypress-without-record) @ monex ---
   There is a cypress.json file at the path: /Users/aretter/code/monex
   
   Cypress version 10.0.0 no longer supports cypress.json.
   
   Please run cypress open to launch the migration tool to migrate to cypress.config.{js,ts,mjs,cjs}.
   
   https://on.cypress.io/migration-guide
   
   [ERROR] Command execution failed.
   

5. Well the above is quite clear, we need to run cypress open to launch the migration tool.
6. We can't run npm run cypress open directly because of 3.1...
7. So instead, we try and do something again with the maven-frontend-plugin. We run: mvn frontend:npx -Dfrontend.npx.arguments="cypress open"
8. Ok we got there!

The issue is that the way to run npm install or cypress open (or any other command that is not already pre-configured by us in the pom.xml) via the maven-frontend-plugin is completely undocumented - I had to read the Java code of the maven-frontend-plugin to find the necessary -D properties to set.
It seems very silly, that we can't merge this PR and then just use npm directly here (when we need to as developers for the tasks it is best suited to).

[adamretter]

Dependabot does not care about the stated node versions. Which external tooling do you have in mind @adamretter?

@line-o Sorry, perhaps I wasn't very clear. I wasn't talking about Dependabot there. I was talking about npm and npx