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

[BUG] limit version prefix to optional v #691

Open
1 task done
Tracked by #501
mbtools opened this issue Mar 20, 2024 · 4 comments
Open
1 task done
Tracked by #501

[BUG] limit version prefix to optional v #691

mbtools opened this issue Mar 20, 2024 · 4 comments
Labels
Bug thing that needs fixing semver:major backwards-incompatible breaking changes

Comments

@mbtools
Copy link
Contributor

mbtools commented Mar 20, 2024

Is there an existing issue for this?

  • I have searched the existing issues

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

  • node-semver: 7.6.0
@mbtools mbtools added Bug thing that needs fixing Needs Triage needs an initial review labels Mar 20, 2024
@wraithgar wraithgar added semver:major backwards-incompatible breaking changes and removed Needs Triage needs an initial review labels Mar 20, 2024
@wraithgar wraithgar mentioned this issue Mar 20, 2024
14 tasks
@wraithgar
Copy link
Member

Added this to #501

@mbtools
Copy link
Contributor Author

mbtools commented Mar 22, 2024

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
Copy link

ljharb commented Mar 22, 2024

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
Copy link
Contributor Author

mbtools commented Mar 27, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing semver:major backwards-incompatible breaking changes
Projects
None yet
Development

No branches or pull requests

3 participants