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

Set pino as a peerDependency and pino-pretty as an optionalDependency #95

Open
ebickle opened this issue Jan 3, 2020 · 2 comments
Open

Comments

@ebickle
Copy link

ebickle commented Jan 3, 2020

In the next 'breaking change' release, would it be possible to modify the package.json dependencies to use peer dependencies instead of direct dependencies?

  • Having pino-pretty as a required dependency doesn't match the core pino documentation which states it generally should not be installed for production builds. hapi-pino does not appear to take a dependency on pino-pretty.
  • Having pino as a required dependency creates a scenario where an application could have multiple versions of pino in its node_modules graph - for example, the application may be creating their own pino logger instance directly and passing it in via options.instance. The version of pino used by the application can, in some scenarios, be different than the one directly depended on by hapi-pino.
@mcollina
Copy link
Collaborator

mcollina commented Jan 5, 2020

Having an optionalDependency will have pino-pretty installed anyway, leading to no benefit at all. I’ll drop the dependency completey in the next major release.

As for having pino as a peerDependency, peerDependencies have different behaviors on different versions of npm. I don’t think it should be used.

@stevendesu
Copy link

Technically optional dependencies can be avoided in production using the --no-optional flag

Although in pino's own package.json they list pino-pretty as a dev dependency. I think this is the proper way to go.

Regarding @ebickle 's suggestion to make pino a peer dependency, that's actually not the intended use for peer dependencies. A peer dependency doesn't say "my module depends on X, but I expect you to have X installed for me". It says "my module extends X, and therefore makes no sense in an application that isn't already using X". If that explanation is a bit confusing, here's the simpler answer:

  • hapi-pino has a peer dependency of @hapi/hapi, not pino

The point of peer dependencies is to specify version compatibility. If hapi-pino only works on HapiJS 17.0+, then you can specify a peer dependency of @hapi/hapi@^17.0. This way if you attempt to install hapi-pino in a project which has already included @hapi/hapi@16 it will throw an error (peer dependency unmet)

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

3 participants