redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.28k stars 991 forks source link

Warning : prettyPrint is deprecated, use the pino-pretty transport instead #3610

Closed simoncrypta closed 3 years ago

simoncrypta commented 3 years ago

I upgrade to canary (0.37.6-canary.57) on my project. Every time I make on update on the codebase, I got : [PINODEP008] PinoWarning: prettyPrint is deprecated, use the pino-pretty transport instead

This came from the upgrade of Pino to V7 #3588 https://github.com/pinojs/pino/blob/master/docs/api.md#prettyprint-boolean--object

The prettyPrint option is deprecated, but everything seem to still working. The solution is to use pino-pretty transport, which will came with #3298

Do you feel that it is a priority to fix this ?

thedavidprice commented 3 years ago

Good heads up @simoncrypta I think we're good until v0.38 comes out (which is likely next week).

Closing this but looping in @dthyresson for additional thoughts (and to reopen if needed).

dthyresson commented 3 years ago

If possible we should allow pretty printing in development to stdout. Any file or other destination doesn’t make sense as if should be in the natural NDJson format.

@simoncrypta do you think this is possible without a warning?

Or remove the option and always pretty print to stdout in development only?

simoncrypta commented 3 years ago

If possible we should allow pretty printing in development to stdout. Any file or other destination doesn’t make sense as if should be in the natural NDJson format.

@simoncrypta do you think this is possible without a warning?

Or remove the option and always pretty print to stdout in development only?

Is possible to use pino-pretty transport to stdout without problem, the only thing I don't know how we would handle is this part :

  // override, but retain default pretty print options
  if (isPretty && options && options.prettyPrint) {
    const prettyOptions = {
      prettyPrint: {
        ...(defaultLoggerOptions.prettyPrint as PrettyOptions),
        ...(options.prettyPrint as PrettyOptions),
      },
    } 
dthyresson commented 3 years ago

Maybe remove the option altogether and always pretty print to the stdout in the dev server?

dthyresson commented 3 years ago

Actually sorry that didn’t make sense what I just wrote. I need to look a bit closer, but if we have to limit or standardize the pretty output that may be ok

simoncrypta commented 3 years ago

always pretty print to the stdout in the dev server

We already do it because we retain default pretty print options, right? The problem is we will force one way of doing logging in dev. It can be okay, we can also add LoggingDev property to createLogger who will overwrite our default transport in dev (pretty print to the stdout).