Skip to content

Strip InputValue from from_input for GraphQL scalars #1327

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

Merged
merged 22 commits into from
Jun 13, 2025

Conversation

tyranron
Copy link
Member

@tyranron tyranron commented Jun 10, 2025

Synopsis

At the moment, defining a GraphQL scalar requires a from_input() function, which accepts InputValue as its argument:

#[derive(GraphQLScalar)]
#[graphql(parse_token(String, i32))]
pub struct ID(String);

impl ID {
    fn to_output<S: ScalarValue>(&self) -> Value<S> {
        Value::scalar(self.0.clone())
    }

    fn from_input<S: ScalarValue>(v: &InputValue<S>) -> Result<Self, String> {
    //                                ^^^^^^^^^^^^^ redundant, we need only `S`
        v.as_string_value()
            .map(str::to_owned)
            .or_else(|| v.as_int_value().as_ref().map(ToString::to_string))
            .map(Self)
            .ok_or_else(|| format!("Expected `String` or `Int`, found: {v}"))
    }
}

However, the only viable option to parse a GraphQLScalar is only from InputValue::Scalar variant. Machinery never passes to this function an InputValue::Enum or other variants. Thus, all the GraphQLScalar declarations use InputValue::as_string_value() or similar methods, which inside use the InputValue::as_scalar_value() method which tries to unwrap an InputValue::Scalar.

It's quite redundant to deal with the whole InputValue in from_input() functions, while the only viable and used variant is always InputValue::Scalar.

Solution

Instead, it makes sense to pass into the from_input() the already unwrapped InputValue::Scalar as ScalarValue, so the user code could deal with it directly without a need to involve any InputValue functionality.

#[derive(GraphQLScalar)]
#[graphql(parse_token(String, i32))]
pub struct ID(String);

impl ID {
    fn to_output<S: ScalarValue>(&self) -> Value<S> {
        Value::scalar(self.0.clone())
    }

    fn from_input(v: &impl ScalarValue) -> Result<Self, String> {
    //                     ^^^^^^^^^^^ use `ScalarValue` directly
        v.as_string()
            .or_else(|| s.as_int().as_ref().map(ToString::to_string))
            .map(Self)
            .ok_or_else(|| format!("Expected `String` or `Int`, found: {v}"))
    }
}

And yet, we can take a step further and provide an input/output type polymorphism for the from_input() function:

#[derive(GraphQLScalar)]
#[graphql(parse_token(String, i32))]
pub struct ID(String);

impl ID {
    fn to_output<S: ScalarValue>(&self) -> Value<S> {
        Value::scalar(self.0.clone())
    }

    fn from_input(v: &str) -> Self { // result could be infallible
    //               ^^^^ use `&str` directly
        Self(s.into())
    }
}

This is achieved by revamping the ScalarValue trait and introducing input type polymorphism via helper TryScalarValueTo conversion trait. We cannot use just TryFrom or TryInto trait for this purpose, because the conversions are made from a reference, and thus, we cannot specify the required conversions as super-trait bounds for the ScalarValue trait (which we need to make them implied in macro expansions or whenever ScalarValue is used).

The only downside, that Rust cannot infer types if the generic is specified directly, and thus, when we need to use a generic in from_input() function, we should use the transparent Scalar wrapper to aid the inference:

#[derive(GraphQLScalar)]
#[graphql(parse_token(String, i32))]
pub struct ID(String);

impl ID {
    fn to_output<S: ScalarValue>(&self) -> Value<S> {
        Value::scalar(self.0.clone())
    }

    fn from_input(v: &Scalar<impl ScalarValue>) -> Result<Self, String> {
    //                ^^^^^^ required to aid type inference
        v.try_to_string()
            .or_else(|| s.try_to_int().as_ref().map(ToString::to_string))
            .map(Self)
            .ok_or_else(|| format!("Expected `String` or `Int`, found: {v}"))
    }
}

Into-Result output type polymorphism is achieved by ::juniper::macros::helper::ToResultCall autoref-based specialization for the provided function.

Checklist

  • Documentation is updated (Book and API docs)
  • CHANGELOG entry is added
  • Tests are updated

@tyranron tyranron added this to the 0.17.0 milestone Jun 10, 2025
@tyranron tyranron self-assigned this Jun 10, 2025
@tyranron tyranron added enhancement Improvement of existing features or bugfix semver::breaking Breaking change in terms of SemVer k::api Related to API (application interface) k::refactor Refactoring, technical debt elimination and other improvements of existing code base labels Jun 10, 2025
@tyranron tyranron marked this pull request as ready for review June 13, 2025 17:57
@tyranron tyranron enabled auto-merge (squash) June 13, 2025 18:41
@tyranron tyranron merged commit c7b5fe9 into master Jun 13, 2025
180 checks passed
@tyranron tyranron deleted the simplify-graphql-scalar-declaration branch June 13, 2025 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::api Related to API (application interface) k::refactor Refactoring, technical debt elimination and other improvements of existing code base semver::breaking Breaking change in terms of SemVer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant