4 ms·

I am one of the maintainers of a popular Node-based CLI (the firebase CLI). This type of thing has happened to us before. I think the real evil here is that by

by habosa 7y ago

I am one of the maintainers of a popular Node-based CLI (the firebase CLI). This type of thing has happened to us before. I think the real evil here is that by default npm does not encourage pinned dependency versions. If I npm install is-promise I'll get something like "^1.2.1" in my package.json not the exact "1.2.1". This means that the next time someone installs my CLI I don't know exactly what code they're getting (unless I shrinkwrap which is uncommon). In other stacks having your dependency versions float around is considered bad practice. If I want to go from depending on 1.2.1 to 1.2.2 there should be a commit in my history showing when I did it and that my CI still passed. I think we miss the forest for the trees when we get mad about Node devs taking small dependencies. If they had pinned their version it would have been fine.

KMnO4 7y ago
That’s still the fault of the package developer. “^1.2.1” means “any version with a public API compatible with 1.2.1”, or in other words “only minor versions”. The whole point of semantic versioning is to guarantee breaking changes are expressed through major versions. If you break your package’s compatibility and bump the version to 1.2.1 instead of 2.0.0 then people absolutely should be upset.
- lukevp 7y ago
  Allowing any version drift of dependencies at all means that if you don’t check in and restore using the package lock file, you cannot have reproducible builds. The package lock files are themselves dependent on which package restore tool you are using (yarn vs npm vs ...) it’s also much too ambitious to believe that all packages in an ecosystem will properly implement semver. There may even be times where a change doesn’t appear to be breaking to the maintainer but is in actuality. For example, suppose a UI library has a css class called card-invalid-data and wanted to rename to card-data-invalid. This is an internal change since it is their own css, but could break a library that overrode this style or depended on this class. I would consider this a minor version but it could still cause a regression for someone.
  - rroblak 7y ago
    > Allowing any version drift of dependencies at all means that if you don’t check in and restore using the package lock file, you cannot have reproducible builds. This is the germane point in this incident. The parent comment mentions that SemVer "guarantee[s] breaking changes are expressed through major versions". This is a common misperception about SemVer. That "guarantee" is purely hypothetical and doesn't apply to the real world where humans make mistakes. The OP `is-promise` issue is an example of the real world intruding on this guarantee. The maintainers clearly didn't intend to break things but they did because everybody makes mistakes Which points to the actual value proposition of SemVer: by obeying these rules, consumers of your package will know your _intention_ with a particular changeset. If the actual behavior of that changeset deviates from the SemVer guidelines (e.g. breaking behavior in a patch bump), then it's a bug and should be fixed accordingly. Back to the parent's point about locking dependency version— I would add that you should also store a copy of your dependencies in a safe location that you control (aka vendoring) if anything serious depends upon your application being continually up and running.
- TSiege 7y ago
  I think you might be misunderstanding the above comment. The default behavior of `npm i <package>` is to add `"<package>": "^1.2.1"` _not_ `"<package>": "1.2.1"`. The point the commenter was trying to make is that the tool itself has a bad default which makes it easy to make mistakes. I would go so far as to argue that when `npm i` does not have the behavior a user would expect from a package manager in that regard.
  - yellowapple 7y ago
    And likewise, I think the point of that above comment is that such a change in default behavior wouldn't be necessary if package authors actually obeyed semantic versioning. That is: "^1.2.1" shouldn't be a bad default relative to "1.2.1"; you generally want to be able to pull in non-breaking security updates automatically, for what I hope are obvious reasons, and if that goes sideways then the blame should be entirely on the package maintainer for violating version semantics, not on the package/dependency manager for obeying version semantics. I don't have much of an opinion on this for Node.js, but the Ruby and Elixir ecosystems (among those of many, many other languages which I've used in recent years) have similar conventions, and I don't seem to recall nearly as many cases of widely-used packages blatantly ignoring semantic versioning. Then again, most typically require the programmer to be explicit about whether or not to allow sub-minor automatic version updates for a given dependency, last I checked (e.g. you edit a configuration file and use the build tool to pull the dependencies specified in that file, as opposed to the build tool itself updating that file like npm apparently does).
