Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kaspar-p opened this issue Oct 30, 2024 · 1 comment
Open

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

kaspar-p opened this issue Oct 30, 2024 · 1 comment

Comments

@kaspar-p
Copy link

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants