mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.33k stars 234 forks source link

Query caching broken with some custom scalars and JIT #997

Open lsanwick opened 1 year ago

lsanwick commented 1 year ago

I'm building a schema that has a number of custom scalars like DateTime that are automatically validated and parsed into a Luxon DateTime object when provided as arguments.

This works great, except it seems to interact poorly with the default request cache, where subsequent requests will just provide the string representation of the argument.

Here's a toy implementation that shows the issue:

scalar DateTime

type Query {
  testDateTime(time: DateTime!): String!
}
const parseDateTime = (astOrString) => DateTime.fromISO(dateTime, { zone: "utc" })

const resolvers = {
  DateTime: new GraphQLScalarType<LuxonDateTime, string>({
    name: "DateTime",
    description:
      "An ISO8601 formatted date time string (e.g. 2023-02-01T08:30:00Z), parsed into a Luxon DateTime",
    serialize(value) {
      if (value instanceof LuxonDateTime) {
        return value.toISO();
      }
      throw new Error("DateTime must be a Luxon DateTime");
    },
    parseValue(dateTime) {
      return DateTime.fromISO(dateTime, { zone: "utc" })
    },
    parseLiteral(ast) {
      if (ast.kind === Kind.STRING) {
        return DateTime.fromISO(ast.value, { zone: "utc" })  
      }
    },
    Query: {
      dateTimeMessage(_root, { time }) {
        return `Good day! It's ${time.toISODate()}`
      }
    }
})

Example query

query {
  testDateTime(date: "2023-05-16T21:30:00.000Z")
}

First response (correct)

{
  "data": {
    "testDateTime": "Good day! It is 2023-05-16"
  }
}

Second response (incorrect)

{
  "data": null,
  "errors": [
    {
      "message": "time.toISODate is not a function",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "testDateTime"
      ]
    }
  ]
}

Disabling the caching with cache: false on mercurius when registering the plugin fixes the issue.

mcollina commented 1 year ago

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

lsanwick commented 1 year ago

Okay, I've created a repoduction repo here: https://github.com/lsanwick/custom-scalar-caching-example

It appears that the issue appears with this configuration:

  await fastify.register(mercurius, {
    schema,
    resolvers,
    jit: true,
    graphiql: true,
  });

If you disable JIT, or disable caching while keeping JIT enabled, I don't see the issue.

  jit: true,
  cache: false,
lsanwick commented 1 year ago

@mcollina Is that sufficient to work off of, or do you need anything else?

mcollina commented 1 year ago

I currently do not have much time to look into it. This looks like an issue inside graphql-jit.

I never experience this exact issue so maybe there is something that intersects between that and luxon.