Change publishData signature

[lukasIO]

I think for this one we'll have to live with a hard change as we simultaneously want to switch from using sids to identities which I don't think we could easily do in a backwards compatible way in a deprecation step. (We'd need to figure out if the user intends to set sids or identities)

[changeset-bot]

🦋 Changeset detected

Latest commit: 9ee0aab

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
livekit-client	Major

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

[davidzhao]

I think it's still kind of tricky for someone who's using the options interface to realize it's changed for them. Either of the following could overcome that:

• separate identity spec from destination field so that it's destinationIdentities
• create a type for Participant.Identity so that it would be impossible to mix up with a standard string.

[lukasIO]

That's true. But unsure what the right way to make this more obvious would be.

separate identity spec from destination field so that it's destinationIdentities

this is not ideal as the accepted type is string | RemoteParticipant

create a type for Participant.Identity so that it would be impossible to mix up with a standard string.

if we let the type be equal to string then there won't be any type errors. If we don't (unsure how) then converting/casting it to the identity type would cause some additional friction for users.

What do you think about renaming it to destinations (plural)? That would throw an error for users using the old way and they would have the docstring hints to clarify they're supposed to enter identities now.

[github-actions]

size-limit report 📦

Path	Size
dist/livekit-client.esm.mjs	78.68 KB (-0.13% 🔽)
dist/livekit-client.umd.js	84.32 KB (-0.06% 🔽)

[davidzhao]

separate identity spec from destination field so that it's destinationIdentities

this is not ideal as the accepted type is string | RemoteParticipant

what I meant is separating them so it's not bundled together

type DataPublishOptions = {
  destinationParticipants?: []RemoteParticipant
  destinationIdentities?: []string
}

My main hope is that users will realize it no longer compiles and specifically set identities field. If it's simply renaming a field, they would not bother reading the details of the change.

[lukasIO]

what happens when both are set? Does it result in the message being receivend by both added together or does one field take precedence over the other?

I think as a user I would feel confused whether I have to set both of them or not.

My main hope is that users will realize it no longer compiles and specifically set identities field. If it's simply renaming a field, they would not bother reading the details of the change.

Yeah that makes sense. But it's a major change after all. Our migration guide should make this clear.

[davidzhao]

what happens when both are set? Does it result in the message being received by both added together or does one field take precedence over the other?

What do you think about throwing an exception? Just an idea tho. It's your decision on which path we ended up taking.

[hiroshihorie]

Not sure if I'm reading the code correctly but DataPublishOptions.reliable is null by default so this will default to LOSSY ?

[lukasIO]

yeah, LOSSY by default is what is intended!

[davidzhao]

lgtm!