Skip to content

[BUG] dangerous check if semver is a valid semver in constructor #354

Open

Description

@GiladShoham

What / Why

in the semver class constructor, you are checking if the value is an instance of SemVer
This check is dangerous since many times a package manager can create a tree of packages where you have 2 instances of SemVer in different places in the filesystem.
This leads to a bug where you pass a semver from one package to another then call the constructor and get an error about Invalid Version

How

Current Behavior

An error when you pass a SemVer instance between packages and call the SemVer constructor.

Steps to Reproduce

create 2 packages with different locked semver package version (for example 7.3.1 and 7.3.2)
(This sometimes happen also when the versions are the same, but it's easier to reproduce with different versions)
pass a SemVer instance from one of them to the other and call the SemVer constructor with this version.

Expected Behavior

Test if the value is valid SemVer by it's content (or content + constructor name) instead of using instanceof

References

This happens to me when using yarn berry API even though the core and the npm-plugin uses the same versions of semver.
When installing my tool as global with npm it creates 2 instances of the SemVer because it hoisted another version from another package.

Activity

  1. GiladShoham commented on Dec 9, 2020

    @GiladShoham
    Author

    A concrete use case with a reproduce repo is described here
    yarnpkg/berry#2224
    the reproduce repo is here - https://github.com/GiladShoham/yarn-invalid-version-bug

  2. added this to the v8 milestone on Oct 31, 2022
  3. mbtools commented on Feb 1, 2024

    @mbtools
    Contributor

    Old issue but imho, getting an error is essential here.

    The implementation in another release might (probably will) have different behaviour. It would be a big assumption that you can pass an instance of one semver release to another and expect consistent results (there are no tests for such cross-release cases either).

    You could stringify the version first and then create the instance in the other package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Bugthing that needs fixingsemver:majorbackwards-incompatible breaking changes

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions