Skip to content

feat: allow strict semver check #789

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

riderx
Copy link

@riderx riderx commented Jun 23, 2025

The Current Implementation does not follow SemVer v2 as stated in the doc.
This allows following the spec by making leading v as invalid with an option named "strict".

I'm open to renaming it if needed.

References

Related to #376
and #376
and PR: #476 who was rejected.

@riderx riderx requested a review from a team as a code owner June 23, 2025 21:51
@wraithgar
Copy link
Member

This introduces a bit of a confusing paradigm here now. We already have the loose option, which defaults to true. I think adding a strict config that now solely interacts with this one part of parsing is confusing and incorrect. Folks would assume (correctly I would say) that these configs are reflections of each other and should act as such.

This is a rather subtle and complex problem to solve unfortunately. The v-prefixed version is likely here to say based on how disruptive removing it would be.

I think there is still merit in having a way to tell this package to ignore that vestigial support, but I don't think adding a strict option to the constructor is the full solution. Also I unfortunately don't have any good input on what the full solution is, which I know is frustrating. More discussion is needed here, and this PR is a good place to have it for now.

@riderx
Copy link
Author

riderx commented Jun 23, 2025

@wraithgar thanks for the prompt reply, I see what you mean and you are right.
I think I could do a SemVer2 option and this will set the loose to false then.

Do you think it could less confusing and valid ?

@ljharb
Copy link
Contributor

ljharb commented Jun 23, 2025

No, because semver 2 doesn't have ranges in it either, and its description of v0 versions doesn't match npm's. Trying to match semver v2 isn't worth the effort - at some point there'll be a semver v3 that matches npm, and npm won't have to change anything.

@riderx
Copy link
Author

riderx commented Jun 23, 2025

@ljharb I'm just trying to be able to use validate from this package and make it closer to the spec, each step closer should be seen as a success.
If the SPEC bothers you, let's open an issue in it https://github.com/semver/semver/issues or maybe fork it and create NPM SemVer spec to lead this change.
Currently, this package misleads people thinking it follows the spec by linking it in the README. Where it don't
Any improvement will misslead less than now

@ljharb
Copy link
Contributor

ljharb commented Jun 23, 2025

I've discussed it with the spec maintainers and there's PRs open for much of what I discussed.

npm's flavor of semver is much more valuable and widespread than the spec itself - specs should document reality, not dictate it.

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

Successfully merging this pull request may close these issues.

3 participants