-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
references #194
base: main
Are you sure you want to change the base?
references #194
Conversation
GAT? 👀 |
588a044
to
bf800ea
Compare
this mostly works, only getting one error at call site for
the library works though, you just cant get the response i'm not very familiar with this type of error, I think I need to make different responses but it's unclear. Maybe it's a bug even |
280: refactor: Use (mostly) References in Requests r=Emilgardis a=Nerixyz Since #194 only implemented a few references and a lot of changes happened in the meantime, I went ahead and tried to refactor (most) request fields to use references. "_most_" in this case means that (1) `after`/`cursor`s remain owned (since I couldn't get pagination to work) and (2) eventsub requests remain owned. I think having (mostly) non-owned requests is already a big step. * Some fields are `&str` or `&types::TypeRef` but should be `Cow<'_, str>` for the `Deserialize` impl (since they might be encoded as escaped characters in JSON). These are mainly user-inputs. user-ids, user-logins, UUIDs and similar can remain, since they aren't escaped in JSON. I think it's fair to make sure `Deserialize` works for JSON, but it shouldn't need to work for _all_ encodings. * The use of `Cow<'_, [&types::TypeRef]>` makes passing (static) slices a bit ugly, because `&["foo"]` is `&[&str; 1]` and `&["foo"][..]` is `&[&str]` and `Cow`'s `Into` only accepts the latter. Co-authored-by: Nerixyz <nerixdev@outlook.de> Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
6bb02b1
to
644af39
Compare
644af39
to
4d0ae22
Compare
Now getting this, and not sure how to proceed... |
```rust ❯ cargo +stable check --no-default-features -F helix,reqwest Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api) error[E0277]: the trait bound `<D as Yokeable<'_>>::Output: helix::_::_serde::Deserialize<'_>` is not satisfied --> src/helix/client.rs:156:41 | 156 | } = <R>::parse_response(Some(request), &uri, &response)?; | ------------------- ^^^^^^^^^^^^^ the trait `helix::_::_serde::Deserialize<'_>` is not implemented for `<D as Yokeable<'_>>::Output` | | | required by a bound introduced by this call | note: required by a bound in `helix::request::RequestGet::parse_response` --> src/helix/request.rs:396:57 | 389 | fn parse_response<'r>( | -------------- required by a bound in this ... 396 | <Self::Response as yoke::Yokeable<'r>>::Output: serde::Deserialize<'r>, | ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `helix::request::RequestGet::parse_response` help: consider further restricting the associated type | 130 | C: Send, <D as Yokeable<'_>>::Output: helix::_::_serde::Deserialize<'_> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ For more information about this error, try `rustc --explain E0277`. error: could not compile `twitch_api` due to previous error ```
```rust ❯ cargo +stable check --no-default-features -F helix,reqwest Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api) error: implementation of `helix::_::_serde::Deserialize` is not general enough --> src/helix/client/client_ext.rs:37:9 | 37 | self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `helix::_::_serde::Deserialize` is not general enough | = note: `YokeTraitHack<Vec<get_users::User<'static>>>` must implement `helix::_::_serde::Deserialize<'0>`, for any lifetime `'0`... = note: ...but it actually implements `helix::_::_serde::Deserialize<'1>`, for some specific lifetime `'1` --> src/helix/request.rs:429:42 | 429 | let response: InnerResponse<_> = parse_json(response, true).map_err(|e| { | ^^^^^^^^^^ the trait `helix::_::_serde::Deserialize<'_>` is not implemented for `<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output` | note: required for `InnerResponse<<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output>` to implement `helix::_::_serde::Deserialize<'_>` --> src/helix/mod.rs:66:21 | 66 | #[derive(PartialEq, Deserialize, Debug)] | ^^^^^^^^^^^ 67 | struct InnerResponse<D> { | ^^^^^^^^^^^^^^^^ note: required by a bound in `parse_json` --> src/lib.rs:276:26 | 276 | pub fn parse_json<'a, T: serde::Deserialize<'a>>( | ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `parse_json` = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider further restricting the associated type | 427 | for<'d> yoke::trait_hack::YokeTraitHack<<Self::Response as yoke::Yokeable<'d>>::Output>: serde::Deserialize<'d>, <<Self as helix::request::Request>::Response as Yokeable<'r>>::Output: helix::_::_serde::Deserialize<'_> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ For more information about this error, try `rustc --explain E0277`. error: could not compile `twitch_api` due to 2 previous errors ```
and now
|
Hitting serde limits now
and almost there!
not sure how to solve this, or what the issue actually is. I think this is why, I might be wrong: pub async fn req_get<'req, R, T>(
&'a self,
// 1. R is Foo<'foo>
request: R,
token: &T,
) -> Result<
Response<R, yoke::Yoke<R::Response<'static>, Vec<u8>>>,
ClientRequestError<<C as crate::HttpClient<'a>>::Error>,
>
where
R: Request + RequestGet, // Here, due to something
// 2. Because R::Response is Yokable only when R::Response<'static>
// R::Response<'de> in the later call becomes 'static
for<'y> R::Response<'static>: yoke::Yokeable<'y>,
T: TwitchToken + ?Sized,
C: Send,
{ ... } or I'm doing something totally wrong |
``` error: implementation of `helix::_::_serde::Deserialize` is not general enough --> src/helix/client/client_ext.rs:37:9 | 37 | self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `helix::_::_serde::Deserialize` is not general enough | = note: `YokeTraitHack<Vec<get_users::User<'static>>>` must implement `helix::_::_serde::Deserialize<'0>`, for any lifetime `'0`... = note: ...but it actually implements `helix::_::_serde::Deserialize<'1>`, for some specific lifetime `'1` ```
So, I feel like the current code is the most correct.
I think this is because fn req_get()
where:
for<'y> R::Response: yoke::Yokeable<'y>,
for<'y> yoke::trait_hack::YokeTraitHack<<R::Response as yoke::Yokeable<'y>>::Output>:
serde::Deserialize<'y>, doesn't say
but instead says (for some reason)
is this a bug? might be rust-lang/rust#70263 I might minimize this a bit and then (unless I solve it myself) see if the icu4x people have any ideas |
This is basically the same issue as we got for #284, but this case should be solveable as we control all surfaces |
Side note: Making the bound fn req_get()
where:
for<'y> R::Response: yoke::Yokeable<'y>,
for<'y> <R::Response as yoke::Yokeable<'y>>::Output:
serde::Deserialize<'y>, gives the following error instead, (interestingly, it's also malformed, no it does not seem to be the same as rust-lang/rust#89196 which is what
|
Ehh, turns out it actually does work as is, it's just the |
this works, but it sucks... imo it's very cluttered... #[zerovec::make_varule(ExampleResponseULE)]
#[zerovec::derive(Deserialize)]
#[derive(serde::Deserialize, yoke::Yokeable, PartialEq, Eq, PartialOrd, Ord)]
pub struct ExampleResponse<'a> {
#[serde(borrow)]
pub a_thing: Cow<'a, str>,
}
impl Request for ExampleRequest {
type Response = zerovec::VarZeroVec<'static, ExampleResponseULE>;
}
impl RequestGet for Example and the Maybe an arena on the client? feels like overkill. Might be best to just make it possible to do this, but in a different way, either self-referential or something else. example impl Response<R,D> {
fn data(self) -> Result<R::Response, _> {
<R>::parse_response_no_fail(data)
}
// `Output` is yoke::Yokable::Output
fn data_ref<'a>(&'a self) -> Result<R::Response::Output<'a>, _> {
<R>::parse_response_ref_no_fail(data, uri, self.data)
}
}
// Owned
let user: User<'static> = client.get_user_by_login("twitchdev").await?.data()?;
// Borrowed
let resp = client.get_user_by_login("twitchdev").await?;
let user: User<'_> = resp.data_ref()?; we might hit the same issues though with the |
No description provided.