-
Notifications
You must be signed in to change notification settings - Fork 50
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
Top-level api should accept channel object as first argument #124
Comments
👍 |
Actually...wait, why? |
@jmeas I had started a comment over in #117 about how the top-level API is actually closer to what we'd want for a mixin. I was playing around and discovered that you can't currently pass a channel instead of a string which just seemed odd either way. It's probably just gonna be a one-line change so I don't see it being harm Pseudo-code: - channel = this.channel(channelName);
+ channel = _.isString(channelName) ? this.channel(channelName) : channelName; |
The whole point of the top level API is if you don't have a handle of a Channel. If you have the Channel, then you should use the API directly... Sure, it's a change to one line of code, but it's completely unnecessary. 👍 to close, |
Sidenote: Can we stop using ":+1: to close", it's confusing as hell at first. I almost closed the page assuming you were okay with this change. Well, I guess my idea here would really be to make the top-level api the inverse relationship we were discussing in #117. I was just experimenting before trying to see how close I could get with the top-level api as a mixin and realized you could only pass a string in. If we do create a mixin, I almost want to kill the top-level api, because that's not how you should be using Radio once there's |
I guess I'm confused. As I understand it one will always want the top-level API so that you don't need to get a reference to a channel if you only need to make one request / command / event. This seems to be a tangential feature to having automated clean ups. |
@jmeas My thinking was that the top-level api is aiding the use case that would ultimately be the opposite of what one would really want to do. Class.extend({
init: function() {
Radio.reply('myChannel', 'myRequest', this.myReply, this);
// vs something like:
this.replyTo(myChannel, 'myRequest', this.myReply);
}
}); If you still wanted to keep it: if and when the inverse-api (mixin?) is added, it makes sense that you'd be able to both pass in a channel and a channel name. this.replyTo(myChannel, 'myRequest', this.myReply);
this.replyTo('myChannel', 'myRequest', this.myReply); |
Yup, agreed. This makes a ton of sense to me. I'm going to keep this issue open. Once the other issue is resolved we can talk about getting rid of the top-level API altogether. |
Actually, going to close this and continue the discussion on #128 |
I was expecting this to work, so I think it'd be good to add:
The text was updated successfully, but these errors were encountered: