fastify / fastify-express

Express compatibility layer for Fastify
MIT License
247 stars 25 forks source link

Why the `expressHook` should be `preHandler` and not `preValidation`? #96

Closed rluvaton closed 2 years ago

rluvaton commented 2 years ago

Prerequisites

Issue

In the docs there is this example in the Troubleshooting - POST request with body hangs up:

const Fastify = require('fastify')
const Express = require('express')
const expressPlugin = require('@fastify/express')
const fastifyFormBody = require('@fastify/formbody')

const fastify = Fastify()
const express = Express()

await fastify.register(fastifyFormBody)
await fastify.register(expressPlugin, {
  // run express after `@fastify/formbody` logic
  expressHook: 'preHandler'
})

fastify.use(express)

// it works!
fastify.post('/hello', (req, reply) => {
  return { hello: 'world' }
})

Why the expressHook should be preHandler and not preValidation?

if it's preHandler, middlewares that want to always run won't (for example - logger [we are planning to migrate to pino :)])

and preValidation is the first hook that have the body parsed, no?

mcollina commented 2 years ago

Why would you not execute the validations?

rluvaton commented 2 years ago

I want to execute the validators I just have logger that log every request and if request fails on the validation than it won't log

mcollina commented 2 years ago

I don't understand the question.

rluvaton commented 2 years ago

In case I have the code below - use express and validator:

and I send this request (name is missing in body):

curl -v -d '{}' -H 'Content-Type:\ application/json' http://localhost:5555/

The logger won't log, but if I change to preValidation the log will happen, so why you suggested to use preHandler and not preValidation is there something I'm missing here

const Fastify = require('fastify');
const expressPlugin = require('@fastify/express');
const fastifyFormBody = require('@fastify/formbody');

(async () => {
    const fastify = Fastify({});

    await fastify.register(fastifyFormBody);
    await fastify.register(expressPlugin, {
        expressHook: 'preHandler',
    });

    // This is actually express-winston middleware but just for the sake of example
    fastify.use((req, res, next) => {
        const resEnd = res.end;
        res.end = function (...args) {
            console.log(`[${res.statusCode}] ${req.method} ${req.url}`, req.body);
            return resEnd.apply(res, args);
        };
        next();
    });

    fastify.post(
        '/',
        {
            schema: {
                body: {
                    type: 'object',
                    properties: {
                        name: {
                            type: 'string',
                        },
                    },
                    required: ['name'],
                },
            },
        },
        async (request, reply) => {
            reply.send({});
        }
    );

    const address = await fastify.listen({
        port: 5555,
    });

    console.log(`server listening on ${address}`);
})()
    .then(console.log)
    .catch(console.error);

Thanks

mcollina commented 2 years ago

What you say makes total sense, for your use case preValidation is better.