pinojs / pino

🌲 super fast, all natural json logger
http://getpino.io
MIT License
14.39k stars 885 forks source link

Allow transport targets to be exports from the target library #2073

Open kaspar-p opened 3 weeks ago

kaspar-p commented 3 weeks ago

Targets by default, as in the docs, are strings like "pino/file" or "pino-pretty". This implicitly requires any package using these transports to have this NPM package installed in the correct way, and any bundling they do to keep this pattern.

There are existing tools like depcheck that can analyze source code to find that all dependencies used are actually installed, and can warn against dependencies that are not installed. However, depcheck doesn't work in this case since these imports are dynamic.

To take advantage of existing static analysis tools it'd be nice if Pino transports were not named by an arbitrary string, but rather some interface that the transport library could export, and pino could expect.

For example:

import { pino } from "pino"l
import { pinoPretty } from "pino-pretty";

pino(pino.transport({
  targets: [
    { target: pinoPretty, level: 'info', options: { destination: 1 }
  ],
}));

and in this way, analysis tools would find usages of transports that aren't installed, and it's obvious to users that these transports need to be installed.

So it'd be nice for target to be type string | PinoTarget or something, where PinoTarget might be:

export type PinoTarget = {
  name: string;
}
mcollina commented 2 weeks ago

This implicitly requires any package using these transports to have this NPM package installed in the correct way, and any bundling they do to keep this pattern.

This is unfortunately incorrect. It's not enough. Bundling usually plays badly with worker threads, making it very hard to spawn threads correctly. In other words, adding the correct dependency is not enough.

I would be happy to do this if there was a way to handle this correctly, but so far I have not found a good pattern that would reduce the friction for bundlers.