nestjs / graphql

GraphQL (TypeScript) module for Nest framework (node.js) 🍷
https://docs.nestjs.com/graphql/quick-start
MIT License
1.46k stars 397 forks source link

CustomScalar parseLiteral doesn't allow null return type #3176

Open silto opened 8 months ago

silto commented 8 months ago

Is there an existing issue for this?

Current behavior

The issue is basically the same as this one: https://github.com/nestjs/graphql/issues/222 That was previously fixed in this PR: https://github.com/nestjs/graphql/pull/749

The sample code in the docs for the parseLiteral method of custom scalar is this:

parseLiteral(ast: ValueNode): Date {
    if (ast.kind === Kind.INT) {
      return new Date(ast.value);
    }
    return null;
  }

But with TS strictNullChecks enabled, it results in an error.

The fix in the PR was to use some graphql types that used "Maybe" synthax to allow for the null return type, but it looks like the types of the graphql module have changed since then and don't include "Maybe" in the return type anymore, resulting in the same type issue reappearing.

Minimum reproduction code

https://github.com/nestjs/nest/blob/master/sample/12-graphql-schema-first/src/common/scalars/date.scalar.ts

Steps to reproduce

No response

Expected behavior

CustomScalar should allow parseLiteral to return null or undefined as demonstrated by the samples.

Package version

12.0.4

Graphql version

graphql: "^16.7.1", @apollo/server: "^4.7.5",

NestJS version

10.0.3

Node.js version

18.17.1

In which operating systems have you tested?

Other

No response

kamilmysliwiec commented 8 months ago

Would you like to create a PR for this issue?

silto commented 8 months ago

I can but I would need to drop the graphql types (GraphQLScalarLiteralParser, GraphQLScalarValueParser,...) and use K | null | undefined. Is that ok?