servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.27k stars 317 forks source link

Add UniFFI support impl FfiConverter for Url - gated behind `uniffi` feature flag. #891

Closed Sajjon closed 6 months ago

Sajjon commented 6 months ago

As UniFFI gathers support, it would be convenient for Url to be able to be used with UniFFI!

Inspired by @novacrazy's work in https://github.com/Lantern-chat/iso8601-timestamp/pull/8 credits goes to Nova!

However, as pointed out by @mhammond at UniFFI users can instead of using this feature flag (part of Url) do it "manually" in their own project like so:

https://github.com/mozilla/uniffi-rs/blob/main/examples/custom-types/src/lib.rs#L36-L47

So might not be worth merging.

valenting commented 6 months ago

However, as pointed out by @mhammond at UniFFI users can instead of using this feature flag (part of Url) do it "manually" in their own project like so: https://github.com/mozilla/uniffi-rs/blob/main/examples/custom-types/src/lib.rs#L36-L47

So might not be worth merging.

Thank you for the PR. I'm not very familiar with UniFFI, so I'm not totally convinced the Url crate needs to support it just yet. If it becomes something that a large share of the crates need, then it would make sense to add the support in the Url crate. As you mention, users can already do this manually, and the FfiConverter you added round-trips through to_string and parse anyway, so it has no inherent advantage to a user provided implementation.

I'll close this for now, but I'm open to reopening if the user needs change in the future.