-
Notifications
You must be signed in to change notification settings - Fork 430
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
Implement ScalarValue
derive macro
#1025
Conversation
FCM
|
# Conflicts: # integration_tests/juniper_tests/src/codegen/mod.rs # juniper/CHANGELOG.md # juniper_codegen/src/lib.rs
# Conflicts: # integration_tests/juniper_tests/src/codegen/mod.rs # integration_tests/juniper_tests/src/codegen/scalar_attr_derive_input.rs # integration_tests/juniper_tests/src/codegen/scalar_attr_type_alias.rs # integration_tests/juniper_tests/src/codegen/scalar_value_transparent.rs # integration_tests/juniper_tests/src/custom_scalar.rs # juniper/CHANGELOG.md # juniper/src/lib.rs # juniper/src/types/scalars.rs # juniper/src/value/scalar.rs # juniper_codegen/src/derive_scalar_value.rs # juniper_codegen/src/graphql_scalar/attr.rs # juniper_codegen/src/graphql_scalar/derive.rs # juniper_codegen/src/graphql_scalar/mod.rs # juniper_codegen/src/lib.rs # juniper_codegen/src/result.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilslv let's reshape it a little bit more.
juniper_codegen/src/lib.rs
Outdated
@@ -549,6 +550,114 @@ pub fn graphql_scalar(attr: TokenStream, body: TokenStream) -> TokenStream { | |||
.into() | |||
} | |||
|
|||
/// `#[derive(GraphQLScalarValue)]` macro for deriving a [`ScalarValue`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's name it #[derive(ScalarValue)]
directly. There is no sense stopping us from that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tyranron should we also rename attributes from #[graphql(...)]
to #[scalar_value(...)]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilslv I think #[value(...)]
will be just OK.
juniper_codegen/src/lib.rs
Outdated
/// | ||
/// To derive [`ScalarValue`] on enum you should mark corresponding enum | ||
/// variants with `as_int`/`as_float`/`as_string`/`into_string`/`as_str`/ | ||
/// `as_boolean` attributes (names correspond to [`ScalarValue`] required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename as_bool
directly in the trait, so it complies with Rust types naming. And remove as_boolean
variant totally.
GraphQLScalarValue
derive macroScalarValue
derive macro
Synopsis
After redesign
GraphQLScalar
value lost ability to deriveScalarValue
on enums.Solution
Implement
GraphQLScalarValue
derive macro to implementScalarValue
on enums.Checklist
Draft:
prefixDraft:
prefix is removed