Return type not correctly set with type providers

[A5rocks]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.1.0

Plugin version

No response

Node.js version

16.14.0

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

11

Description

Return types aren't being checked correctly when inferred by a type provider.

Steps to Reproduce

import fastify, { FastifyTypeProvider } from "fastify"

interface ExampleTypeProvider extends FastifyTypeProvider {
    output: string  // everything is typed as a string
}

fastify().withTypeProvider<ExampleTypeProvider>().get("/", {
    schema: {
        response: "string"
    } as const
}, (_, res) => {
    res.send({ foo: 555 })  // this makes typescript complain
    
    return { foo: 555 }  // this does not
})

Expected Behavior

I expect return { foo: 555 } to error, because it's not return "some string" or the like.

[A5rocks]

cc @sinclairzx81 as you did #3524

It appears that if you inline ReturnType in either RouteHandlerMethod or RouteShorthandMethod, then it errors as expected. For example:

export type RouteHandlerMethod<
  RawServer extends RawServerBase = RawServerDefault,
  RawRequest extends RawRequestDefaultExpression<RawServer> = RawRequestDefaultExpression<RawServer>,
  RawReply extends RawReplyDefaultExpression<RawServer> = RawReplyDefaultExpression<RawServer>,
  RouteGeneric extends RouteGenericInterface = RouteGenericInterface,
  ContextConfig = ContextConfigDefault,
  SchemaCompiler extends FastifySchema = FastifySchema,
  TypeProvider extends FastifyTypeProvider = FastifyTypeProviderDefault,
  // ReturnType = ResolveFastifyReplyReturnType<TypeProvider, SchemaCompiler, RouteGeneric>,
  RequestType extends FastifyRequestType = ResolveFastifyRequestType<TypeProvider, SchemaCompiler, RouteGeneric>,
  Logger extends FastifyLoggerInstance = FastifyLoggerInstance
> = (
  this: FastifyInstance<RawServer, RawRequest, RawReply, Logger, TypeProvider>,
  request: FastifyRequest<RouteGeneric, RawServer, RawRequest, SchemaCompiler, TypeProvider, ContextConfig, RequestType, Logger>,
  reply: FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>
) => ResolveFastifyReplyReturnType<TypeProvider, SchemaCompiler, RouteGeneric>

I'm not at all sure why this would happen.

[RafaelGSS]

This is the same as fastify/fastify-type-provider-json-schema-to-ts#12, right?

[A5rocks]

I don't think so. The minimum reproduction I provided does not use json schema (nor the type provider in the issue), the original codebase I found this in uses a home-made zod type provider, the issue is only about return type (res.send works), and the fixes that I have found are type changes in fastify types.

[sinclairzx81]

@A5rocks Hi! Just had a quick look into this. As you mentioned, inlining the ReturnType generic parameter should resolve this issue, but you're right that the inference behavior here is quite puzzling. I note that if the ReturnType is used as an argument to the callback, then TypeScript will infer the appropriate return type in advance. So by doing this you can actually coax TS to infer for the correct return. Consider the following.

Test RouteHandlerMethod

export type RouteHandlerMethod<
  RawServer extends RawServerBase = RawServerDefault,
  RawRequest extends RawRequestDefaultExpression<RawServer> = RawRequestDefaultExpression<RawServer>,
  RawReply extends RawReplyDefaultExpression<RawServer> = RawReplyDefaultExpression<RawServer>,
  RouteGeneric extends RouteGenericInterface = RouteGenericInterface,
  ContextConfig = ContextConfigDefault,
  SchemaCompiler extends FastifySchema = FastifySchema,
  TypeProvider extends FastifyTypeProvider = FastifyTypeProviderDefault,
  ReturnType = ResolveFastifyReplyReturnType<TypeProvider, SchemaCompiler, RouteGeneric>,
  RequestType extends FastifyRequestType = ResolveFastifyRequestType<TypeProvider, SchemaCompiler, RouteGeneric>,
  Logger extends FastifyLoggerInstance = FastifyLoggerInstance
> = (
  this: FastifyInstance<RawServer, RawRequest, RawReply, Logger, TypeProvider>,
  request: FastifyRequest<RouteGeneric, RawServer, RawRequest, SchemaCompiler, TypeProvider, ContextConfig, RequestType, Logger>,
  reply: FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>,
  test: ReturnType  // 1) When the return type is used as an argument, TS must eagerly resolve the type.
) => ReturnType     // 2) Because the type is known on the argument. This now works

Example

interface TestProvider extends FastifyTypeProvider { output: string }

fastify().withTypeProvider<TestProvider>().get('/', {
  schema: {
    response: {
      200: ''
    }
  }
},  (req, res, test) => { // test is 'string | void | Promise<string | void>'

  return 1 // error: test type is known. so return type is also known.
})

Resolution

I think probably the easiest solution here is to go with inlining the return type (as per your example), but perhaps with some comments left around TS's inference behavior for the generic return type.

@A5rocks Would you be interested in submitting a PR for the inline?

@RafaelGSS I think this inference behavior might be a signal to consider a refactor on the ReturnType and ResponseType as both types are essentially the same. It may be possible to infer the correct ResponseType inside the RouteHandlerMethod and push that type down into the FastifyReply<...> (meaning the FastifyReply no longer needs to compute it's return type).

In this regard, because the reply is an argument of the callback, this should cause TS to eagerly resolve the correct return (as above), but would also be a bit more optimal in terms of an inference path as TS needs to only compute the Response/Return type once (currently it's resolving twice, once for the response type on .send() and again for the return).

It may be good to defer this refactor for the time being however, as I think it would share some cross over with "status code narrowing" that was briefly mentioned in the past. For the time being the solution to inline the return type (as per @A5rocks
example) seems the most straight forward under the current design.

Hope that helps!
S

[A5rocks]

This might be fixed in #4089.

@sinclairzx81 Could you check that out? (Along with #4076 which is already merged, but also changes type-provider linked things).

[sinclairzx81]

@A5rocks #4089 looks good to me :) As for #4076, this also looks good too. Note: The keyof was mostly left as a stub for status code narrowing in future (currently the types take a union of all possible status codes). But ultimately the plan there was to look at enabling the following in later revisions.

server.get('/', {
   schema: {
      response: {
          200: { type: 'string' },     // keyof: 200 | 500
          500: { type: 'number' }
      }
   }
}, (req, res) => {
  return res.send('hello')             // ok: string | number
  return res.send(10)                  // ok: string | number
  return res.status(200).send('hello') // ok: string
  return res.status(500).send('hello') // fail: expect number
                    ^ narrow here
})

There is a little bit of crossover between the status codes, return types and response types. But the #4089 should help carry things through until such time as the status code narrowing work gets some focus.

Good work!
S