Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support node.engines #618

Closed
adrianmcli opened this issue Apr 25, 2020 · 15 comments
Closed

Support node.engines #618

adrianmcli opened this issue Apr 25, 2020 · 15 comments
Assignees

Comments

@adrianmcli
Copy link

adrianmcli commented Apr 25, 2020

Many projects use the engines.node property in the package.json file to denote the version of Node.js to be used for that specific project. Although it isn't a required specification, it is ubiquitous enough that there are serious discussions around supporting it in competing tools (see below).

Also, some additional context from #547.

Prior discussions

Precedent

ps-nvm already supports this: https://github.com/aaronpowell/ps-nvm#packagejson-enginesnode

Potential issues

engines.node is a challenge because n is implemented as a shell script, and parsing json is non-trivial

From: #547 (comment)

@shadowspawn
Copy link
Collaborator

(Thanks for the multiple links, much appreciated.)

@shadowspawn
Copy link
Collaborator

shadowspawn commented Apr 26, 2020

Proof of concept of an approach using node and semver, albeit quite limiting as needs a node and npx available:

range="$(node -e "console.log(require('./package.json').engines.node)")"
version_per_line="$(N_MAX_REMOTE_MATCHES =999 n lsr)"
versions_one_line=$(echo ${version_per_line} | tr '\n' ' ')
npx --quiet semver -r "${range}" ${versions_one_line} | tail -n 1

@shadowspawn
Copy link
Collaborator

Related older issue: #192

@adrianmcli
Copy link
Author

needs a node and npx available

Yea I can see this is non-trivial without pulling in all sorts of dependencies, or alternatively, significantly inflating the code. That being said, if it's a (relatively modern) Node project shouldn't node and npx already be available?

I've never worked with JSON via Shell/Bash so I don't think I can be of much help here unfortunately. The first thing I would reach for is jq, but obviously that's even worse than the PoC above.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Apr 27, 2020

That being said, if it's a (relatively modern) Node project shouldn't node and npx already be available?

I see two main cases when this won't be true. Firstly, there won't be any node when bootstrapping and using n to install the first/only version of node, especially in a Docker container.

The other case is an old version of node, such as when the node.engines field in the previous project took node back to some not-relatively-modern version. A work-around would be to not install such old versions but use them explicitly in scripts, like say n run engine build or n exec engine run-all-tests.

(A possible crude but simple work-around for implementation is to install an extra version of node if needed, to perform the check!)

@shadowspawn
Copy link
Collaborator

shadowspawn commented Apr 27, 2020

A question for interested readers, post a reaction to show your view.

Is engines support that only works when there is (say) a version of node >= 6 available going to be useful in practice?

👍 Sure, I am just switching between modern versions
👎 Not good enough, I want it to just work, including when none or old node

@shadowspawn shadowspawn self-assigned this Jun 22, 2020
This was referenced Jun 24, 2020
@jasonkarns
Copy link

nodenv has a plugin that respects the engines stanza in package.json: https://github.com/nodenv/nodenv-package-json-engine

For JSON and semver support in POSIX shell, it leverages https://github.com/dominictarr/JSON.sh for parsing the package.json and https://github.com/qzb/sh-semver for the semver matching.

Each of these are functional, but the plugin is certainly much slower than the standard mechanism for specifying a nodenv version (.node-version file). For most use cases, this performance overhead isn't an issue, because the version resolution occurs at the moment a user invokes node or npm (via the shims). However, for users who prefer to invoke nodenv in order to display the activated node version in their shell prompt; the performance delay is noticeable and painful. (It's worth noting that the plugin is only consulted as a fallback when a nodenv version isn't already set via $NODENV_VERSION or .node-version; ergo, the performance penalty does not apply in all projects.)

@shadowspawn
Copy link
Collaborator

Thanks @jasonkarns , interesting information and links

@shadowspawn
Copy link
Collaborator

Heroku have a semver api for node which is an interesting alternative, but the backing project has been archived: https://semver.io

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jun 27, 2020

Can avoid invoking full semver for the simpler expressions that map onto supported n syntax. This might cover many of the practical uses of engine. I don't want to go overboard on the hand parsing, but I like the idea of avoiding the full semver treatment when not needed. e.g.

  • v8.1.2 => v8.1.2
  • =10.2.3 => 10.2.3
  • 10.x => 10
  • ~10.2 => 10.2
  • ^10.2 => 10
  • >=4.0.0 => current

https://docs.npmjs.com/misc/semver

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jun 28, 2020

I have a first cut at implementation and reasonably happy, noisy progress so can tell what is going on ("resolving" is using npx and semver and a slow step):

$ nvh install auto
       found : package.json
        read : >8 <10
   resolving : >8 <10
   installed : v9.11.2 (with npm 5.6.0)

$ # (change package.json)

$ nvh install auto
       found : package.json
        read : ^10.0.0
   installed : v10.21.0 (with npm 6.14.4)

@jasonkarns
Copy link

Can avoid invoking full semver for the simpler expressions that map onto supported n syntax.

Oh, I like that idea. Would be a nice improvement to the node version plugin as well; to short circuit and avoid the semver evaluation when possible.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jul 4, 2020

I have released support in nvh first.

Give it a try if you are keen! Comments welcome here in this thread, or open an issue there.

e.g. nvh install auto

@shadowspawn
Copy link
Collaborator

Released in v6.7.0

@shadowspawn
Copy link
Collaborator

The current implementation with package.json support rolled into auto has drawbacks for some use cases. I am interested in feedback on splitting out the package.json support from the more specific node version files. See #640 to add a reaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants