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

Peer dependency version ranges #60

Open
ronkorving opened this issue May 31, 2017 · 9 comments
Open

Peer dependency version ranges #60

ronkorving opened this issue May 31, 2017 · 9 comments
Assignees

Comments

@ronkorving
Copy link
Member

Currently, the couchbase optional peer dependency is semver range ^2.1.6 (ie: 2.x).

This semver range communicates to me:

We've tested the couchbase module at version 2.1.6, any newer than that, at your own risk.

Another way to read it is:

We know this to work from 2.1.6 up until a version we do not mention, or up until 2.Infinity.Infinity.

Since I read it the first way (and I know this to be the case with at least the couchbase module), I'm scared to install anything newer unless I go through the full changelog of the couchbase module, and check which APIs of it are being used by MAGE and how. That's a lot of work that we can't expect normal users of MAGE to do.

I on the other hand have gone through the entire couchbase diff between version 2.1.6 and (latest) 2.3.3 and can confirm that it should be absolutely safe to use 2.3.3 as far as MAGE is concerned.

How people use couchbase directly is not in our hands, so we can't be too restrictive on versions (we can't bump the minimum required version too aggressively), but it's very important that we communicate our certainty about peer dependency versions working well with MAGE.

For that reason I propose that we change our semver range policy to include both the lowest known version and the highest known version known to work with MAGE.

So in the case of couchbase, I suggest we would mark it as >=2.1.6 <=2.3.3.

Thoughts?

@stelcheck
Copy link
Member

I'm mostly against it - at least when considering how codependency currently works.

Peer dependencies are dependencies that are managed by the parent project. All codependency is saying is, "you will need at least this version if you expect it to run" - which we can absolutely verify.

But putting an upper boundary should only be done if we know in just as a verifiable way that passed a certain version, it will not work. Which is not the case here.

Let's put it this way; what if couchbase makes an update with features I really want to use in my project? In that case, MAGE will just tell me nope, no can run - unless you fork MAGE or wait for MAGE's package.json to be updated. This is quite bad in cases where said upgrade would contain much needed security fixes and/or bug fixes.

The only way I would be OK with this would be if we could recommend a version range without actively failing if that range is not respected. Or if you had a mean to override MAGE's versions as set in its package.json in some way. Then, the end user remains in control. But if I understand correctly, both alternatives would require an update to codependency, correct?

@stelcheck
Copy link
Member

@ronkorving any feedback?

@AlmirKadric
Copy link
Contributor

I agree with @ronkorving we should never give our users the experience where things are broken and don't work. This is all too familiar and painful. Also with MAGE errors not being as descriptive as they should be, it makes it hard to identify the issues (especially couchbase and all their funkiness).

I would say the upper boundary should be moved naturally as we continue to develop and test. This is why I think internally we should make is a habbit (process) to update the upper boundary on all mage dependencies whenever a project is started.

@stelcheck
Copy link
Member

That's fine, for us, but I doubt it would be as convenient for external users whom would not be able to change MAGE's predefined upper boundaries.

In the case I was making with Couchbase, the feature I may want to use could be absolutely unrelated to MAGE itself (or how it uses the module); for instance, I may want to use the latest query feature of Couchbase. Or I may want to resolve a security issue by upgrading to the latest.

The real solution would be to take out co-dependencies, and encapsulate features into external modules... but I doubt this would happen soon.

In the meantime, we have three alternative solutions:

  1. Fully implement greenkeeper, and hope it upgrades devDeps. Then put all our optionalPeerDeps into devDeps, and whenever a PR comes in manually update optionalPeerDeps.
  2. Soft-warnings: log that the version used is beyond what is known to MAGE
  3. Hard-error w/ ignores: implement what is suggested here, but provide developers with a way to override the limitation manually

@AlmirKadric
Copy link
Contributor

To be honest, I see both opinions and both have pros and cons. What about a middle ground, the upper boundary could be a semver range in the format of a carat or tilde. If the version detected is greater than the upper boundary as an absolute, we can allow it but warn that it was not tested by the MAGE team and isn't officially supported. (Whether or not this is doable in semver and if we have to implement something different can be a separate discussion)

@stelcheck
Copy link
Member

That's pretty much what I was suggesting as option 2.

@ronkorving
Copy link
Member Author

I don't know if you can mix verA - verB ranges with carets or tildes. But major.minor for example should work.

@stelcheck stelcheck added this to the 1.3.0 milestone Aug 6, 2017
@stelcheck
Copy link
Member

At any rate, this now appears like the discussion would need to move to codependency first; it would need to provide an API for requiring and warning.

@ronkorving
Copy link
Member Author

I've created an issue: Wizcorp/codependency#13

@stelcheck stelcheck removed this from the 1.3.0 milestone Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants