relay icon indicating copy to clipboard operation
relay copied to clipboard

Hydrate scalar fields on client

Open KyleAMathews opened this issue 10 years ago • 30 comments

E.g. a DateTime field could be automatically turned into a Moment.js object.

Is this possible already and I just missed it in the docs?

KyleAMathews avatar Aug 17 '15 17:08 KyleAMathews

This isn't currently supported, and you'd have to manually convert complex types e.g. in your components given the plain data from Relay. This is definitely a use-case worth considering.

I'll leave this open to gather feedback about the idea and examples of data types people may need.

josephsavona avatar Aug 17 '15 19:08 josephsavona

The most common use case that comes to mind is indeed DateTime (that was the first custom scalar type we added in our own GraphQL server). Another example that would be useful in apps that deal with location data could be a geo point, for example hydrated to google.maps.LatLng.

Because these scalar types and their client-side representation are application specific, if this is going to be added to Relay it should probably be fully extendable, e.g. you could define a mapping from your GraphQL types to your JavaScript object builders. Transit-js might be a good place to look for an example on how to provide extension points for this.

I think this is could be an interesting idea. While it can also be implemented separately in the transport layer, e.g. with Transit, the GraphQL schema already has the necessary type information for hydrating types, meaning no extra metadata about the object type would need to be passed in the responses. This might save lots of bytes in apps that handle large amounts of data.

On the other hand this could add complexity to the framework, so maybe it would be worth it to consider if this could be done as a separate library outside Relay? A custom RelayNetworkLayer gets access to the runtime representation of each query in sendQueries and sendMutation. Maybe if the Babel plugin would store enough information about the types in the tree, the hydration of responses and serialization of requests could happen in the network layer using that information about the query?

fson avatar Aug 17 '15 20:08 fson

Yeah, Transit-js was what I was thinking of when I wrote this issue.

KyleAMathews avatar Aug 17 '15 21:08 KyleAMathews

This would be useful for having fields that represent plain JSON objects as well.

taion avatar Aug 18 '15 19:08 taion

The transport layer is probably the wrong place to introduce this as it couples too much of the system together; in particular, the transpiler (to annotate types), the query representation (to expose this metadata), and any network implementation. It also increases the responsibility of a network layer, putting a burden on all implementors.

The best way to approach this is to change babel-relay-plugin to annotate the types of leaf fields, and add support in the payload processing phase (RelayQueryWriter) for deserializing "complex" values. For each leaf field, check if it has a non-scalar (Int, String, Boolean) type and if so, determine if there is an injected handler for that type. Pseudo code:

// In `RelayQueryWriter`
visitField(field, value) {
  var type = field.getType();
  if (type && !isScalarType(type)) {
    var handler = Relay.getInjectedTypeHandler(type);
    value = handler.deserialize(value);
  }
}

// define handler with:
Relay.injectTypeHandler("DateTime", {
  deserialize: (value) => ...,
  serialize: (value) => ...,
});

cc @yungsters

josephsavona avatar Aug 18 '15 19:08 josephsavona

Edit: summarizing on offline discussion with @yungsters - given the complexity of the "ideal" solution I described above, we're inclined to not add support for deserializing complex values right now. Doing so could conflict with other planned improvements such as querying local/device data and representing the cache as an immutable object.

We'd be curious to see how you approach this in a custom network layer and see about integrating that work down the road.

josephsavona avatar Aug 18 '15 19:08 josephsavona

For me not super high as a) it'll be a month or so before I start implementing relay and b) it's pretty easy to just hydrate in views for now. On Tue, Aug 18, 2015 at 12:56 PM Joseph Savona [email protected] wrote:

How would you rate the priority of this?

— Reply to this email directly or view it on GitHub https://github.com/facebook/relay/issues/91#issuecomment-132332431.

KyleAMathews avatar Aug 18 '15 20:08 KyleAMathews

It's of very low importance to me as well - it's easy enough to handle this in my own code for now.

taion avatar Aug 18 '15 20:08 taion

Thanks for the feedback. I'll close this for now but feel free to reopen later!

josephsavona avatar Aug 18 '15 20:08 josephsavona

On using this pattern a bit, I find that there's a bit of a complication in that naively dehydrating data from Relay on-the-fly causes issues with breaking reference equality, which breaks using shallow equality for pure components.

I've taken to doing something like https://gist.github.com/taion/df126b9252825927b037 to avoid e.g. breaking referential equality on blob1 when e.g. blob2 changes because arg changes.

taion avatar Aug 29 '15 21:08 taion

Someone mentioned seeing related performance issues with repeatedly instantiating Moment.js objects on the component.

Would it be possible to re-open this issue? I understand that this is unlikely to be a priority, especially given adequate user-side workarounds, but this is something that needs to be handled... somewhere.

taion avatar Oct 12 '15 06:10 taion

Here's an additional problem.

Suppose I have a field that deserializes into a non-scalar type. If I use this field in multiple nested components, I have a choice of either deserializing this in each component, or doing this once at top-level and passing through the deserialized value.

Neither is ideal. If I deserialize at top-level, the non-scalar prop defeats the Relay container shouldComponentUpdate check, and I get excessive re-renders. If I deserialize in each component, then I'm doing work multiple times for no good reason.

taion avatar Dec 02 '15 21:12 taion

I've talked this over with some people and have written a draft specification. We're not pursuing this internally, but if folks are motivated to get this working, we'd be happy to take a look at something similar to this proposed spec:

A method for serializing/deserializing scalar fields on the client

Given a scalar value sent from the server, like ‘1980-03-27’, you might want to interact with it as a JavaScript Date object on the client (eg. by creating a Date instance with new Date('1980-03-27')). What follows is a proposal to implement custom, runtime serializers/deserializers on the client.

Custom scalar types as deserializable

The idea is to consider all fields of type GraphQLScalarType as in need of a serializer/deserializer to be used with Relay. Given a field with this .graphql definition:

scalar ISODate

…which was, for instance, generated with this graphql-js schema:

const ISODate = new GraphQLScalarType({
  name: 'ISODate',
  serialize: date => date.toISOString().toJSON(),
  parseValue: isoString => new Date(isoString),
  parseLiteral: ast => new Date(ast.value),
});

…Relay should look up a custom serializer/deserializer at runtime to use to deal with values of this type.

Specification

  1. Map typenames to their serializer/deserializer using the following API:

    relayEnvironmentInstance.registerTypeHandler('ISODate', {deserialize, serialize});
    
  2. Check to see if you have a registered deserializer when you go to read a scalar value (see readRelayQueryData#_readScalar())

    • if no deserializer is registered, return the raw value
    • if a deserializer is registered use it to produce the value
  3. Memoize the deserialized values and clean up the memo when those fields are no longer used (see, for instance _updateGarbageCollectorSubscriptionCount in GraphQLStoreQueryResolver)

  4. When printing mutation queries, serialize the values for transport over the wire. See printArgument of printRelayOSSQuery.

steveluscher avatar Jun 27 '16 23:06 steveluscher

Going to close this as Relay 2 will have support for arbitrary "handle" fields, for which you can register a handler in a way similar to what @steveluscher has suggested. As we're unlikely to undertake the work required to implement something like that on Relay 1, I'm going to mark this one as closed for now.

Thanks to everybody for their input!

wincent avatar Sep 03 '16 00:09 wincent

So I tried to use this to implement a JSON scalar. I managed to hook up a handler like this:

fragment {
   config @__clientField(handle: "json"),
  ...
}

And pass handlerProvider to the Relay Environment, which looks like this:

import {ConnectionHandler} from 'relay-runtime';

function updateJSON(proxy, payload) {
  const record = proxy.get(payload.dataID);
  if (!record) {
    return;
  }
  const parsed = JSON.parse(record.getValue(payload.fieldKey));

  // Whatever the value you set for payload.handleKey is what the React component will see.
  record.setValue(parsed, payload.handleKey);
}

function handlerProvider(handle) {
  if (handle === 'json')
    return {update: updateJSON};

  if (handle === 'connection')
    return ConnectionHandler;
}

But this doesn't work, because Record.setValue() doesn't allow objects:

 RelayRecordProxy#setValue(): Expected a scalar value, got ...

This means we can also not parse Dates.

miracle2k avatar May 25 '17 14:05 miracle2k

Should this issue be reopened? As @miracle2k mentioned, it seems it's not currently possible to handle the cases discussed in this thread (e.g. Date, JSON) via handlerProvider because of the scalar values restriction.

apalm avatar Jun 06 '17 16:06 apalm

JSON "just works" even with Relay Modern w/graphql-type-json, BTW.

taion avatar Jun 06 '17 17:06 taion

@taion I use the same setup as graphql-type-json, and it largely seemed to work at first, but if you try to update such a JSON field that is already in the store you'll get errors (for example, through a second query, or by reading a mutation payload). There is code in Relay Modern that tries to, I think, merge the new objects into the old ones to keep as much as possible their identify if they didn't change, and it recurses into the JSON structures.

I don't have the exact file/line saved unfortunately.

miracle2k avatar Jun 06 '17 18:06 miracle2k

I'll reopen this as it might be interesting to explore how we could for example hydrate a custom URL scalar into an URL instance or a timestamp into a Date on the client.

I'm not sure about all the implications of this on Mutations (we should still validate that people don't set a deep structure on what should be a linked record) or other places this would have.

kassens avatar Jun 11 '17 20:06 kassens

Thanks for the input here. We're currently going through old issues that appear to have gone stale (ie. not updated in about the last 3 months) because the volume of material in the issue tracker is becoming hard to manage. If this is still important to you please comment and we'll re-open this.

Thanks once again!

wincent avatar Sep 11 '17 17:09 wincent

@wincent can you reopen ticket? In our project we needed scalar type hydration. And as we understand the only solution is to allow to save in store JS objects, not only JSON types (strings, numbers, arrays, objects). Thanks!

mxck avatar Oct 23 '17 03:10 mxck

@wincent I notice this issue is still closed. Does that mean there are no plans to work on this? Custom scalars are a pretty awesome feature of GraphQL and are currently super type-unsafe in Relay since they can't be typechecked by Flow due to #2162.

benkuhn avatar Feb 17 '18 18:02 benkuhn

@wincent it would be great to have an update on this topic when you get the chance. Thanks.

tlvenn avatar Aug 15 '19 06:08 tlvenn

I'm not working on Relay anymore.

wincent avatar Aug 15 '19 07:08 wincent

So is this issue dead? This is still pretty much the only discussion I can find on this topic through Google. I have a bunch of "date" leafs in my GraphQL schema and it's a real pain to repeatedly have to convert them to Date objects in the views they are needed.

Hubro avatar May 24 '20 13:05 Hubro

This feature request is tricky; it's come up often enough over the years that it is clearly useful to some folks, but it doesn't appear to be a major point for too many people. At present we aren't planning to work on this, but I agree that this is a conceptually missing feature and it would be great if we could find a clean way to address it.

In terms of implementation, my instinct is that it makes sense for the store to contain the serialized form of the value (e.g. a DateTime as a string), and then deserialize into a custom object at the point that we read data out of the store to vend to components. The catch is that any type used in this way would have to implement some form of equality check, so that we could determine whether the value has changed. Another aspect is how users would enable this special behavior: there it makes sense to either have schema directives or compiler config that allows one to map specific scalar types to a custom resolver module, such that Relay Compiler could automatically detect instances of e.g. DateTime fields and enable deserialization automatically in all fragments that select DateTime fields.

The above sketch is not intended to act as a blueprint for a PR immediately. It's just a sketch of some thoughts right now. We have a few things in the works that touch on the relevant code here so if you're interested in contributing from the community, reach out here or (preferably) on Slack first.

josephsavona avatar May 11 '21 13:05 josephsavona

asdasda

anhthiencao avatar Sep 24 '21 14:09 anhthiencao

test va

var-outlook-ie avatar Jan 03 '22 13:01 var-outlook-ie

awd

Charlotte-z avatar Jan 20 '22 09:01 Charlotte-z

With the help from some comments on this and other issues I came up with this solution:

https://www.npmjs.com/package/relay-transform-scalar

I couldn't yet figure out how to make the compiler insert the __clienftField directive for every field of a type. It seems compiler plugins are not supported anymore. I found a promising configuration option around "custom transforms", however it seems it is only accessible from rust and I would need to compile a custom compiler? If anyone has an idea, please reach out.

phikes avatar Jul 24 '24 08:07 phikes