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

Use package.json engines version? #60

Closed
tomdavidson opened this issue Oct 23, 2017 · 9 comments
Closed

Use package.json engines version? #60

tomdavidson opened this issue Oct 23, 2017 · 9 comments

Comments

@tomdavidson
Copy link

Thoughts on reading the engines object from package.json to select the correct nodejs version?

nvm-sh/nvm#651

@MrQubo
Copy link

MrQubo commented Sep 18, 2018

If we have both package.json and .tool-versions in the same directory it's obvious that we should choose the version in .tool-versions.

But what to do if .tool-versions is in a parent directory relative to the package.json?
e.g.

foo/
├── .tool-versions
└── bar/
   └── package.json

Also, take into an account that the version inside the package.json might be actually a range of versions.

Would you rather install the version from engines after using node or npm for the first time inside the project or use the version from engines only if it is already installed?

@dboune
Copy link

dboune commented Jun 26, 2019

Replying to @MrQubo's questions with an opinion.

.tool-versions should take precedence.

In a subdirectory, any legacy file, or .tool-versions if available, should take precedence for that directory.

For a range of versions, the latest version allowable, and installed, should be used, if available.

The last question seems to point to automatic installation, which I do not believe is reasonable. If a specified version is not available, that would be an error.

@Stratus3D
Copy link
Member

After looking at this a bit more, I don't think we should add support for this. Adding support for this would require a lot of code. Here is the PR for nvm (which isn't going to be merged) - nvm-sh/nvm#1535. It is over 1200 lines of code.

I think my biggest objection to this is that it doesn't make sense semantically.

I don't think the feature makes sense semantically - in other words, that the package.json "minimum supported version range" field makes sense as a "run it under this one single version of node/io.js"
-- @ljharb

package.json specifies a version or range of versions that the package is compatible with, whereas .tool-versions and .node-version specify a single version that must be used. We'd have to implement our own logic for choosing the highest or lowest version for the package, or maybe prefer a version the user already has installed. That gets very complicated. I think we should just stick with .tool-versions and .node-version.

@Stratus3D
Copy link
Member

Here is a PR that attempts to add support for package.json - #120. The problem with this PR is that the engine value isn't a plain version that can be used by asdf-nodejs. We would have to address all the issues nvm would have to address on their PR.

@Stratus3D
Copy link
Member

Closing since I don't think this makes sense as a feature. Please comment if you think I should re-open this ticket.

@dkniffin
Copy link

dkniffin commented Jan 3, 2022

@Stratus3D I would really like to see support for engines in asdf. I understand the complexity involved in supporting the semver expressions though.

What if we compromised and asdf supported strings that have a specific version without semver syntax? So if it had 16.x, it'd get ignored like it does currently. But if it had 16.13.1, it'd install that specific version.

Thoughts?

@Stratus3D
Copy link
Member

@augustobmoura what do you think about that? I suppose it wouldn't really break asdf-nodejs semantics, but would it be worth the hassle to implement?

@ProfessorManhattan
Copy link

ProfessorManhattan commented Jan 10, 2022

+1 I would love to see this feature in asdf. There are already so many files that need to be in the root of a project and package.json already officially has a field that designates what version should be used. Following DRY, we should use package.json.

Fringe Use Benefit: I imagine this feature would also be useful in some cases where the latest version of Node causes a break -- with the ">=14.0.0" specified in package.json, this would force developers to have their code tested with the latest version before sending it off to a public registry.

In regard to @Stratus3D 's comment about nvm's implementation being over 1200 lines -- most of the code is test code/mock data for the tests. The actual bash script logic is about 300 lines in the nvm.sh file.

If we brought this feature to asdf, we could probably recycle the code from the nvm commit.

I don't have the bandwidth to pick this up currently plus I'm not in active communication with the maintainer but I'd be willing to fund the PR by giving an NFT of a girl's fart in a jar to see this get done (Source: https://news.bitcoin.com/wind-breaking-nfts-reality-star-who-made-200k-selling-farts-in-mason-jars-launches-nft-collection/)

@jpolvora
Copy link

jpolvora commented Apr 2, 2022

I just came here to suggest this feature to be implemented, but before opening a new issue, I've searched and found this issue already opened.

I think this feature would be very welcome. Should be disabled by default and explicitly enabled through configuration $HOME/.asdfrc. In case the developer wants to use this feature, he must to be aware of the effects that it may cause.

Is there a way to print an alert message in terminal when the directory is navigated to ?

Another observation is that the implementation of this feature should be simpler as possible, nothing more than check if exists a package.json file, parse it as JSON, check the presence of the "engines.node" field filled, validate it with a regular expression, and resolve the version, keeping the behavior aligned as if the version was read from .toolversion.

My suggestion to solve the question raised about the engines.node field allowing the semver format which in some cases may not be compatible with asdf, the field must be validated using the regular expression found on the official website https://semver.org, and in case of failure, ignore and use the default strategy. It should also be clear in the documentation that in order to use package.json as a source of information for the node.js version, the engines.node field must be in a format compatible with the format expected by asdf. Otherwise, it makes no sense to use package.json as a replacement for .toolversions.

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

7 participants