feat: add shouldPush to contentTypes messages

[kele-leanes]

depends on xmtp/xmtp-ios#231 & xmtp/xmtp-android#184

closes #236

• added shouldPush to ContentCodec type
• added shouldPush method to Codecs
• send and get the value from native

[nplasterer]

@kele-leanes I think here instead of calling model.shouldPush maybe you can copy how the test does it

		let message = try await MessageV2.encode(
			client: client,
			content: model.encodedContentt,
			topic: model.topic,
			keyMaterial: Data(model.keyMaterial),
			codec: model.encodedContent.contentType // MAP THIS TO A CODEC?
		)

then it would be message.shouldPush

Or something along these lines to get the push without having to make changes to the iOS sdk directly.

[kele-leanes]

To do that I need to make MessageV2 and codecRegistry public and add keyMaterial to model which is not currently sent

[kele-leanes]

and making this method async/await would add more complexity where this method is called

[nplasterer]

I have no issue with MessageV2 and codecRegistry being public. @nakajima do you have any thoughts?

[kele-leanes]

besides that, since the codec registry in RN lives on the JS side, the MessageV2.encode method can not access the codecs from RN.
e.g: If I register a custom codec, which returns true from the shouldPush method, the native side can’t get this flag.
Is the reason why I suggest sending the value encoded as in this PR or sending it as a part of a SendOption object

[kele-leanes]

Regarding the behavior in iOS, shouldPush is already established and calculated based on the CodecTypes existing on the iOS side. However, in React Native (RN), CodecTypes are generated and stored on the JavaScript side. Consequently, the determination of this flag should occur on the JS side and then communicated to iOS. I've devised two solutions:

1. Include the shouldPush value as part of the EncodedContent using the proto, similar to how we currently send the fallback. This value can then be decoded and set when a MessageV2 is created. (Link to PR)

2. Send the shouldPush value as a plain Boolean, as proposed in this PR, and include it as an optional part of the SendOption.

The latter approach deviates from the convention of maintaining methods as similar as possible across platforms.

[richardhuaaa]

Gotcha! Thanks for explaining.

So I don't think we should do (1) - it muddies the protocol, and the protos are shared across all of the platforms as well as sent over the network.

in React Native (RN), CodecTypes are generated and stored on the JavaScript side

More of a long-term question/not blocking this PR, but why is this the case? Is there a reason we can't push that down to the native platform?

The latter approach deviates from the convention of maintaining methods as similar as possible across platforms.

So the issue is that we are deriving shouldPush from the non-encoded content, however the SDK is structured so that only the encoded content is sent down today, which means iOS is unable to derive shouldPush? If so, then does that mean RN Android has the same issue and would also benefit from this?

It seems like we have no choice with the current design but to do the sendOption thing even if it's a little ugly. Another question - given the contentType is in the SendOptions already, would it be feasible to have iOS construct the codec based on the content type, and then determine the shouldPush value from that? That way we are not giving app developers a way to set shouldPush directly.

[kele-leanes]

More of a long-term question/not blocking this PR, but why is this the case? Is there a reason we can't push that down to the native platform?

I'm not fully aware of the impact of this change, but the structure of a contentType has methods that you can not pass down to the native platform. This would solve our issues

Would it be feasible to have iOS construct the codec based on the content type, and then determine the shouldPush value from that?

We are not able to construct the codec since the contentType resides on the JS side. Specifically the custom ones. When the platform (in this case iOS) is trying to get the codec from a contentTypeId it searches throughout the generics codecs which are on the native side.

These changes must be added to the Android as well

[richardhuaaa]

I see. In that case it seems that adding shouldPush to the SendOptions is the only way for now. Perhaps we can name it something that makes it clear that it is internal-use only, like __shouldPush or something.

Long-term, there will probably be more cases like this where the RN SDK wants some functionality from the iOS SDK that we don't actually want to expose publicly. I wonder if it makes sense to define an iOS type/interface that is internal-use only, that we don't document or provide any guarantees about publicly.

[kele-leanes]

resolved in ccceabe