fix(uniffi): register the Bytes custom type once, in livekit-common - #1343
fix(uniffi): register the Bytes custom type once, in livekit-common#1343pblazej wants to merge 5 commits into
Conversation
`bytes::Bytes` was registered with `custom_type!` in both livekit-uniffi and livekit-datatrack. They are separate UniFFI components, so each emitted its own `typealias Bytes` and `FfiConverterTypeBytes` — and cargo-swift compiles both component files into one Swift module, so they collided. Borrow datatrack's registration with `use_remote_type!` instead of adding a second one. The type is then emitted once, in the component that owns it, which makes the `swift-workarounds` perl task unnecessary — and that task has to go in the same commit, since it would otherwise strip the only remaining definition. Unlike the regex, this is compile-checked and also fixes Kotlin and Python, which were still shipping two `Bytes` declarations. Upstream: mozilla/uniffi-rs#2933 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Changeset incompleteThis PR's changeset is missing version bumps for packages that are affected by the change. The following packages still require a bump:
Already covered:
A package must be bumped when its own files change, and whenever a package it depends on is bumped (so downstream consumers get a matching release). Click here to create a changeset for the missing packages The link pre-populates a changeset file with If this change doesn't require a version bump, add the |
| // `Bytes` is a remote type, so each UniFFI component that registers it with | ||
| // `custom_type!` emits its own converter — and the two component Swift files are | ||
| // compiled into one module, so a second `typealias Bytes` fails to build | ||
| // ("invalid redeclaration of 'Bytes'"). Reuse livekit-datatrack's registration | ||
| // instead of adding a second one; the type is then emitted once, in the file that | ||
| // owns it. Upstream: https://github.com/mozilla/uniffi-rs/issues/2933 | ||
| uniffi::use_remote_type!(livekit_datatrack::Bytes); |
There was a problem hiding this comment.
My main concern with putting this in livekit-datatrack is that it's a somewhat unintuitive place for this definition to go. I'm also not sure how this would impact data streams v2 given the uniffi bindings will have the same problem (and a dependency between livekit-datatrack and livekit-data-stream sounds like a bad idea).
I realize it's a bit more work, but what do you think about doing something like defining this custom_type! in some third package which livekit-uniffi, livekit-datatrack, and data streams v2 uniffi once that is is merged can all pull in via uniffi::use_remote_type!(...)? I'm not sure what this third package would be called, but my initial thought is maybe it could be in livekit-common - that package could gain a uniffi feature which gets set through each layer when building livekit-uniffi.
Thoughts on this?
There was a problem hiding this comment.
Prototyped it — works, Bytes lands once in livekit_common.swift. But it has to be a full component: a hand-written UniFfiTag compiles, then bindgen refuses with Unknown namespace for CustomType. Cost is a third namespace — livekit_common.swift at 558 lines, 553 of them duplicated runtime, publishing one typealias Bytes = Data — plus +4,160 B on the cdylib.
#1286 doesn't hit this either: no new setup_scaffolding!(), it reuses this crate's registration. And it answers the general question the other way, hand-mirroring common::EncryptionType/ClientCapability into livekit-uniffi.
So it turns on whether livekit-common becomes a component — bigger than this PR, and a two-line follow-up either way.
There was a problem hiding this comment.
Done — pushed b46be14, livekit-common owns it now.
Verified: livekit-common and livekit-datatrack still build without the feature (the dep is optional, gated on uniffi), the three-file module compiles, client-sdk-swift main builds clean against the generated package with tests, and the xcframework carries all three headers in one modulemap. Cost is 4,160 B on the cdylib, 66 KiB under the iOS gate.
|
Wait, what happens if you simply do this: uniffi::use_remote_type!(bytes::Bytes);At least the docs for this macro imply this would work. |
|
@ladvoc looks like it won't work: |
A remote type can only be registered once per UniFFI component, so owning the
registration in livekit-datatrack made every other component borrow from a crate
that has nothing to do with byte buffers. Register it in livekit-common instead,
which is where shared foundational types already live, and have each component
borrow it with `use_remote_type!`.
Owning a registration means being a component: `custom_type!` resolves
`crate::UniFfiTag`, and bindgen rejects type metadata belonging to no namespace
("Unknown namespace for CustomType"). So livekit-common gains an optional
`uniffi` feature, enabled transitively by livekit-uniffi, and a third generated
Swift file. That file has to be listed in uniffi-swift.yml — the publish step
enumerates sources rather than globbing, so an unlisted component silently ships
a package that cannot build.
Costs 4,160 bytes on the release cdylib (1,107,856 -> 1,112,016), leaving 66 KiB
under the iOS size gate.
Verified: livekit-common and livekit-datatrack still build without the feature;
the three-file Swift module compiles; client-sdk-swift main builds clean against
the generated package, tests included; the xcframework carries all three headers
in one framework modulemap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The publish step enumerated each generated source, so a new UniFFI component would have shipped a package missing its types — and the action only aborts on listed-but-absent files, never on an unlisted one. livekit/publish-xcframework-action#5 makes `files` sources path patterns, the same convention as actions/upload-artifact and softprops/action-gh-release, with a trailing-slash destination meaning "directory, keep basenames". Match `Sources/<name>/*.swift` so adding a component needs no workflow change; a pattern matching nothing still fails the job. Pinned to that PR's head commit; repin to the merge commit once it lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It is only used by `ffi_types`, which is gated on the `uniffi` feature, so a consumer that doesn't want the FFI registrations shouldn't take the dependency edge. Without the feature the crate's only direct dependency is livekit-protocol again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
livekit/publish-xcframework-action#5 is merged; pin the merge commit rather than the PR branch head. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1egoman
left a comment
There was a problem hiding this comment.
Nice, I'm a fan of the livekit-common approach!
bytes::Byteswas registered withcustom_type!in bothlivekit-uniffiandlivekit-datatrack. A remote type can only be registered once per UniFFI component: each emits its ownpublic typealias BytesandFfiConverterTypeBytes, and cargo-swift compiles every component file into one Swift module —invalid redeclaration of 'Bytes'. UniFFI's own duplicated helpers coexist because they'refileprivate;custom_type!emitspublic, so they don't. Patched until now by a perl regex inswift-workarounds, which is deleted here.livekit-commonnow owns the registration and every component borrows it:Owning a registration means being a component —
custom_type!resolvescrate::UniFfiTag, and bindgen rejects metadata belonging to no namespace (Unknown namespace for CustomType). Solivekit-commongains an optionaluniffifeature, enabled transitively bylivekit-uniffi, and a third generated Swift file. That file had to be added touniffi-swift.yml— the publish step enumerates sources rather than globbing, so an unlisted component silently ships a package that can't build. Costs 4,160 B on the cdylib (1,107,856 → 1,112,016), leaving 66 KiB under the iOS gate.Verified:
livekit-commonandlivekit-datatrackstill build without the feature; the three-file Swift module compiles;client-sdk-swiftmainbuilds clean against the generated package, tests included; the xcframework carries all three headers in one framework modulemap.Alternatives for the collision
livekit-datatrackown itlivekit-datatrackto borrow it. Was the first version of this PR.uniffifromlivekit-datatrackand mirror its types with#[uniffi::remote]with_foreigntraits, and dropping#[non_exhaustive]from two enums. Separate PR.Upstream: #2933 (open) is this bug, and this PR takes the external-type route it proposes. #2126 (open) — no
#[uniffi::remote(Trait)]in 0.31 or 0.32 — is why collapsing needs trait adapters. #2802 (open) is the same conflict across separate UniFFI libraries.Alternatives for the duplication
Not addressed here. Each component re-emits the whole FFI runtime: ≈19 KB across three of them — 8.3 KB native and ~10.6 KB of Swift code+data, from two measured per-component deltas of ~4.2 KB and 5,287 B.
internalfileUpstream: #408 / #409 (closed) asked to eliminate re-declared Swift helpers and were resolved with the
UNIFFI_SHARED_Hguard plusfileprivate— duplicates left to coexist rather than removed. #2257 (closed) asked to combine the per-crate outputs; closed without a feature. #2153 (open) is the only live proposal that would share the runtime, and also documents the Python breakage: bindgen writesfrom . import livekit_datatrackinto a flat out-dir with no__init__.py, givingImportError: attempted relative import with no known parent package. #2930 (open) is the Swift → bindings-pipeline migration such a change would land on.