expo / snack

Expo Snack lets you run Expo in the browser.

https://snack.expo.dev

MIT License

372 stars 111 forks source link

[snack-sdk] add snackpub integration #376

Closed Kudo closed 1 year ago

Kudo commented 1 year ago

Why

close ENG-7101

pr series

👉 #376

377

378

379

380

381 How

clean webplayer transport code that it still relied on pubnub transport
refactor transports so now we have three different transports
- TransportImplPubNub
- TransportImplWebPlayer
- TrafficMirroringTransport = TransportImplPubNub + TransportImplSocketIO with traffic mirror
original createTransport will create TransportImplPubNub and new createSnackPubTransport will create TrafficMirroringTransport

Test Plan

snackpub integration tests
unit tests passed (refactor from PubNub mock to TestTransport mock)

linear[bot] commented 1 year ago

ENG-7101 [SnackSdk] Add socket.io based transport

* Enable WebSocket only * Mirroring traffic with PubNub transport. For subscribers to get duplicated message, we should process only one of them. * ~^ So we need a messageId to SnackTransportMessage~ Update: we don't have a messageId. so to use JSON.stringify() and compare the whole string in current implementation. * ~^ Maybe we need to add metrics for message missed from SocketIoTransport but arrived from PubNubTransport~ Update: PubNub SDK doesn't expose this feature. We cannot have a similar metric from PubNub.

ide commented 1 year ago

I'm removing myself as a review from this PR and if you and Cedric is comfortable moving forward with this, let's merge it. I left one comment as a suggestion about naming.

Kudo commented 1 year ago

i'm going to merge this pr first as all review comments should be followed up and it's hard to maintain those stocked prs. if there's any other further comments, please let me know and i could do it in a separated pr.

also, this pr only touches snack-sdk, it should be safe to merge.