search

pinojs / pino-pretty

🌲Basic prettifier for Pino log lines
MIT License
1.27k stars 150 forks source link

Refactor causes breaking change #468

Closed Macavirus closed 1 year ago

Macavirus commented 1 year ago

Hi,

Some change in the recent days has caused a major regression causing pino-pretty to be unusable. I'm fairly certain it's related to this change, specifically the addition of the optional chaining operator:

https://github.com/pinojs/pino-pretty/pull/453/files#diff-887bbb0852c0c0b3ae228d8ba60b83eef1c67c73565475e3feea15d1a7930912R31

https://github.com/pinojs/pino-pretty/pull/453/files#diff-b614f57571c9ca2c88504f6e682ba9bc83f5f2dff6515581ab2fa6412ea51facR29

Reproducing:

First, use node 12 using asdf, nvm or another node version manager

echo '{ "level": "DEBUG", "msg": "hola mundo" }' | npx pino-pretty

Result:

npx: installed 38 in 6.709s
Unexpected token '.'

Fix:

I wonder if you would consider changing these back to not use the optional chaining operator. I can make a PR for this if you like. Otherwise it would be great to bump the version as this is a breaking change to (most likely, accidentally) drop node 12 support.

mcollina commented 1 year ago

Node v12 is not supported by the latest releases of this module. We don't test against it, so we can't verify it works correctly:

https://github.com/pinojs/pino-pretty/blob/master/.github/workflows/ci.yml