ts-rest / ts-rest

RPC-like client, contract, and server implementation for a pure REST API
https://ts-rest.com
MIT License
2.36k stars 112 forks source link

@ts-rest/fastify move validation away from handler into the validation and serealization of fastify #636

Open rkreienbuehl opened 2 months ago

rkreienbuehl commented 2 months ago

Is your feature request related to a problem? Please describe. Validation with @ts-rest/fastify is done within the handler. This makes the whole fastify hook system more or less useless.

Describe the solution you'd like It would be better and cleaner if fastify would add a type-provider for fastify and register a validationCompiler with app.setValidatorCompiler(validatorCompiler) and a serializerCompiler with app.setSerializerCompiler(serializerCompiler).

Describe alternatives you've considered Instead of writing an own type-provider, maybe fastify-type-provider-zod could be used.

Additional context In a first test I used fastify-type-provider-zod to move the validation into validate and serealize into the fastify mechanism. This could be improved to fully support ts-rest by writing an own type-provider similar to fastify-type-provider-zod.

The changed parts of my code (@ts-rest/fastify v3.46.0) look like this:

...

export const initServer = () => (
  ...
  ) => {
    // setup type-provider
    app.setValidatorCompiler(validatorCompiler);
    app.setSerializerCompiler(serializerCompiler);

    app.withTypeProvider<ZodTypeProvider>();

    ...
  },
  plugin: <T extends AppRouter>(
    router: RecursiveRouterObj<T>
  ): FastifyPluginCallback<RegisterRouterOptions<T>> => (
    app,
    opts = {
      logInitialization: true,
      jsonQuery: false,
      responseValidation: false,
      requestValidationErrorHandler: 'combined',
    },
    done
  ) => {
    // setup type-provider
    app.setValidatorCompiler(validatorCompiler);
    app.setSerializerCompiler(serializerCompiler);

    app.withTypeProvider<ZodTypeProvider>();

    ...
  },
});

...

const registerRoute = <TAppRoute extends AppRoute>(
  routeImpl: AppRouteImplementationOrOptions<AppRoute>,
  appRoute: TAppRoute,
  app: FastifyInstance,
  options: BaseRegisterRouterOptions
) => {

  ...

  // construct schema
  const schema: Record<string, unknown> = {};
  if (Object.prototype.hasOwnProperty.call(appRoute, 'body')) {
    // @ts-expect-error
    schema.body = appRoute.body;
  }
  if (Object.prototype.hasOwnProperty.call(appRoute, 'query')) {
    schema.querystring = appRoute.query;
  }
  if (Object.prototype.hasOwnProperty.call(appRoute, 'pathParams')) {
    schema.params = appRoute.pathParams;
  }
  if (appRoute.headers && Object.keys(appRoute.headers).length > 0) {
    schema.headers = appRoute.headers;
  }

  if (options.responseValidation) {
    const responses: Record<string | number, unknown> = {};
    for (const [key, value] of Object.entries(appRoute.responses)) {
      const v = isAppRouteNoBody(value)
        ? z.undefined()
        : isAppRouteOtherResponse(value)
        ? value.body
        : isZodType(value)
        ? value
        : undefined;

      if (v) {
        responses[key] = v;
      }
    }
    schema.response = responses;
  }

  const route: RouteOptions<
    RawServerDefault,
    RawRequestDefaultExpression,
    RawReplyDefaultExpression,
    RouteGenericInterface,
    FastifyContextConfig<AppRoute>
  > = {
    ...hooks,
    schema, // register schema for validation
    method: appRoute.method,
    url: appRoute.path,
    config: {
      tsRestRoute: appRoute,
    },
    handler: async (request, reply) => {
      let result: { status: HTTPStatusCode; body: unknown };
      try {
        result = await handler({
          // eslint-disable-next-line @typescript-eslint/no-explicit-any
          params: request.params as any,
          query: request.query,
          // eslint-disable-next-line @typescript-eslint/no-explicit-any
          headers: request.headers as any,
          request,
          // eslint-disable-next-line @typescript-eslint/no-explicit-any
          body: request.body as any,
          reply,
          appRoute,
        });
      } catch (e) {
        if (e instanceof TsRestResponseError) {
          result = {
            status: e.statusCode,
            body: e.body,
          };
        } else {
          throw e;
        }
      }

      const statusCode = result.status;

      const responseType = appRoute.responses[statusCode];

      if (isAppRouteNoBody(responseType)) {
        return reply.status(statusCode).send();
      }

      if (isAppRouteOtherResponse(responseType)) {
        reply.header('content-type', responseType.contentType);
      }

      return reply.status(statusCode).send(result.body);
    },
  };

  app.route(route);
};

...
Gabrola commented 2 months ago

Good suggestion. Although, we are trying to consolidate as much code as we can in order to share them between the different server libraries, I think this makes sense given that fastify allows you to hook pre-validation, pre-parsing, etc. Only thing I think that wouldn't make much sense is the type provider since the handlers consume typed request parameters separately, so no need to have them typed again inside request.*. We are also forced to manually define some many of the generics that define the types in order to add the tsRestRoute context

type FastifyContextConfig<T extends AppRouter | AppRoute> = {
  tsRestRoute: T extends AppRoute ? T : FlattenAppRouter<T>;
};

type AppRouteImplementation<T extends AppRoute> = (
  input: ServerInferRequest<T, fastify.FastifyRequest['headers']> & {
    request: fastify.FastifyRequest<
      fastify.RouteGenericInterface,
      fastify.RawServerDefault,
      fastify.RawRequestDefaultExpression,
      fastify.FastifySchema,
      fastify.FastifyTypeProviderDefault,
      FastifyContextConfig<T>
    >;
    reply: fastify.FastifyReply<
      fastify.RawServerDefault,
      fastify.RawRequestDefaultExpression,
      fastify.RawReplyDefaultExpression,
      fastify.RouteGenericInterface,
      FastifyContextConfig<T>
    >;
    appRoute: T;
  },
) => Promise<ServerInferResponses<T>>;

So actually it seems like we'd have to make sure we pass our own correct types to replace fastify.FastifySchema and fastify.FastifyTypeProviderDefault, rather than calling withTypeProvider, since this only makes sense in the context of when you are manually calling server.get(..., { schema{ ... } }, (request, reply) => ...);, so it can type request correctly for you. But since ts-rest is doing that, it would be useless.

Seems like you've played around with this for a bit, feel free to submit a PR if you'd like because I doubt we'd get around to this soon tbh

Gabrola commented 2 months ago

Another idea to note is that I've been floating around the idea to provide such hooks from the ts-rest side in the serverless library, and this could serve as a base to provide this as common functionality for all the other server libraries including fastify. In this case, yes, the fastify hooks for these would be useless but you'd have an alternative. Or even we can just opaquely utilize these fastify hooks under the hood but the code would look exactly the same as if it were run on express or any other framework

rkreienbuehl commented 2 months ago

Thanks for your response. I will do some more tests and make a pull-request as soon as I find the time. Personally I would use fastify-hooks under the hood, if hooks are added to ts-rest. This wouldn't "break" the fastify lifecycle.