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

Pin request version #76

Closed
morgante opened this issue Nov 20, 2015 · 9 comments
Closed

Pin request version #76

morgante opened this issue Nov 20, 2015 · 9 comments

Comments

@morgante
Copy link

Unfortunately, it seems #5 might have been premature. Request does not currently follow semver, so it is dangerous to have a loose dependency on it.

I was recently affected by this and recommend that request-promise switch to an explicit request version dependency.

@analog-nico
Copy link
Member

Thanks @morgante I didn't know Request is not following semver. However, if Request-Promise would switch to a fixed Request version and update the Request version from time to time then Request-Promise either doesn't follow semver anymore, too, or would bump the major version each time because who knows if the new Request version introduced a breaking change.

IMHO you should define the specific Request version in your own package.json. Or use npm shrinkwrap. However, looking forward turning Request into a peerDependency might help. Then once everyone starts using npm@3 they have to explicitly install Request on their own. But I am not sure if that is a wise choice.

Do you have some thoughts you can add?

@morgante
Copy link
Author

would bump the major version each time because who knows if the new Request version introduced a breaking change.

I think this is potentially the best option. Yes, it's annoying to bump the major version every time but at least then request-promise is abstracting out the fact that the underlying request doesn't follow semver. As it is now, someone can have the request-promise version pinned and not realize that the underlying request version is fluctuating with major changes.

I'm using npm shrinkwrap to mitigate the problem currently, but setting it up for only a single dependency is definitely non-ideal.

Turning request into a peerDependency could also make sense, to the extent that request-promise is actually just offering another way of interacting with request.

@simov
Copy link
Member

simov commented Nov 20, 2015

to the extent that request-promise is actually just offering another way of interacting with request

Exactly.

and not realize that the underlying request version is fluctuating with major changes

Actually the current request code base is in maintenance state, so I try to not introduce breaking changes, but as you can see things go out of hand sometimes, especially when not tested.

@morgante
Copy link
Author

Given that, I propose that either request is pinned or moved to a peerDependency.

@simov
Copy link
Member

simov commented Nov 20, 2015

IMO peerDependency is more appropriate in this case.

@analog-nico
Copy link
Member

Since I don't prefer pinning - think about projects that migrated just some of their requests from Request to Request-Promise and want to use the same Request version throughout the project - let's go for a peerDependency. I will include this in the upcoming v3 release. Since it will also introduce ES6 promise support I will have to rewrite the installation section in the readme anyway. So it is good timing.

@morgante
Copy link
Author

👍

@analog-nico
Copy link
Member

Hi @morgante , I just released request-promise@4.0.0 which defines request as a peer dependency. You may now pin the request version in your project if you like. Cheers!

@morgante
Copy link
Author

Great, thanks!

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