github-community-projects / graphql-client

A Ruby library for declaring, composing and executing GraphQL queries
MIT License
42 stars 218 forks source link

Automatic parsing of `ISO8601Date` and other custom types #17

Open patch0 opened 8 months ago

patch0 commented 8 months ago

We use graphql-ruby and graphql-client to talk to each other. The graphql-ruby library defines a few extra types beyond the standard GraphQL types, e.g. ISO8601Date, ISO8601DateTime, etc, which we make use of.

In the client we've found that the values of these fields are not getting automatically cast back to Date / Time objects, like they do for other types of object.

I was wondering what might be the best way to get automatic casting for these types. At the moment the only way I can see it might work is by monkey-patching the GraphQL::Schema::BUILT_IN_TYPES hash, which is a bit 🤢 .

module GraphQL
  class Schema
    BUILT_IN_TYPES['ISO8601Date'] = GraphQL::Types::ISO8601Date
    BUILT_IN_TYPES['ISO8601DateTime'] = GraphQL::Types::ISO8601DateTime
  end
end

Is there a better way of managing this? I can imagine that other GraphQL schemas might have other sorts of custom types, but then how should they be re-interpreted by the client?

Incidentally the tests don't tickle this situation, as the schema is defined using the DSL. To get the datetime/date parsing tests to fail, change test/test_query_result.rb to dump/reload the schema:

ReloadedSchema = GraphQL::Client.load_schema(Schema.execute(GraphQL::Introspection.query(include_deprecated_args: true, include_schema_description: true, include_specified_by_url: true, include_is_repeatable: true)))

def setup
  @client = GraphQL::Client.new(schema: ReloadedSchema, execute: Schema, enforce_collocated_callers: true)
end

Then the test fails:

bundle exec rake test TEST=test/test_query_result.rb
Run options: --seed 9819

# Running:

F.........F.......F...................

Finished in 0.077330s, 491.4005 runs/s, 1551.7910 assertions/s.

  1) Failure:
TestQueryResult#test_interface_within_union_values [test/test_query_result.rb:592]:
Expected: 1970-01-01 00:00:01 UTC
  Actual: "1970-01-01T01:00:01+01:00"

  2) Failure:
TestQueryResult#test_date_scalar_casting [test/test_query_result.rb:628]:
--- expected
+++ actual
@@ -1 +1,3 @@
-#<Date: 1970-01-01 ((2440588j,0s,0n),+0s,2299161j)>
+# encoding: US-ASCII
+#    valid: true
+"1970-01-01"

  3) Failure:
TestQueryResult#test_datetime_scalar_casting [test/test_query_result.rb:610]:
Expected: 1970-01-01 01:00:00 +0100
  Actual: "1970-01-01T01:00:00+01:00"

38 runs, 120 assertions, 3 failures, 0 errors, 0 skips
rake aborted!

Adding the "monkey patch" above then re-fixes the tests as the casting happens automatically.

This all feels a bit gross! Is there a better way?

Thanks!

rmosolgo commented 8 months ago

Hey, thanks for the suggestion -- I agree that this would be a good default, but we should take care to to support backwards compatibility for people who have already found a solution. I'm not sure the best way to add this behavior but I'll take a look soon and follow up here.

Compatibility-wise, I bet we could add support for these on an opt-in basis in 0.x and then make them default in 1.0 (#15).

patch0 commented 8 months ago

I think there's a more general issue here too, about how to handle custom GraphQL types in general. This probably comes in under the main graphql-ruby repo, too. It has docs around custom scalers, and I guess would be really useful to have the same sort of casting functionality in the client too.

Not sure how to implement it though, as the graphql-ruby library does the heavy lifting here and that's where the types are mapped to classes (and also here). Seems a bit tricky to fixinate.