ts-rest icon indicating copy to clipboard operation
ts-rest copied to clipboard

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

Open rkreienbuehl opened this issue 1 year ago • 3 comments

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);
};

...

rkreienbuehl avatar Jul 22 '24 15:07 rkreienbuehl

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 avatar Jul 23 '24 23:07 Gabrola

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

Gabrola avatar Jul 23 '24 23:07 Gabrola

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.

rkreienbuehl avatar Jul 25 '24 06:07 rkreienbuehl