Closed gauntface closed 8 years ago
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 method we encourage devs to use, just an implementation detail.
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:
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.
— Reply to this email directly or view it on GitHub https://github.com/GoogleChrome/push-encryption-node/issues/10#issuecomment-191890079 .
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
sendWebPush(subscriptionNoKeys,
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?