nialv7 7y ago
Probably off topic, but just want to say cargo does the same things on the Rust side, and it has been annoying me to hell. And it's even worse in cargo, because specifying "1.2.1" means the same thing as "^1.2.1".
joepie91_ 7y ago
> If I npm install is-promise I'll get something like "^1.2.1" in my package.json not the exact "1.2.1". This means that the next time someone installs my CLI I don't know exactly what code they're getting (unless I shrinkwrap which is uncommon). Yes, this is by design. If this weren't the case, the ecosystem would be an absolute minefield of non-updated transitive dependencies with unpatched security issues.

by habosa 7y ago

KMnO4 7y ago

That’s still the fault of the package developer. “^1.2.1” means “any version with a public API compatible with 1.2.1”, or in other words “only minor versions”. The whole point of semantic versioning is to guarantee breaking changes are expressed through major versions. If you break your package’s compatibility and bump the version to 1.2.1 instead of 2.0.0 then people absolutely should be upset.

lukevp 7y ago
Allowing any version drift of dependencies at all means that if you don’t check in and restore using the package lock file, you cannot have reproducible builds. The package lock files are themselves dependent on which package restore tool you are using (yarn vs npm vs ...) it’s also much too ambitious to believe that all packages in an ecosystem will properly implement semver. There may even be times where a change doesn’t appear to be breaking to the maintainer but is in actuality. For example, suppose a UI library has a css class called card-invalid-data and wanted to rename to card-data-invalid. This is an internal change since it is their own css, but could break a library that overrode this style or depended on this class. I would consider this a minor version but it could still cause a regression for someone.
- rroblak 7y ago
  > Allowing any version drift of dependencies at all means that if you don’t check in and restore using the package lock file, you cannot have reproducible builds. This is the germane point in this incident. The parent comment mentions that SemVer "guarantee[s] breaking changes are expressed through major versions". This is a common misperception about SemVer. That "guarantee" is purely hypothetical and doesn't apply to the real world where humans make mistakes. The OP `is-promise` issue is an example of the real world intruding on this guarantee. The maintainers clearly didn't intend to break things but they did because everybody makes mistakes Which points to the actual value proposition of SemVer: by obeying these rules, consumers of your package will know your _intention_ with a particular changeset. If the actual behavior of that changeset deviates from the SemVer guidelines (e.g. breaking behavior in a patch bump), then it's a bug and should be fixed accordingly. Back to the parent's point about locking dependency version— I would add that you should also store a copy of your dependencies in a safe location that you control (aka vendoring) if anything serious depends upon your application being continually up and running.
TSiege 7y ago
I think you might be misunderstanding the above comment. The default behavior of `npm i <package>` is to add `"<package>": "^1.2.1"` _not_ `"<package>": "1.2.1"`. The point the commenter was trying to make is that the tool itself has a bad default which makes it easy to make mistakes. I would go so far as to argue that when `npm i` does not have the behavior a user would expect from a package manager in that regard.
- yellowapple 7y ago
  And likewise, I think the point of that above comment is that such a change in default behavior wouldn't be necessary if package authors actually obeyed semantic versioning. That is: "^1.2.1" shouldn't be a bad default relative to "1.2.1"; you generally want to be able to pull in non-breaking security updates automatically, for what I hope are obvious reasons, and if that goes sideways then the blame should be entirely on the package maintainer for violating version semantics, not on the package/dependency manager for obeying version semantics. I don't have much of an opinion on this for Node.js, but the Ruby and Elixir ecosystems (among those of many, many other languages which I've used in recent years) have similar conventions, and I don't seem to recall nearly as many cases of widely-used packages blatantly ignoring semantic versioning. Then again, most typically require the programmer to be explicit about whether or not to allow sub-minor automatic version updates for a given dependency, last I checked (e.g. you edit a configuration file and use the build tool to pull the dependencies specified in that file, as opposed to the build tool itself updating that file like npm apparently does).

nialv7 7y ago

Probably off topic, but just want to say cargo does the same things on the Rust side, and it has been annoying me to hell. And it's even worse in cargo, because specifying "1.2.1" means the same thing as "^1.2.1".

joepie91_ 7y ago

> If I npm install is-promise I'll get something like "^1.2.1" in my package.json not the exact "1.2.1". This means that the next time someone installs my CLI I don't know exactly what code they're getting (unless I shrinkwrap which is uncommon). Yes, this is by design. If this weren't the case, the ecosystem would be an absolute minefield of non-updated transitive dependencies with unpatched security issues.