make generic options in params available

[oitan]

when trying to use express Request and Response for pinoHttp options, TypeScript argues that they should be of type IncomingMessage and ServerResponse (the default ones). This make generics for pinoHttp available again inside of Params type.

Code:

import { Environment } from 'src/config/environment.enum';
import { stdTimeFunctions } from 'pino';
import { XCorrelationIdHeader } from './headers.const';
import { AppConfigService } from 'src/config/app-config.service';
// import { Params } from '../../../nestjs-pino/src';
import { Params } from 'nestjs-pino';

export function getLoggerConfig(appConfigService: AppConfigService): Params {
  const pinoHttp: Options<Request, Response> = {
    genReqId: (req) => req.headers[XCorrelationIdHeader] || randomUUID(),
    level: appConfigService.env === Environment.PRODUCTION ? 'info' : 'debug',
    formatters: {
      level: (label) => ({ lvl: label.toUpperCase() }),
    },
    serializers: {
      req: (req: Request) => ({ id: req.id }),
    },
    base: undefined,
    timestamp: stdTimeFunctions.isoTime,
    nestedKey: 'payload',
    customReceivedObject: (req: Request, _res, _val) => ({
      url: req.url,
      method: req.method,
      query: req.query,
      params: req.params,
      body: req.body,
    }),
    customReceivedMessage: (_req, _res) => 'new API request',
  };
  // ERROR ON THE NEXT LINE
  return { pinoHttp };
}

TS Error:

Type 'Options<Request<ParamsDictionary, any, any, ParsedQs, Record<string, any>>, Response<any, Record<string, any>>, never>' is not assignable to type 'Options<IncomingMessage, ServerResponse<IncomingMessage>, never> | DestinationStream | [...] | undefined'.
     Type 'Options<Request<ParamsDictionary, any, any, ParsedQs, Record<string, any>>, Response<any, Record<string, any>>, never>' is not assignable to type 'Options<IncomingMessage, ServerResponse<IncomingMessage>, never>'.
       Types of property 'genReqId' are incompatible.
         Type 'GenReqId<Request<ParamsDictionary, any, any, ParsedQs, Record<string, any>>, Response<any, Record<string, any>>> | undefined' is not assignable to type 'GenReqId<IncomingMessage, ServerResponse<IncomingMessage>> | undefined'.
           Type 'GenReqId<Request<ParamsDictionary, any, any, ParsedQs, Record<string, any>>, Response<any, Record<string, any>>>' is not assignable to type 'GenReqId<IncomingMessage, ServerResponse<IncomingMessage>>'.
             Type 'IncomingMessage' is missing the following properties from type 'Request<ParamsDictionary, any, any, ParsedQs, Record<string, any>>': get, header, accepts, acceptsCharsets, and 32 more. [2322]

PRs solution allow to which resolves the issue:

Params<Request, Response>

[iamolegga]

Thank you for this PR, the only one thing that disturbs me is that this feature was introduced in pino-http@v8.5.0, but in current package-json we support pino-http since v6.4.0. So, I believe it could be an error for users with outdated versions, to check that we need to provide pino-http versions as well for the CI tests here and here, and after we can decide if this is a breaking change or not, and based on that we will release this feature in a new major or fix version

[oitan]

I have added v6.4, v7, v8, v9, v10 into pino-http-version field of the matrix (and disabled other github action jobs, only build-lint-test is active for faster testing purposes). As you can see, all the other ones are failed except for version 10. I don't know what it means, since you said that we support from v6.40.I don't think the code I have changed affected the tests, since generic defaults for Params are the same as generic defaults for Options of pinoHttp field. So I guess those are general breaking change issues between versions. Should I try to run the tests without my changes as well? Will it help to understand the situation?

In the image in the jobs list, the right-most number in braces are the pino-http-version values.

[iamolegga]

Thanks, will get back to this soon

[oitan]

have removed my changes to see the gh action result. still seems to be the same.

[oitan]

Thanks, will get back to this soon

@iamolegga just let me know if I can do something else or check something. I am happy to finish this PR or close it if it doesn't work at this particular moment. I understand that you are busy, and I am happy to do all the necessary checks, test hypotesis, etc. Just guide me a little bit please :)

[iamolegga]

@oitan thanks, just give me a couple of days, I'll check everything on my own and we will merge it