DateTime Scalar returns Date as opposed to String

[antho1jp]

As I understand DateTime was taken from graphql-iso-date. In that repo the return type from serialize is 'String' while here it is 'Date'. Is there anyway to regain that functionality? In the documentation on graphql-scalars website for DateTime it mentions date-time string. Is there a way that all DateTime types be sent to the client in String format without having to specify it on every resolver?

[ardatan]

You can see DateTime scalar's Result Coercion section for the behavior. Due to the JSON serialization, Date instances are converted to JSON string so the client should receive string not Date objects. If you see this does opposite, please share a failing test or a reproduction so we can help you better :)

[falkenhawk]

hi @ardatan , I've found this discussion only after I submitted #1641

I can see arguments in the discussion from your side and that This is actually an optimal way to avoid multiple serialization and de-serialization in case of combining multiple schemas.
but as I see it, this works well only for local GraphQL execution...

Why would astFromValue from graphql-js package complain and throw an error on such serialized values otherwise?

since servers are already serializing those values. <- doesn't that imply that there is actually additional serialization step happening now, because first all values are "serialized" to "Date" which then a server has to serialize that again to e.g. a string, to be able to carry the value over graphql protocol (and we cannot be sure HOW the server will serialize that, if even properly - that is out of our control)

And still any graphql client will have to parse the serialized value to restore the original value on their side - so we cannot avoid re-constructing on client side in any case, because instructions how to automatically deal with such values are not carried over the graphql protocol, other than in textual comments to scalar type definitions...

I was very confused why the DateTime serialization implementation was changed with v1.2.0 without any mention in the commit 3b1352c#diff-5bff20d592f8d56ae20cad088bf374d5ce38af414afd5631ab82f42481bb8473 or release notes (there are no release notes for v1.2.0 at all)
and that made me to submit the PR basically reverting that change (and additionally, fixing handling unix timestamps)

yet Date and Time scalars are still serialized to string values.
...but before the situation escalates into modifying Date and Time scalars to also "serialize" to Date instances... please consider reverting the serialization DateTime scalar to string :)

I, same as @antho1jp, am coming from graphql-iso-date, trying to migrate to this package, as it is recommended in their README.

[falkenhawk]

You can see DateTime scalar's Result Coercion section for the behavior.

if you related to what is written in docs for that, https://www.graphql-scalars.dev/docs/scalars/date-time
it is written there exactly what me and @antho1jp expected that

JavaScript Date instances and timestamps (represented as 32-bit signed integers) are coerced to RFC 3339 compliant date-time strings

(coerced to strings, not Date instances)

[ardatan]

Why would astFromValue from graphql-js package complains and throws an error on such serialized values otherwise?

I think I need an example to understand this :)

doesn't that imply that there is actually additional serialization step happening now, because first all values are "serialized" to "Date" which then a server has to serialize that again to e.g. a string, to be able to carry the value over graphql protocol (and we cannot be sure HOW the server will serialize that, if even properly - that is out of our control)

We parse incoming serialized result to Date once, and server is serializing again back once so there are no duplicate process here.

And still any graphql client will have to parse the serialized value to restore the original value on their side - so we cannot avoid re-constructing on client side in any case, because instructions how to automatically deal with such values are not carried over the graphql protocol, other than in textual comments to scalar type definitions...

It depends on which type of the client you are using. For example, I would expect Date instead of a string if I just execute a schema locally. If you use HTTP, it is still OK because most of the clients don't parse dates and gives you the string (serialized by your server). So for local execution, I can avoid "reconstruction" for local execution by keeping Date.

Related to the documentation, it seems outdated/wrong and needs to be fixed. But still it is not optimal to do "stringify" in our scalar implementation.

[falkenhawk]

Why would astFromValue from graphql-js package complain and throw an error on such serialized values otherwise?

I think I need an example to understand this :)

The most simple reproduction:

// package.json
{
  "dependencies": {
    "graphql": "16.5.0",
    "graphql-scalars": "1.17.0"
  }
}

// index.js
const astFromValue = require('graphql').astFromValue;
const DateTime = require('graphql-scalars').GraphQLDateTime;

const ast = astFromValue('2021-10-02T00:00:00.000Z', DateTime);

❯ node index.js
(...)\node_modules\graphql\utilities\astFromValue.js:171
    throw new TypeError(
    ^

TypeError: Cannot convert value to AST: 2021-10-02T00:00:00.000Z.
    at astFromValue ((...)\node_modules\graphql\utilities\astFromValue.js:171:11)
    at Object.<anonymous> ((...)\index.js:4:13)
    at Module._compile (node:internal/modules/cjs/loader:1095:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1124:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:816:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
    at node:internal/main/run_main_module:17:47

Graphql AST cannot be constructed when GraphQLScalarType.serialize() returns anything else than a "JSON-compatible format" https://www.apollographql.com/docs/apollo-server/schema/custom-scalars/#serialize
But what is a "JSON-compatible format" you ask. Well, according to what astFromValue can handle, reading from the code https://github.com/graphql/graphql-js/blob/main/src/utilities/astFromValue.ts it can be boolean, number, string, null, undefined, or object or Array containing only aforementioned types. Date is not among them, nor is any other function/class/etc.

If you think it makes more sense that serialize should be free to return anything, then imo graphql-js has to receive and accept a pull request, adjusting that on their side first. Otherwise we have a conflict and broken state.

IMO it is not a good idea to allow instances of classes or other functions being thrown at any graphql server implementation to let it transform those values to json themselves (I repeat: we will not have any control over that) unless you accept having non-deterministic silly 💩 like [object Object] etc. here and there in your graphql responses.

[falkenhawk]

That said, I won't use any more of my or your time trying to convince you here. Please feel free to do what you want, it is your lib after all. 👍

I am happy with my fork if I need it, but for now I reverted back to graphql-iso-date anyway.