oortcloud / node-ddp-client

A callback style DDP (Meteor's Distributed Data Protocol) node client.
Other
263 stars 80 forks source link

DDPClient.prototype.call parameter confusion #69

Open license2e opened 8 years ago

license2e commented 8 years ago

In JS function.call expects parameters to be passed in directly instead of an array of parameters...

Also, Meteor.call accepts direct parameters while Meteor.apply accepts an array

The proposal:

Change from: DDPClient.prototype.call and DDPClient.prototype.callWithRandomSeed to: DDPClient.prototype.send and DDPClient.prototype.sendWithRandomSeed

vsivsi commented 8 years ago

Hi, your observation is correct regarding the parameters of call vs. apply. I noticed this myself when I first started using this package... But, this proposal, and the accompanying PR are a breaking change to this package, correct?

So all dependent packages that use the .call method to invoke Meteor methods (a sizable fraction, I would estimate) will break with the next update and need to change?

As I said above, I agree that the incongruity here is unfortunate, but this package has a fairly long list of dependents. As such, I would argue that this ship has sailed, so to speak, and we should just live with it, at least until some more pressing breaking changes are warranted, necessitating a full 1.0 semver major release. IMO, this is a nicety, but not worth all of the fuss it seems likely to cause.

license2e commented 8 years ago

@vsivsi I see your point, but that would just be an easy fix:

  1. next version announce API change with backward compatibility (ie. .call* would just call .send*)
  2. also, just designate that 1 or 2 versions from now the call would be deprecate
  3. iteration 2, start issuing console.log warnings (like other libraries do)
  4. when deprecating the call, just throw an exception for all the .call* functions
  5. iteration 3 just remove all together

In my experience, when developers are upgrading code, as long as you dont just change the API completely without a fair warning, they are ok with it.

I can add all the backward compatibility to my PR..