Skip to content
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

getOptimisticResponse for custom scalars not working as expected #368

Closed
juhaelee opened this issue Sep 22, 2015 · 10 comments
Closed

getOptimisticResponse for custom scalars not working as expected #368

juhaelee opened this issue Sep 22, 2015 · 10 comments
Labels

Comments

@juhaelee
Copy link

I have a custom scalar that is called coordinate that takes a shape like this {x: INT, y: INT}

When I try to do use getOptimisticResponse on an "RANGE_ADD" I have something like this:

return {
      gridEdge: {
        node: {
          coordinate: this.props.point
        }
      }
}

but when my container receives this update, coordinate exists, but appears as an empty object without the coordinate attributes. ( coordinate: {dataID: "client:SOMENUMBER"} ). this.props.point has the correct value before it is returned in getOptimisticResponse as well.

I've also tried something like this, to no success:

return {
      gridEdge: {
        node: {
          coordinate: {
               x: this.props.point.x,
               y: this.props.point.y
          }
        }
      }
}

Wondering if Relay can't support custom scalars in optimistic updating yet?

@juhaelee
Copy link
Author

It looks like that objects can't be passed through a field in getOptimisticResponse? I'm getting around this situation currently, by using JSON.stringify(coordinate) and passing that though

@josephsavona
Copy link
Contributor

@juhaelee What is the fat query for this mutation?

@juhaelee
Copy link
Author

@josephsavona

getFatQuery(){
    return Relay.QL`
      fragment on CreatePointPayload {
        gridEdge,
      }
    `;
  }

Here is also my custom scalar for reference: (I'm using Joi for validation)

import { GraphQLScalarType } from "graphql";
import { Kind } from "graphql/language";

import Joi from "joi";

const coerceCoordinate = (val) => {
  var schema = {
    x: Joi.number(),
    y: Joi.number()
  };
  const {error, value} = Joi.validate(val, schema);
  if(error){ return null; }
  return value;
};

export default new GraphQLScalarType({
  name: "Coordinate",
  serialize: coerceCoordinate,
  parseValue: coerceCoordinate,
  parseLiteral(ast){
    return ast.kind === Kind.OBJECT ? ast.value : null;
  }
});

@josephsavona
Copy link
Contributor

Custom scalar types are a new feature in OSS GraphQL, but it would seem that the value for such a field would have to actually be a scalar, ie, it should serialize to a string, number, or boolean. I think that JSON.stringify is the right solution here.

@juhaelee
Copy link
Author

Just to clarify, JSON.stringify in getOptimisticResponse and not the custom scalar implementation correct?

@josephsavona
Copy link
Contributor

I'm double-checking my assumption with @dschafer - but yeah, I was thinking in getOptimisticPayload

@josephsavona
Copy link
Contributor

I looked into this and the problem is in inferRelayFieldsFromData, where we attempt to infer the query that would correspond to an optimistic payload: objects are assumed to correspond to records, so in this case Relay traverses coordinate as if it were a non-scalar and writes the optimistic values for x/y as if they were subfields.

Using JSON.stringify in getOptimisticResponse is a temporary workaround, but this means that view code will have to handle both the JSON-stringified value and object value for coordinate. Long-term we should evaluate using tracked queries to construct a more correct query to match the optimistic response payload.

cc @yuzhi @yungsters

@juhaelee
Copy link
Author

Cool, thanks for all your help!

@yuzhi
Copy link
Contributor

yuzhi commented Sep 28, 2015

inferRelayFieldsFromData does an best effort for inferring queries for optimistic. We didn't intend it to work for every case. It's mainly for basic queries. It doesn't work for fields with calls or custom scalar types. It also doesn't have access to the schema.
We used to have a way for people to supply custom queries for optimistic payloads for these cases. We should consider bringing that back.

@josephsavona
Copy link
Contributor

This looks related to #91 - Relay only supports custom "scalar" types if those types are JS scalar values - boolean, number, or strings. Let's merge the discussion into #91.

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

No branches or pull requests

3 participants