Redesign interfaces (#1000) #1009
Conversation
ilslv
added
enhancement
Resolving some prior issues
interface Human {
pets(where: AnimalWhereInput):[Animal]
}
impl<'a, 'me, S> Field<S, { hash("planet") }> for &'me Droid {
type Context = ();
// To achive this we basically need to to the compilers job
// which is impossible in general case with elided lifetimes.
type Out = &'me str;
fn call(
self,
args: &::juniper::Arguments<S>,
executor: &::juniper::Executor<Self::Context, S>,
) -> Self::Out {
self.planet()
}
}But upon further digging I've found out that there is entire Redesign itselfFrom what I can tell this proposal covers all spec caveats /// Non-cryptographic hash with good dispersion to use [`str`](prim@str) in
/// const generics. See [spec] for more info.
///
/// [spec]: https://datatracker.ietf.org/doc/html/draft-eastlake-fnv-17.html
pub const fn fnv1a128(str: &str) -> u128 { ... }
// Field's argument
struct Argument<T, const N: u128>(T);
// Marker trait for defining sub-type relation.
trait IsSubtype<T: ?Sized> {
fn mark() {}
}
// Explicitly there is no blanket impl for T
// impl<T> IsSubtype<T> for T {}
// Will be generated with scalar macros
impl IsSubtype<String> for String {}
// Some manual additional work will be required
impl IsSubtype<String> for &'static str {}
impl IsSubtype<&'static str> for &'static str {}
// This will allow us to provide actually useful blanket impls
impl<T, S> IsSubtype<Vec<S>> for Vec<T> where T: IsSubtype<S> {}
impl<T, S> IsSubtype<Option<S>> for T where T: IsSubtype<S> {}
// Those impls will be generated by macros and will allow us
// to ensure that downstream implementors have all required
// arguments on the field. Also providing support for more
// `Option`al arguments.
// Example shows impls for up to 3 field arguments.
impl IsSubtype<()> for () {}
impl<S1, const N1: u128>
IsSubtype<(
Option<Argument<S1, N1>>,
)> for () {}
impl<S1, S2, const N1: u128, const N2: u128>
IsSubtype<(
Option<Argument<S1, N1>>,
Option<Argument<S2, N2>>,
)> for () {}
impl<S1, S2, S3, const N1: u128, const N2: u128, const N3: u128>
IsSubtype<(
Option<Argument<S1, N1>>,
Option<Argument<S2, N2>>,
Option<Argument<S3, N3>>,
)> for () {}
impl<T1, S1, const N1: u128>
IsSubtype<(
Argument<S1, N1>,
)> for (Argument<T1, N1>,)
where
T1: IsSubtype<S1> {}
impl<T1, S1, S2, const N1: u128, const N2: u128>
IsSubtype<(
Argument<S1, N1>,
Option<Argument<S2, N2>>
)> for (Argument<T1, N1>,)
where
T1: IsSubtype<S1>,{}
impl<T1, S1, S2, const N1: u128, const N2: u128>
IsSubtype<(
Option<Argument<S2, N2>>,
Argument<S1, N1>
)> for (Argument<T1, N1>,)
where
T1: IsSubtype<S1>, {}
impl<T1, S1, S2, S3, const N1: u128, const N2: u128, const N3: u128>
IsSubtype<(
Argument<S1, N1>,
Option<Argument<S2, N2>>,
Option<Argument<S3, N3>>,
)> for (Argument<T1, N1>,)
where
T1: IsSubtype<S1>, {}
impl<T1, S1, S2, S3, const N1: u128, const N2: u128, const N3: u128>
IsSubtype<(
Option<Argument<S2, N2>>,
Argument<S1, N1>,
Option<Argument<S3, N3>>,
)> for (Argument<T1, N1>,)
where
T1: IsSubtype<S1>, {}
impl<T1, S1, S2, S3, const N1: u128, const N2: u128, const N3: u128>
IsSubtype<(
Option<Argument<S3, N3>>,
Option<Argument<S2, N2>>,
Argument<S1, N1>,
)> for (Argument<T1, N1>,)
where
T1: IsSubtype<S1>, {}
// Field of an interface or object.
trait Field<S: ScalarValue, const N: u128> {
type Context;
type TypeInfo;
type Ret;
type ArgTypes;
fn call(
&self,
info: &Self::TypeInfo,
args: &::juniper::Arguments<S>,
executor: &::juniper::Executor<Self::Context, S>,
) -> ::juniper::ExecutionResult<S>;
}
// #[derive(GraphQLInterface)]
// #[graphql(for(Human, Droid))]
trait Character {
fn id(required: String) -> String;
}
impl IsSubtype<Human> for Character {}
impl IsSubtype<Droid> for Character {}
enum CharacterEnumValue<I1, I2> {
Human(I1),
Droid(I2),
}
type CharacterValue = CharacterEnumValue<Human, Droid>;
impl<S: ScalarValue> Field<S, { fnv1a128("id") }> for CharacterValue {
type Context = ();
type TypeInfo = ();
type Ret = String;
type ArgTypes = (Argument<String, { fnv1a128("required") }>,);
fn call(
&self,
info: &Self::TypeInfo,
args: &juniper::Arguments<S>,
executor: &juniper::Executor<Self::Context, S>,
) -> juniper::ExecutionResult<S> {
match self {
CharacterValue::Human(v) => {
let _ = <Self::Ret as IsSubtype<
<Human as Field<S, { fnv1a128("id") }>>::Ret,
>>::mark;
let _ = <Self::ArgTypes as IsSubtype<
<Human as Field<S, { fnv1a128("id") }>>::ArgTypes,
>>::mark;
<_ as Field<S, { fnv1a128("id") }>>::call(v, info, args, executor)
}
CharacterValue::Droid(v) => {
let _ = <Self::Ret as IsSubtype<
<Droid as Field<S, { fnv1a128("id") }>>::Ret,
>>::mark;
let _ = <Self::ArgTypes as IsSubtype<
<Droid as Field<S, { fnv1a128("id") }>>::ArgTypes,
>>::mark;
<_ as Field<S, { fnv1a128("id") }>>::call(v, info, args, executor)
}
}
}
}
struct Human {
id: String,
home_planet: String,
}
#[graphql_object(impl = CharacterValue)]
impl Human {
fn id(&self, requried: &str, optional: Option<i32>) -> &str {
&self.id
}
fn home_planet(&self) -> &str {
&self.primary_function
}
}
impl<S: ScalarValue> Field<S, { fnv1a128("id") }> for Human {
type Context = ();
type TypeInfo = ();
type Ret = String;
type ArgTypes = (
Argument<&'static str, { fnv1a128("required") }>,
Option<Argument<i32, { fnv1a128("optional") }>>,
);
fn call(
&self,
info: &Self::TypeInfo,
_: &juniper::Arguments<S>,
executor: &juniper::Executor<Self::Context, S>,
) -> juniper::ExecutionResult<S> {
let res = &self.id;
::juniper::IntoResolvable::into(res, executor.context()).and_then(|res| match res {
Some((ctx, r)) => executor.replaced_context(ctx).resolve_with_ctx(info, &r),
None => Ok(::juniper::Value::null()),
})
}
}
struct Droid {
id: String,
primary_function: String,
}
#[graphql_object(impl = CharacterValue)]
impl Droid {
fn id(&self, requied: String) -> &str {
&self.id
}
fn primary_function(&self) -> &str {
&self.primary_function
}
}
impl<S: ScalarValue> Field<S, { fnv1a128("id") }> for Droid {
type Context = ();
type TypeInfo = ();
type Ret = &'static str;
type ArgTypes = (Argument<String, { fnv1a128("required") }>,);
fn call(
&self,
info: &Self::TypeInfo,
_: &juniper::Arguments<S>,
executor: &juniper::Executor<Self::Context, S>,
) -> juniper::ExecutionResult<S> {
let res = self.id();
::juniper::IntoResolvable::into(res, executor.context()).and_then(|res| match res {
Some((ctx, r)) => executor.replaced_context(ctx).resolve_with_ctx(info, &r),
None => Ok(::juniper::Value::null()),
})
}
} |
Yup, similarly to
Good finding 👍 We still can have the type as associated for type-level reffering and assertions, but the method signature is better be compliant with I do tend to think that this designt will evolve further over our codebase, because granularity over fields unlocks for us many longely desired capabilities (eventually we will be able to have multiple |
|
@ilslv as for the proposal itself:
As we don't have some third-party inputs, so the Hmm... but
These a better generated on object macro expansion, not interface. Ideally, GraphQL interface shouldn't explicitly mention its implementors (but implementorst do mention ther interfaces explicitly), though due to
The idea looks neat, but I do feel unsure about it. You're trying here to build subtyping on top of Rust trait system. That sounds like something quite troublesome in long prespective. As Rust trait system has no notion of subtyping, you propose to specify subtyping relation manually (like between I'd argue, that we do not need subtyping construct to check whether 2 different Rust types are considered the same GraphQL type. We better have here some projection into string literal ( Abusing May you look better into it and try to piggyback this part of design on I do realize that we still would need some form of subtyping for interfaces hierarchy. But it seems that it may be simplified for this particular case. |
|
@ilslv wait...
are these the same fields according to the spec? They're seem to be different ones. |
|
Yeah, of course:
Shiiii... 🤯 |
|
Now these make much more sense:
The thing that still bothers here, is that we express relations between Rust types here. While what we really need is to express subtyping relations between GraphQL types.
Maybe this one confuses me the most as you're abusing GraphQL type subtyping machinery to prove the fact that 2 different Rust types represent the same GraphQL type. That I've talked about:
Maybe for these particular assertions we need another machinery along? So we express GraphQL signatures inheritance via |
Yes, all of the above does work on stable Rust. Regarding error messages, there are not so readable, but better then nothing:
I've tried doing it with something like trait Type {
const NAME: &'static str
}
trait SubTypes {
const NAMES: &'static [&'static str]
}This does work, until we wrap impl<T, S> IsSubtype<Vec<S>> for Vec<T> where T: IsSubtype<S> {}Especially when you start stacking up those But I'll try to return to this design. |
|
I guess I've found a way to deal with subtyping in const assertions // Named GraphQL type.
trait Type {
const NAME: &'static str;
}
// Transperent for Type, but see WrappedType for differences.
impl<T: Type> Type for Option<T> {
const NAME: &'static str = T::NAME;
}
// Transperent for Type, but see WrappedType for differences.
impl<T: Type> Type for Vec<T> {
const NAME: &'static str = T::NAME;
}
impl Type for String {
const NAME: &'static str = "String";
}
impl<'a> Type for &'a str {
const NAME: &'static str = "String";
}
impl Type for i32 {
const NAME: &'static str = "Int";
}
// Subtypes of a GraphQL type, including self.
trait SubTypes {
const NAMES: &'static [&'static str];
}
// Transperent for SubTypes, but see WrappedType for differences.
impl<T: SubTypes> SubTypes for Option<T> {
const NAMES: &'static [&'static str] = T::NAMES;
}
// Transperent for SubTypes, but see WrappedType for differences.
impl<T: SubTypes> SubTypes for Vec<T> {
const NAMES: &'static [&'static str] = T::NAMES;
}
impl SubTypes for String {
const NAMES: &'static [&'static str] = &[<Self as Type>::NAME];
}
impl<'a> SubTypes for &'a str {
const NAMES: &'static [&'static str] = &[<Self as Type>::NAME];
}
impl<'a> SubTypes for i32 {
const NAMES: &'static [&'static str] = &[<Self as Type>::NAME];
}
// The main idea of this trait is to represent GraphQL type wrapped in lists
// and options as a number:
// - We always start with 1 as a base type
// - If wrapped in Option we multiply by 10 and add 2, so n -> n2
// - If wrapped in Vec we multiply by 10 and add 3, so n -> n3
//
// Example:
// `[[Int]!] - <Option<Vec<Vec<Option<i32>>>> as WrappedType>::N = 12332`
trait WrappedType {
const N: u128;
}
impl<T: WrappedType> WrappedType for Option<T> {
const N: u128 = T::N * 10 + 2;
}
impl<T: WrappedType> WrappedType for Vec<T> {
const N: u128 = T::N * 10 + 3;
}
impl WrappedType for String {
const N: u128 = 1;
}
impl<'a> WrappedType for &'a str {
const N: u128 = 1;
}
impl WrappedType for i32 {
const N: u128 = 1;
}
trait Field<S: ScalarValue, const N: u128> {
type Context;
type TypeInfo;
// List of all suptypes for a return type.
const RETURN_SUB_TYPES: &'static [&'static str];
// WrappedType::N for a return type.
const RETURN_WRAPPED_VALUE: u128;
// List of name, type, and WrappedType::N of a field's arguments.
const ARGUMENTS: &'static [(&'static str, &'static str, u128)];
fn call(
&self,
info: &Self::TypeInfo,
args: &::juniper::Arguments<S>,
executor: &::juniper::Executor<Self::Context, S>,
) -> ::juniper::ExecutionResult<S>;
}
impl<S: ScalarValue> Field<S, { fnv1a128("id") }> for CharacterValue {
type Context = ();
type TypeInfo = ();
const RETURN_SUB_TYPES: &'static [&'static str] = <Option<String> as SubTypes>::NAMES;
const RETURN_WRAPPED_VALUE: u128 = <Option<String> as WrappedType>::N;
// Returns an optional string ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
const ARGUMENTS: &'static [(&'static str, &'static str, u128)] =
&[("required", "String", 1)];
fn call(
&self,
info: &Self::TypeInfo,
args: &juniper::Arguments<S>,
executor: &juniper::Executor<Self::Context, S>,
) -> juniper::ExecutionResult<S> {
match self {
CharacterValue::Human(v) => {
const _: () = assert!(is_superset(
<Option<String> as SubTypes>::NAMES,
<Human as Field<DefaultScalarValue, { fnv1a128("id") }>>::SUB_TYPES,
));
const _: () = assert!(is_subtype(
<Option<String> as WrappedType>::N,
<Human as Field<DefaultScalarValue, { fnv1a128("id") }>>::WRAPPED_VALUE,
));
const _: () = assert!(is_valid_field_args(
<CharacterValue as Field<DefaultScalarValue, { fnv1a128("id") }>>::ARGUMENTS,
<Human as Field<DefaultScalarValue, { fnv1a128("id") }>>::ARGUMENTS,
));
<_ as Field<S, { fnv1a128("id") }>>::call(v, info, args, executor)
}
CharacterValue::Droid(v) => {
const _: () = assert!(is_superset(
<Option<String> as SubTypes>::NAMES,
<Droid as Field<DefaultScalarValue, { fnv1a128("id") }>>::SUB_TYPES,
));
const _: () = assert!(is_subtype(
<Option<String> as WrappedType>::N,
<Droid as Field<DefaultScalarValue, { fnv1a128("id") }>>::WRAPPED_VALUE,
));
const _: () = assert!(is_valid_field_args(
<CharacterValue as Field<DefaultScalarValue, { fnv1a128("id") }>>::ARGUMENTS,
<Droid as Field<DefaultScalarValue, { fnv1a128("id") }>>::ARGUMENTS,
));
<_ as Field<S, { fnv1a128("id") }>>::call(v, info, args, executor)
}
}
}
}
impl<S: ScalarValue> Field<S, { fnv1a128("id") }> for Human {
type Context = ();
type TypeInfo = ();
const RETURN_SUB_TYPES: &'static [&'static str] = <String as SubTypes>::NAMES;
const RETURN_WRAPPED_VALUE: u128 = <String as WrappedType>::N;
// Returns an cocreet string ^^^^^^^^^^^^^^^^^^^^^^^^^^
const ARGUMENTS: &'static [(&'static str, &'static str, u128)] =
&[("required", "String", 1), ("optional", "String", 12)];
// Additional optional argument ^^^^^^^^^^^^^^^^^^^^^^^^^^
fn call(
&self,
info: &Self::TypeInfo,
_: &juniper::Arguments<S>,
executor: &juniper::Executor<Self::Context, S>,
) -> juniper::ExecutionResult<S> {
let res = &self.id;
::juniper::IntoResolvable::into(res, executor.context()).and_then(|res| match res {
Some((ctx, r)) => executor.replaced_context(ctx).resolve_with_ctx(info, &r),
None => Ok(::juniper::Value::null()),
})
}
}
impl<S: ScalarValue> Field<S, { fnv1a128("id") }> for Droid {
type Context = ();
type TypeInfo = ();
const RETURN_SUB_TYPES: &'static [&'static str] = <Option<&str> as SubTypes>::NAMES;
const RETURN_WRAPPED_VALUE: u128 = <Option<&str> as WrappedType>::N;
// Returns an optional &str ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// Note that we don't have to dance around lifetimes 🎉
const ARGUMENTS: &'static [(&'static str, &'static str, u128)] =
&[("required", "String", 1), ("optional", "Int", 12)];
// Additional optional argument ^^^^^^^^^^^^^^^^^^^^^^^
fn call(
&self,
info: &Self::TypeInfo,
_: &juniper::Arguments<S>,
executor: &juniper::Executor<Self::Context, S>,
) -> juniper::ExecutionResult<S> {
let res = self.id();
::juniper::IntoResolvable::into(res, executor.context()).and_then(|res| match res {
Some((ctx, r)) => executor.replaced_context(ctx).resolve_with_ctx(info, &r),
None => Ok(::juniper::Value::null()),
})
}
}
// Checks possibility of subtyping based on WrappedType.
const fn is_subtype(ty: u128, subtype: u128) -> bool {
let ty_current = ty % 10;
let subtype_current = subtype % 10;
if ty_current == subtype_current {
if ty_current == 1 {
// Recursion base.
true
} else {
// Continue comparing
is_subtype(ty / 10, subtype / 10)
}
} else if ty_current == 2 {
// If optional, try stripping it.
is_subtype(ty / 10, subtype)
} else {
false
}
} |
|
@ilslv now you're talking! Regarding the trait WrappedType {
const NAMES: &'static [&'static str];
}
impl WrappedType for String {
const NAMES: &'static [&'static str] = &[<Self as Type>::NAME];
}
impl<'a> WrappedType for &'a str {
const NAMES: &'static [&'static str] = &[<Self as Type>::NAME];
}
impl<T: WrappedType> WrappedType for Option<T> {
const NAMES: &'static [&'static str] = prepend("Nullable", T::NAMES);
}
impl<T: WrappedType> WrappedType for Vec<T> {
const NAMES: &'static [&'static str] = prepend("List", T::NAMES);
}So now, instead of numbers like (To comply with GraphQL spec I would like also revert the Anything else seems fine. Elegant solution 👍 Side note: it would be also nice, if we name const asserting functions similarly to algos in spec. |
|
@ilslv another side note: type Type = &'static str;
type Types = &'static [Type];
type Name = &'static str;
type FieldName = u128;
trait Field<S: ScalarValue, const N: FieldName> {
type Context;
type TypeInfo;
const RETURN_SUB_TYPES: Types;
const RETURN_WRAPPED_TYPE: Types;
const ARGUMENTS: &'static [(Name, Type, Types)];
fn call(
&self,
info: &Self::TypeInfo,
args: &::juniper::Arguments<S>,
executor: &::juniper::Executor<Self::Context, S>,
) -> ::juniper::ExecutionResult<S>;
} |
|
Actually, I don't think that impl<T: WrappedType> WrappedType for Option<T> {
const NAMES: &'static [&'static str] = prepend("Nullable", T::NAMES);
}Main limitation is that we can't get const fn prepend(val: Type, arr: Types) -> Types {
const RES_LEN: usize = arr.len() + 1;
// ------------- ^^^ non-constant value
// |
// help: consider using `let` instead of `const`: `let RES_LEN`
const RES_ARR: [Type; RES_LEN] = {
let mut res: [Type; RES_LEN] = [val; RES_LEN];
let mut i = 0;
while i < arr.len() {
res[i + 1] = arr[i];
i += 1;
}
res
};
const RES_SLICE: Types = &RES_ARR;
RES_SLICE
}Solution should be const generics, but current limitations don't allow us to do that const fn prepend<const N: usize>(val: Type, arr: Types) -> Types {
// - const parameter from outer function
const RES_LEN: usize = N + 1;
// ^ use of generic parameter from outer function
const RES_ARR: [Type; RES_LEN] = {
let mut res: [Type; RES_LEN] = [val; RES_LEN];
let mut i = 0;
while i < arr.len() {
res[i + 1] = arr[i];
i += 1;
}
res
};
const RES_SLICE: Types = &RES_ARR;
RES_SLICE
}And even in-place impl<S, T: WrappedTypeStr<S>> WrappedTypeStr<S> for Option<T> {
// - type parameter from outer function
const VALUES: Types = {
const RES_LEN: usize = T::VALUES.len() + 1;
// ^^^^^^^^^ use of generic parameter from outer function
const RES_ARR: [juniper::macros::helper::Type; RES_LEN] = {
let mut res: [juniper::macros::helper::Type; RES_LEN] = ["Nullable"; RES_LEN];
let mut i = 0;
while i < RES_LEN - 1 {
res[i + 1] = T::VALUES[i];
i += 1;
}
res
};
const RES_SLICE: juniper::macros::helper::Types = &RES_ARR;
RES_SLICE
};
} |
|
@tyranron already tried that, but fundamental problems are the same: playground And moving |
|
@ilslv yeah, the problem is So ok, if we cannot provide other better solution at the moment, let's do it with encoding in numbers and place there a huge |
|
@tyranron I've played with that idea a bit, but no luck. We can't use nested tuples in
trait T {
const CONST;
type TYPE;
}
impl T for Something {
// works fine.
const CONST = <&str as SomeOtherTrait>::CONST;
type TYPE = <&str as SomeOtherTrait>::TYPE;
// ^ expected named lifetime parameter
} |
|
To sum up changes done in this PR. As GraphQL interfaces are more like Go's structural interfaces, rather than Rust traits that are more like typeclasses, we've decided to ditch #[graphql_interface(for = [Human, Droid])]
trait Character {
fn id(&self) -> String;
}
#[derive(GraphQLObject)]
#[graphql(impl = CharacterValue)]
struct Human {
// Inner machinery borrows String instead of cloning,
// despite trait returning String.
id: String,
home_planet: String,
}
struct Droid {
id: String,
primary_function: String,
}
#[graphql_object(impl = CharacterValue)]
impl Droid {
// Returning `&str` instead of `String` is also fine.
fn id(&self) -> &str {
&self.id
}
fn primary_function(&self) -> &str {
&self.primary_function
}
}In case some of the fields/arguments are missing or have incompatible types user will get easy-to-understand error messages: All those changes not only lay ground for future suport of interfaces implementing other interfaces, but also fully support June 2018 GraphQL spec features which weren't supported before:
#[graphql_interface(for = Droid)]
trait Character {
fn id(&self) -> String;
}
struct Droid {
id: String,
primary_function: String,
}
#[graphql_object(impl = CharacterValue)]
impl Droid {
fn id(&self, is_present: Option<bool>) -> &str {
is_present
.unwrap_or_default()
.then(|| self.as_str())
.unwrap_or("missing")
}
fn primary_function(&self) -> &str {
&self.primary_function
}
}In case additional field isn't nullable, user will get similar error mesage
#[derive(GraphQLObject)]
struct Strength {
value: i32,
}
#[derive(GraphQLObject)]
struct Knowledge {
value: i32,
}
#[allow(dead_code)]
#[derive(GraphQLUnion)]
enum KeyFeature {
Strength(Strength),
Knowledge(Knowledge),
}
#[graphql_interface(for = [Human, Droid])]
trait Character {
fn id(&self) -> Option<String>;
fn key_feature(&self) -> KeyFeature;
}
#[derive(GraphQLObject)]
#[graphql(impl = CharacterValue)]
struct Human {
id: String,
home_planet: String,
key_feature: Knowledge,
}
struct Droid {
id: String,
primary_function: String,
strength: i32,
}
#[graphql_object(impl = CharacterValue)]
impl Droid {
fn id(&self) -> &str {
&self.id
}
fn primary_function(&self) -> &str {
&self.primary_function
}
fn key_feature(&self) -> Strength {
Strength {
value: self.strength,
}
}
}Error messages are also understandable: Also support for the explicit #[graphql_interface(for = Droid, context = Database)]
/// A character in the Star Wars Trilogy
pub trait Character {
/// The id of the character
fn id(&self) -> &str;
/// The name of the character
fn name(&self) -> Option<&str>;
/// The friends of the character
fn friends(&self, ctx: &Database) -> Vec<CharacterValue>;
/// Which movies they appear in
fn appears_in(&self) -> &[Episode];
#[graphql(ignore)]
fn friends_ids(&self) -> &[String];
}
pub struct Droid {
id: String,
name: String,
friend_ids: Vec<String>,
appears_in: Vec<Episode>,
#[allow(dead_code)]
secret_backstory: Option<String>,
primary_function: Option<String>,
}
/// A mechanical creature in the Star Wars universe.
#[graphql_object(context = Database, impl = CharacterValue)]
impl Droid {
/// The id of the droid
fn id(&self) -> &str {
&self.id
}
/// The name of the droid
fn name(&self) -> Option<&str> {
Some(self.name.as_str())
}
/// The friends of the droid
fn friends(&self, ctx: &Database) -> Vec<CharacterValue> {
ctx.get_friends(self) // Works fine
}
/// Which movies they appear in
fn appears_in(&self) -> &[Episode] {
&self.appears_in
}
/// The primary function of the droid
fn primary_function(&self) -> &Option<String> {
&self.primary_function
}
}
impl Character for Droid {
fn id(&self) -> &str {
&self.id
}
fn name(&self) -> Option<&str> {
Some(&self.name)
}
fn friends(&self, ctx: &Database) -> Vec<CharacterValue> {
ctx.get_friends(self) // Works fine
}
fn appears_in(&self) -> &[Episode] {
&self.appears_in
}
fn friends_ids(&self) -> &[String] {
&self.friend_ids
}
}
#[derive(Default, Clone)]
pub struct Database {
humans: HashMap<String, Human>,
droids: HashMap<String, Droid>,
}
impl Context for Database {}
impl Database {
pub fn get_friends(&self, c: &dyn Character) -> Vec<CharacterValue> {
// Accepts &dyn Trait ^^^^^^^^^^^^^^
c.friends_ids()
.iter()
.flat_map(|id| self.get_character(id))
.collect()
}
}ack: @LegNeato @davidpdrsn |
FCM |
| @@ -652,49 +499,25 @@ mod trivial_async { | |||
| async fn id(&self) -> &str; | |||
There was a problem hiding this comment.
Well, I think we should factor out async_trait here completely and just ignore async keyworf in fn signatures as it's not related to GraphQL types. On the current stable it will give an error, which is good, and once real async tratis will land into stable, we just don't bother.
There was a problem hiding this comment.
So this whole tests should test async fields of GraphQL objects rather that of GraphQL interface.
There was a problem hiding this comment.
@tyranron the trouble with current approach is implementation that non-async trait method can be implemented with non-async object method only, while async trait method can be implemented with both async and non-async object methods. This happens because non-async methods are represented with both Field and AsyncField traits, while async ones with AsyncField only. I should definitely add pretty assertion for it.
So implementing non-async trait method with async object field will error.
#[graphql_interface]
trait Character {
fn id(&self) -> String;
}
impl Human {
// Errors
async fn id(&seld) -> String {
/// ...
}
}There was a problem hiding this comment.
I don't think this is possible to get rid of asyncness in traits, as they are represented with enums, which are implementing Field and/or AsyncField reflection traits.
Hm, actually I guess we can with some autoref-based specialisation, as we have all exact types. So we would panic on trying to resolve sync enum method with async impl.
UPD: yep, autoref-based specialisation works
There was a problem hiding this comment.
Also looking at test case
#[graphql_object(scalar = S: ScalarValue, impl = CharacterValue<S>)]
impl Human {
async fn id<'a, S: ScalarValue>(&self, executor: &'a Executor<'_, '_, (), S>) -> &'a str {
executor.look_ahead().field_name()
}
fn home_planet(&self) -> &str {
&self.home_planet
}
async fn info<'b>(&'b self, _arg: Option<i32>) -> &'b str {
&self.home_planet
}
}I guess we can transform this into
#[graphql_object(scalar = S, impl = CharacterValue<S>)]
// No `: ScalarValue` ^
impl Human {
async fn id<'a, S: ScalarValue>(&self, executor: &'a Executor<'_, '_, (), S>) -> &'a str {
executor.look_ahead().field_name()
}
fn home_planet(&self) -> &str {
&self.home_planet
}
async fn info<'b>(&'b self, _arg: Option<i32>) -> &'b str {
&self.home_planet
}
}by searching for S generic not only in impl, but also in object methods
There was a problem hiding this comment.
I guess we can transform this into
Not the case for this PR. Once we land it, we will reconsider all the attributes in a separate PR anyway.
But I do tend to have a visual separation between types and type params for library user.
the trouble with current approach is implementation that non-
asynctrait method can be implemented with non-asyncobject method only, whileasynctrait method can be implemented with bothasyncand non-asyncobject methods. This happens because non-asyncmethods are represented with bothFieldandAsyncFieldtraits, whileasyncones withAsyncFieldonly. I should definitely add pretty assertion for it.
Well I don't think that deciding such things should be a library user burden. Ideally, we don't want sync/async coloring to be a thing here. We do want working it transparantely, providing a meaningful compilation error if something goes wrong.
So... as a GraphQL interface is just a bunch of fields, which are checked structuraly by signatures, the asyncness notion doesn't depend on the interface itself, it rather depends on how the objects implementing the this interface_ are resolved. So our job here is to have pretty const assertions and that's it.
| @@ -721,20 +544,6 @@ mod trivial_async { | |||
| .into(), | |||
| } | |||
| } | |||
|
|
|||
| fn hero<S: ScalarValue + Send + Sync>(&self) -> Box<DynHero<'_, S>> { | |||
There was a problem hiding this comment.
With dyn being ripped out we also shouldn't need to use something like #[graphql_object(scalar = S: ScalarValue + Send + Sync)] here in tests, just #[graphql_object] should be enough.
| }}"#, | ||
| interface, | ||
| ); | ||
| let doc = r#"{ |
There was a problem hiding this comment.
This should be const DOC: &str to comply with other tests in this file.
| } | ||
|
|
||
| fn info(&self) -> &str; | ||
| async fn info(&self) -> String; |
There was a problem hiding this comment.
And this explicit_async section maybe ripped out too, as we don't have now notion of asyncness regarding GraphQL interfaces.
There was a problem hiding this comment.
Same goes for generic_async and generic_lifetime_async sections.
|
Nice job, can't wait for this feature, wish this be merged soon. |
Part of #1000
Resolves #113
Partially resolves #814
Synopsis
Current approach to the interfaces most of the times works fine, but can't fully support GraphQL spec and may be misused.
Solution
Checklist
Draft:prefixDraft:prefix is removed