hapi-pino icon indicating copy to clipboard operation
hapi-pino copied to clipboard

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

Open ebickle opened this issue 5 years ago • 2 comments

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.

ebickle avatar Jan 03 '20 21:01 ebickle

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.

mcollina avatar Jan 05 '20 16:01 mcollina

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)

stevendesu avatar Apr 27 '20 20:04 stevendesu