-
Notifications
You must be signed in to change notification settings - Fork 10
Hide serialization implemenation to client apps #5
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 8000 you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely! And thanks for the extra tests 👍
// public func reply(to message: Message, with data: MessageData) { | ||
// let replyMessage = message.replacing(data: data) | ||
// callBridgeFunction("send", arguments: [replyMessage.toJSON()]) | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting approach, I didn't realize that there was a reply()
in the iOS library. Let's leave this commented out for now to see if we want to offer this in both the iOS and Android libraries. I do like the language that it makes it clear that you're replying to a previously handled message. I'll think about this some more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you @svara!
This PR hides the serialization by introducing an internal
InternalMessage
class and extracts the related code from theMessage
.The public
Message
's data has changed from[String: AnyHashable]
toString
. This way we discourage users of pulling out values individually.Other changes:
InternalMessage
.Message
.