Open gregmartyn opened 9 months ago
Hey @gregmartyn 👋
Interesting idea! To be honest, I think in general we need a more holistic approach to custom scalars anyways (something there is a lot of interest in: https://github.com/apollographql/apollo-feature-requests/issues/368). This might be a good test case for whatever solution we introduce for that feature to ensure we serialize variables before comparing them.
Thanks for bringing this case up! This is really good for us to be thinking about.
Apollo Client currently uses @wry/equality to determine whether
variables
has changed between invocations ofuseQuery
. See: source 1, source 2, source 3 and maybe more.That works fine for the specific classes that
equality
has special handling for (e.g. Date) and standard objects and primitives, but doesn't work for custom classes used by custom scalars. Unlike standard objects,equality
compares instances of custom classes by reference, not by value.equality
doesn't have a special-case for the Temporal classes used by graphql-temporal, souseQuery(query, { variables: { aDate: Temporal.Instant.from('2020-01-01T00:00+00') } })
results in an infinite loop of identical requests, all withaDate=2020-01-01T00:00:00Z
.I propose that variables should be compared by
JSON.stringify()
'ing them instead.!equal(newOptions.variables, oldVariables)
would becomeJSON.stringify(newOptions.variables) !== JSON.stringify(oldVariables)
. All variables have to be stringified to go over the wire anyway, so this should be compatible with all types. This should be backward compatible because any code relying onuseQuery
checking reference equality would have infinite-looped.