Skip to content

Pass down context info to serialize method of leaf nodes #2268

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

Closed

Conversation

willhl
Copy link

@willhl willhl commented Nov 25, 2019

This came from a requirement to change the serialization behaviour based on what arguments were supplied to the field in the query. Specifically, a custom scalar was developed to convert values to the units requested in a "unit" field argument, e.g. request length values in meters instead of feet.

With this change this now possible by inspecting the fieldNodes and its arguments. Having the info passed down too is required to find the default value of the field arguments.

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Nov 25, 2019

@willhl Thanks for PR 👍

But scalar serialization behavior should be independent of field argument.
Scalars should be used both as input/output types and also inside SDL, e.g.:

scalar Distance

input Dimensions {
  x: Distance = 1 # We can't provide args inside SDL
  y: Distance = 1 # so we can't specify the unit of measurements
  z: Distance = 1 
}

A possible solution would be to create a wrapper function for your resolvers:

function changeScalarBasedOnArgs(resolver) {
   return (source, args, context, info) => {
     const result = resolver(source, args, context, info);
     return { value: result, unitOfMeasurement: args.unitOfMeasurement };
   }
}
new GraphQLScalarType({
  name: 'Distance',
  serialize: ({ value, unitOfMeasurement }) => {
    switch(unitOfMeasurement) {
      case 'feets':
        return value * 3.281;
      case 'meters':
      default:
        return value;
    }
  }
});

Does it solves your problem?
If not can you please provide more details about your use case?

@willhl
Copy link
Author

willhl commented Nov 26, 2019

Thanks for your response @IvanGoncharov, I suspected there would be an issue around SoC. I didn't come to this solution lightly and did have a think about other methods (not the one you've suggested though).

The problem with specifying "Distance" without args is it's not possible to infer from introspection what units it is returning by default. In your example it's meters, but there's no way to know this without looking into the code. I guess you could provide this info in separate documentation but it would be nice for introspection to provider everything you need to know.

So the use cases I was trying to solve:

  • Allow the units to be inferred via introspection, for both queries and mutations
  • Allow queries to ask for values to be returned in other applicable units. I know this is an application level problem to solve, but in reality this is going to be a very common thing to need to in my case, so why not have one implementation to do it for you? Sometimes breaking the rules is ok?

I'm also using neo4j-graphql-js where mutations are generated automatically from the types, so this makes it really convenient to infer the units expected where these fields appear in mutations.

Your solution looks promising though, I'll have another think about the idea based on your suggestion and see where it takes me.

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Nov 26, 2019

The problem with specifying "Distance" without args is it's not possible to infer from introspection what units it is returning by default.

@willhl Not sure I understand this point, so can you please provide an example of how this PR helps you to achieve this?

In your example it's meters, but there's no way to know this without looking into the code. I guess you could provide this info in separate documentation but it would be nice for introspection to provider everything you need to know.

You can specify default values for arguments:

type Dimensions {
  height(unitOfMeasurement: UnitOfMeasurement = METERS)
  length(unitOfMeasurement: UnitOfMeasurement = METERS)
} 

Also, do you need to switch units for every scalar field separately or once per query?

query MyQuery(
  $measurementSystem: MeasurmentSystemName = METRIC
) @measurementSystem(name: $measurementSystem) {
  # ...
}

@IvanGoncharov
Copy link
Member

@willhl Closing this PR.
Please feel free to open a new issue and describe your use case in more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants