Add PushDeliveryTarget - #5
Conversation
tib
left a comment
There was a problem hiding this comment.
Why do we introduced another push client protocol?
This is already VERY confusing to me:
- PushClient -> i know i need a client to send push...
- TopicPushClient -> ???
Shouldn't we use an enum to differentiate delivery targets?
enum PushDeliveryTarget {
case deviceToken(String)
case topic(String)
}This way push client code could be:
public protocol PushClient: Sendable {
func send(
notification: PushNotification,
to target: PushDeliveryTarget
)
}Also call site would look a lot better:
pushClient.send(
notification: PushNotification,
to: .deviceToken("my-device")
)
pushClient.send(
notification: PushNotification,
to: .topic("my-topic")
)WDYT @viaszkadi ? 🤔
|
+1: I'm well aware that APNS topic has to be the bundle identifier, and it's completely different what Firebase defines as topic. Still from a framework perspective, I'd like to have 1 interface to handle things, not multiple abstraction protocols. If APNs has no support for real topics, then we're screwed anyways, so we could simply trigger a If someone uses the FCM driver then it'll be just fine and we should be able to send to both platforms using topics... |
PushDeliveryTargetfor device tokens and topics