-
Notifications
You must be signed in to change notification settings - Fork 40
Allow multiple versions of tchannel to be require()d. #259
Comments
TChannel is a singleton. There should only be one. We enforce this. |
There are better ways:
You should warn, document, but never just throw an error. This causes unpredictable behavior across builds. |
I agree there are better ways in an open source project to do this. However we have a genuine support problem that must open source projects do not have. We've had an issue in the past where engineers created two instances of tchannel and advertised twice in the same process. This created a weird edge case in terms of load balancing behavior and they reported it as a bug. When we said "I'm sorry; you cant create two tchannels. that's not a supported feature" they were really upset. They mentioned that we must support the library even if its used incorrectly. They mentioned that if we want to stop people from using it incorrectly we should throw. I agree with this trade-off. We throw when TChannel is used in an unsupported fashion and we have to support all other use cases and permutations in which the library is used. However to accommodate your need; I will add a
As an aside shrinkwrap is the recommended way to get predictable behavior across builds ;) |
Tchannel should't make any assumption about the module tree in which it is in. Let's assume we have the following
node_modules
tree:Both
dep1
anddep2
have tchannel as dependency matching all minor version up to 4:Running
npm install
on this tree will (rightfully) mark both 3.5.0 and 3.6.0 versions as valid and matching their respective semver ranges.I think it's a very bad practice for a module to expect a shape of the project's entire modules tree in which it is.
The text was updated successfully, but these errors were encountered: