[BUG] Version with v-prefix is incorrectly marked as valid

[LouisMT]

According to SemVer 2.0.0, a version like v1.2.3 is not valid (https://semver.org/#is-v123-a-semantic-version).

The library incorrectly marks a version like this as valid:

const semverValid = require('semver/functions/valid')

semverValid('v1.2.3')
// true

semverValid('v1.2.3', {loose: false})
// true

I understand that the v-prefix is often used to indicate that some string represents a version. However, when I explicitly want to check if a string is valid according to the SemVer spec, I expect the method to return false in this case, especially when I set the loose option to false.

A fix (or an option to be extra strict for backwards compatibility) would be appreciated!

[lukekarrys]

As a "fix" this would be a breaking change, so I would be against that. But I do think there is room for a feature that would allow for a more strict parsing according to the spec.

[ljharb]

The npm ecosystem doesn’t follow semver 2.0 - it follows 1.0, i believe - because version changes under v1 aren’t always breaking.

That said, i don’t think the v has ever been included according to the spec, but also, in the npm ecosystem it’s very often there. I don’t think there’d be any value in rejecting these.

[riderx]

@ljharb that not what is writed in the readme:
A "version" is described by the v2.0.0 specification found at https://semver.org/.
As @LouisMT i expected the validation to return false.
Especially when that is written in the readme one line after A leading "=" or "v" character is stripped off and ignored.

[ljharb]

Then i guess the docs are wrong.

[riderx]

@ljharb that what you believe, to me this package follow v2.0.0
According to this comment the npm ecosystem does too.
At least for Angular, react, node, typescript, svelte, nuxt, next, and most of the bigger packages.
it's seemed to me fairly legit to include this check in the validation by default in the next MAJOR version, and optional in the current one.

[ljharb]

That said, i don’t think the v has ever been included according to the spec, but also, in the npm ecosystem it’s very often there. I don’t think there’d be any value in rejecting these.

[riderx]

I see value in rejecting thoses.
Simple example:
I check SemVer in node ecosystem after these versions are uses in mobile (swift and android) where SemVer is implemented correctly and get refused.
SemVer should be implemented as the RFC for interoperability, not for NPM ecosystem.

[ljharb]

this package is explicitly and only for npm. it's not "the official semver package", it's "the npm ecosystem's official semver package"

[riderx]

This package is download more than 200,921,654 a week, there are a lot of chance that is used for standar SemVer and for NPM SemVer.
Implementation in Android and Swift have the name SemVer and implement the RFC not a language specific one.
if it's not "the official semver package", it could use a specific namespace like @npm/semver

[ljharb]

Everything on npm is node-specific by default, no scope is needed.

If you want something that can work in Swift, there should be a Swift package for it - which there is (https://github.com/ddddxxx/Semver, for example).

[riderx]

I agree for all packages who are 100% node-specific.
Sadly, versioning is in use everywhere.
Semver package is a parser for something who is notnode-specific.

Any parser (HTML, CSS, MARKDOWN) in NPM follow the RFC as much as they can.

We try to stop having browsers interpreting the JS differently, and follow the RFC for interoperability.
Package should do it too when RFC exist.

[ljharb]

Browsers have nothing to do with this - this package is for npm's semver implementation only. If you're using it for non-npm semver, you're misusing it.

[riderx]

I'm talking about interoperability and use example like Browsers and JS story to find a common ground where we can understand each others.

I use this package in a CLI who is coded in JS, but made for other usage than node.

I misusing it for sure, the namespace, the README let me think it was a generic package following a generic convention.

If you believe i'm the only one who going to think that, you're missing some perspectives. :/

[ljharb]

The repo is named “npm/node-semver” and it’s a node package published on npm, I’m not sure how much clearer it could be ¯\_(ツ)_/¯

[riderx]

Agree for GitHub, but that out of the problem.

On NPM, it's just semver, that specific common name meansemantic versionning.

A package who not follow the RFC of a subject shouldn't use the common name.

And if then in the current situation, a big warning should be in it
It is a special node implementation for NPM the RFC is not guarantee

[riderx]

i openned PR to add it to the README #476

[ljharb]

It's on npm. Everything on npm has the implicit context "the node ecosystem".

[riderx]

I can say the same when it's wrote Semver it has the implicit context of SemVer RFC.

By saying that we go nowhere…
I'm sorry to bother you with this topic, i was hoping to have a comprehensive conversation where we all tried to understand each other.

[silverwind]

I do agree that a "strict" mode that actually follows the spec would be nice. As seen in npm/cli#6370, the currently sloppy parsing already causes issues for npm itself.

[isaacs]

The npm ecosystem doesn’t follow semver 2.0 - it follows 1.0, i believe - because version changes under v1 aren’t always breaking.

@ljharb This is only mostly correct.

The npm ecosystem started with SemVer 1.0, which was a much less clear and precisely worded spec. SemVer 2.0 brought a lot more rigor, but the npm ecosystem already had a fairly large corpus of version numbers that were only compliant with (a particular interpretation of) SemVer 1.0. So, it has "strict mode" (used for publishing, npm version, and so on) and a "loose mode" (used for interpreting the versions of dependencies, since they might be from the prior age).

The presence of a v has always been allowed, which yes, is somewhat "nonstandard" according to the spec. But semver.valid('v1.2.3') will return the "canonical" form '1.2.3'. Arguably a more strict API there would've been more correct, but in many instances less useful, since this is also used to pull out versions from git tags created by npm, which by default do have the v prefix.

Historical accidents all the way down, but the cost of changing to not allow the v in semver.valid() is almost certainly higher than any value it would provide. If you really want to detect whether a version is 100% strict and compliant, just replace this:

if (semver.valid(version)) {

with this:

if (version === semver.valid(version)) {

[wraithgar]

The npm@10 roadmap does have plans to stop auto-correcting package.json entries. However we will always have to account for the fact that packuments in existing tarballs have this data.

[wraithgar]

That said, this is not a bug per se w/ this module but with npm itself. Folks using this package can get whatever behavior they want, either cleaning it or not.

[riderx]

make sense for the explaining of how npm ecosystem works and that difficult to change.
there still 2 things who can be improved:

• the readme say this package follow the spec v2 where it should say v1 of semver then
• the valid method could have option to be "ultra scrict" or v2.

For both improvement i'm willing to make a PR but i would like to know if you are open to this changes

[wraithgar]

I think it's worth exploring a way to make valid choose between v1 and v2. Docs would probably end up being part of that PR right?