search

npm / node-semver

The semver parser for node (the one npm uses)
ISC License
5.07k stars 492 forks source link

[BUG] limit version prefix to optional `v` #691

Open mbtools opened 6 months ago

mbtools commented 6 months ago

Is there an existing issue for this?

Current Behavior

The following "ranges" are currently considered valid: > v=1.2.3, > vvv1.2.3, > v==1.2.3, > v=vv==v1.x

The validation is not strict enough and allows =, v=, and nonsensical prefixes as well.

Expected Behavior

With next major semver, the version prefix should be limited to a single, optional v. Other prefixes shall not be permitted anymore.

Suggested change:

// createToken('LOOSEPLAIN', `[v=\\s]*${src[t.MAINVERSIONLOOSE] // old
createToken('LOOSEPLAIN', `v?\\s*${src[t.MAINVERSIONLOOSE] // new

// createToken('XRANGEPLAIN', `[v=\\s]*(${src[t.XRANGEIDENTIFIER]})` +  // old
createToken('XRANGEPLAIN', `v?(${src[t.XRANGEIDENTIFIER]})` +// new

// createToken('XRANGEPLAINLOOSE', `[v=\\s]*(${src[t.XRANGEIDENTIFIERLOOSE]})` + // old
createToken('XRANGEPLAINLOOSE', `v?\\s*(${src[t.XRANGEIDENTIFIERLOOSE]})` + // new

Steps To Reproduce

validRange('> v=1.2.3')

Environment

wraithgar commented 6 months ago

Added this to https://github.com/npm/node-semver/issues/501

mbtools commented 6 months ago

I have done some analysis of all versions, dependencies and engine ranges used in the top 5200 npm package.json files:

Type of Range Count
^ 52049
semver 14049
>= 2934
~ 1979
|| 841
* 730
x 502
major 348
\= 162
"latest" 154
npm: 101
git: 69
- 60
file: 44
> 25
major.minor 24
"next" 15
http/s: 12
link: 7
portal: 3
\< 3
. 3
v1.1.0 1
./ 1
workspace: 1
v1.13.4 1
Grand Total 74118

The use of v or v= prefixes is extremely rare. It's only 2 cases (which could be tags and valid).

~I would suggest to put up a deprecation notice of these semver v1 notations and then completely stick to semver v2 (i.e. no prefixes, not even a v) with the next major.~

ljharb commented 6 months ago

why even bother removing the v? there's nothing conceptually wrong with it; npm doesn't comply with semver v2 anyways; and https://semver.org/#is-v123-a-semantic-version supports its use as an optional prefix.

mbtools commented 6 months ago

Agree. Removing v won't make a difference. Stats are interesting nevertheless 😃