Replace ts-proto with protobuf-es #700
Conversation
🦋 Changeset detectedLatest commit: 2874e05 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
| @@ -63,7 +70,7 @@ export interface SignalOptions { | |||
|
|
|||
| type SignalMessage = SignalRequest['message']; | |||
|
|
|||
| type SignalKind = NonNullable<SignalMessage>['$case']; | |||
| type SignalKind = NonNullable<SignalMessage>['case']; | |||
There was a problem hiding this comment.
interesting to see they kept this design
src/room/track/LocalVideoTrack.ts
Outdated
| @@ -404,7 +406,8 @@ export function videoQualityForRid(rid: string): VideoQuality { | |||
| case 'q': | |||
| return VideoQuality.LOW; | |||
| default: | |||
| return VideoQuality.UNRECOGNIZED; | |||
| // FIXME should be unrecognized | |||
| return VideoQuality.OFF; | |||
There was a problem hiding this comment.
maybe default to something other than off? I know this is likely an invalid case.. but OFF is a reserved enum that only the server uses
There was a problem hiding this comment.
makes sense!
I think protobuf behaviour generally would be to default to the first enum value, but HIGH does make more sense in this context.
|
the test-workflow compat check is failing is due to robatwilliams/es-compat#72. But it's currently failing on this branch, because The runtime check of |
temporarily set the log level of this check to |
The only inconsistency that I could find was that we were using protobuf enum values of
Enum.UNRECOGNIZEDin two places. The generated types of protobuf-es do not expose thatUNRECOGNIZEDoption, so not sure what the best way to work around this would be.