-
Notifications
You must be signed in to change notification settings - Fork 18
sendWebPush() support no payload? #10
Comments
I prefer a single method, I don't have to dig through docs to figure out which one I should use, and in my code, I can pass a null/undefined value if I don't want to send anything. |
Yeah, sorry I meant the methods should be hidden from the user / not the But it does sound like you are pro having support for no payloads. On Thu, 3 Mar 2016, 18:06 Pete LePage, notifications@github.com wrote:
|
Yes to pro support for no payloads... Internal implementation details, I'm less concerned about but want Devs to be able to understand it, and fork it to use for their own purposes if necessary. |
Yes, sounds good, and is easy to implement. It's almost just an if statement around a few lines |
What should happen if you set a message but the subscription doesn't have keys? Silently just send a tickle? Throw an error? I'm thinking that this should throw, but open to being persuaded. |
I vote throw. If I screw up, I want to know that I screwed up, not silently fail (sending a tickle). |
+1 throw. API will be: sendWebPush(subscriptionNoKeys); // Triggers tickle |
At the moment, the sendWebPush() method expects a payload and throws and error if there is no payload.
There are a few scenarios where this may not be desirable:
1.) Using existing subscription objects that dont have keys
2.) Desire to use tickles and not send payload for some messages
3.) Make it easier to drop in the library
@wibblymat thoughts on this?
Looking briefly through code, it would be a case of just checking input data and picking either sendWebPushTickle() sendWebPushWithData() OR single method (same as now), but switch between which headers are set.
@petele @paullewis @owencm thoughts on this?
The text was updated successfully, but these errors were encountered: