Dependencies are too tight and should follow SemVer

[moll]

Hey!

Stumbled upon some issues with Express/Connect that are fixed in newer versions. Unfortunately Keystone depends on packages by specifying their minor versions. That's a bad strategy for those dependencies which proclaim to follow SemVer as it prevents people from upgrading those modules outside of waiting for Keystone to upgrade itself.

Unfortunately it's a common problem in the NPM world and mark my words that this is going to bite people big time in the future should a high value bug appear in a commonly used, yet too tightly version-specified package. ;-)

[JedWatson]

Hey,

It's actually by explicit decision that we require that version of express; there were breaking changes made in 3.4.x that are incompatible with our current file upload implementation (and, probably, implementations in Keystone-powered projects).

The file uploading needs to be replaced and the work has been picked up recently with #309. As soon as this change lands we can upgrade express to the latest version (which should be accompanied by a minor version bump to Keystone) and we can prepare for Express 4.0.

Not sure if I misunderstand what you mean by "should follow SemVer" but Keystone's dependencies are locked to the minor version because otherwise, upgrades could be released that would break Keystone for users without our review.

We're very attentive to new versions of our dependencies being released and, with the exception of breaking changes, upgrade the versions specified in Keystone's package.json regularly.

Another recent breaking change that comes to mind was in Moment, between 2.2 and 2.3; they significantly changed the API (so an invalid date would return undefined instead of a moment object). We had to refactor everything for the upgrade, then refactor it back when they rolled back the decision with the next version.

It took a while to ensure compatibility with the new release. Locking versions at the minor level has, to date, been very good for the stability of Keystone, and lets us find and address breaking changes in our dependencies before our users suffer for them.

[moll]

Hey. Thanks for the thorough answer! I understand your reasons for Express and Moment modules, but do not concur overall. SemVer states that minor version upgrades MUST be backwards compatible. If you were indeed using non-public API parts of Express, then, yeah, breakage deserved.

It seems to me Moment did exactly the right thing — they fixed a bug which broke backwards compatibility. Kudos to them. But then again, if it was intentional in the beginning, they should be kicked in the nuts. What was not right was Keystone assuming this change was business as usual. You could've then either locked Moment.js and/or prevent only that particular buggy version from being used. Libraries that don't follow SemVer are a menace for exactly those breakage reasons you describe, but we don't need to accept all the shit that's thrown at us. Fight and educate, otherwise we're going to end up with people doing things for the detriment of us all. ;-)

As a general practice, I strongly suggest you prefer modules that follow SemVer over those that don't and depend only on major versions for the majority of libraries. Why make your own and your clients' life harder? Bugs do happen, yeah, but for the most part they're fixed fairly fast. Only with SemVer there's no guarantee they're fixed within one minor version. In the end, if shit hits the fan, it's going to be Keystone who'll be blamed for forcing to use unfixed versions, even if only for 8 hours.

[moll]

Oh, one more thing: Don't think it's your or Keystone's job to protect people from all buggy versions. It's not. It's the job of the end-app developer who should lock their dependencies recursively with something like NPM shrinkwrap. Those who don't are doing it wrong.

[JedWatson]

If you were indeed using non-public API parts of Express, then, yeah, breakage deserved.

Nope; using a public part of express; which they've been replacing / deprecating ever since and now removed with 4.0.

It seems to me Moment did exactly the right thing

Nope; they introduced a breaking change between 2.1 and 2.2, and then left it there for about a month. I'm not going to complain because date parsing is hard and it's a great library, but it was pretty annoying to refactor everything only to have it "fixed" in 2.3 - which was, again, a breaking change.

In the end, if shit hits the fan, it's going to be Keystone who'll be blamed for forcing to use unfixed versions, even if only for 8 hours.

True that we could to be blamed for using unfixed versions, but when the fixed versions also introduce compatibility issues, what can we do? We upgrade as soon as they're stable, or we're compatible.

Oh, one more thing: Don't think it's your or Keystone's job to protect people from all buggy versions. It's not.

If a new user tries yo keystone and it doesn't work because some package we depend on has released a new, incompatible version, it's not the package that broke for them; it's Keystone.

This is a different issue to shrink-wrapping dependencies for production.

Specifying dependencies with ~ over ^ and being proactive about upgrading them goes a long way to mitigate this issue.

this is going to bite people big time in the future should a high value bug appear in a commonly used, yet too tightly version-specified package. ;-)

Hopefully if a high value bug appears, it'll be fixed with a patch version, not a minor bump. Isn't that the point?

All this aside it doesn't seem that changing it will fix your original problem (older version of express) - Let's get on with ensuring we're compatible with 3.5.3 so we can upgrade. The core of that work is integrating multiparty directly to handle file uploads and smoothing out the API so it works more closely to how it used to in express 3.8, if that's possible.

[balupton]

This is essentially the same discussion we had over on npm here:
npm/npm#4587

[JedWatson]

I'm going to close this for now but keep your advice in mind, @moll, and see if we can lighten up the dependencies on some or all packages down the track, particularly for packages with a good track record of not introducing breaking changes on minor version bumps.

[moll]

Kay.

I'll just reply to what you asked before:

Nope; they introduced a breaking change between 2.1 and 2.2, and then left it there for about a month.

Ah, okay, then they're you-know-what. ;-)

Specifying dependencies with ~ over ^ and being proactive about upgrading them goes a long way to mitigate this issue.

If you meant Keystone's going to use ^, then yep, that was what I was advocating all along.

Hopefully if a high value bug appears, it'll be fixed with a patch version, not a minor bump. Isn't that the point?

Certainly, but the minor version has probably changed since then, too, because every single new function you add to your library needs a bump of the minor.