You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add first class support for newtypes in Queryable and QueryableByName
A consistent problem that's existed for us has been "what do we do about
library X when Diesel doesn't want to care about X, and X doesn't want
to care about Diesel?" Another permutation of this problem has been
folks wanting type conversions that we're never going to support (such
as `impl FromSql<Binary, Sqlite> for Uuid`).
Ultimately the answer for this has pretty much always come down to
newtypes. Whether you're doing it in your app, or having them provided
by a library, that's the only real solution to the orphan rule here.
This commit introduces a new API to all our relevant deserialization
derives: `#[deserialize_as]`. The API is pretty straightforward. You
give us the name of a type. That type has to satisfy whatever
constraints would exist if you used that type directly on your struct.
It also has to implement `Into` for whatever type is on your struct.
This is similar to, but slightly different than
`#[serde(deserialize_with)]`, in that it takes a type name rather than a
function. We cannot mirror serde's API here, as we use this in a
situation where we cannot infer the type you need based on the argument
to the function.
I intend to follow this PR with an equivalent `#[serialize_as]`
attribute for `Insertable` and `AsChangeset`.
Unresolved Questions
--------------------
I'm really not liking using `#[diesel(...)]` to avoid namespace issues.
The main problem with doing that is that we cannot communicate to the
compiler what is allowed inside of it. We also cannot use our normal
"warn if there's unrecognized options" strategy in this case, since
there can be multiple derives that use `#[diesel(...)]` on the same
attribute.
The result of this is that we silently ignore any abnormal options in a
`#[diesel]` attribute. While writing this, I wrote `deserialize_with`
instead of `deserialize_as` twice and couldn't figure out what went
wront. There's a few options here, but I'm not sure which is best.
One would be for us to maintain a global list of all options to
`#[diesel]` that all derives recognize, and warn if something doesn't
match that list. The main downsides to that implemenation is that it's
very brittle, and we won't warn if you use something only recognized by
`#[derive(QueryableByName)]` when that derive isn't present.
The other option is to deprecated `#[diesel(...)]`, and replace all
uses with `#[diesel_...]`. This is the option I'm currently leaning
towards (though I do not think it needs to be done in this PR), since
the only downside is that if you are passing more than one thing to
`#[diesel]`, it will need to be on multiple lines. I do not think we
currently have any cases where that is even reasonable.
0 commit comments