WpEndpointUrl & ApiEndpointUrl
#113
Conversation
| } | ||
| } | ||
|
|
||
| // TODO: Remove this because we want to build all requests within the crate |
There was a problem hiding this comment.
Not sure I agree with this – I'd be happy to have a failable initializer that won't let you make an invalid WPNetworkRequest, but I'd expect that some app developer will have a need to construct their own and use the underlying library plumbing to execute it?
WDYT?
There was a problem hiding this comment.
We are discussing this a bit on Slack. If we decide to remove it, I'll do so as part of another PR.
| let mut request = self | ||
| .client | ||
| .request(Self::request_method(wp_request.method), wp_request.url) | ||
| .request(Self::request_method(wp_request.method), wp_request.url.0) |
There was a problem hiding this comment.
Could we implement ToString on WpEndpointUrl to avoid the need for the .0?
There was a problem hiding this comment.
ToString doesn't work unfortunately. We'd have to implement IntoUrl which is part of reqwest and only available as a dev dependency. We'll likely have a cargo feature to add reqwest at which time we can do this, but until then, I think we can keep this as is.
2 participants
Adds
WpEndpointUrlto be used instead ofStringfor theurlfield ofWPNetworkRequest. AddsApiEndpointUrlto be used internally.WpEndpointUrlis a new type instead of a properstruct. I've played with both approaches and I found the new type to be more ergonomic. We are also not trying to guard against anything as we never takeWpEndpointUrlas a parameter and don't use it anywhere internally - that's whatApiEndpointUrlis for. So, the main point of this type is to communicate that this is a value that's constructed by us.ApiEndpointUrlis the internal type that each endpoint implementation will return. This is a wrapper aroundUrland allows further processing. I'd have really like to use this type insideWPNetworkRequestand only expose a function that returns the url as a string, however I can't find a way to do this with uniffi. I've spent quite a bit of time trying to get a structure where Rust consumers will get direct access toUrltype, but every solution I found made it worse either performance-wise or risky in terms of FFI. We don't really expect Rust clients to further process this value either, so practically speaking, not much value is lost by converting to a string.