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

DDPClient.prototype.call parameter confusion #69

Open
license2e opened this issue Oct 23, 2015 · 2 comments
Open

DDPClient.prototype.call parameter confusion #69

license2e opened this issue Oct 23, 2015 · 2 comments

Comments

@license2e
Copy link

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
Copy link
Member

vsivsi commented Oct 23, 2015

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
Copy link
Author

@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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants