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

Can dtrace be a peer dependency? #601

Open
kbirger opened this issue Aug 14, 2018 · 4 comments
Open

Can dtrace be a peer dependency? #601

kbirger opened this issue Aug 14, 2018 · 4 comments

Comments

@kbirger
Copy link

kbirger commented Aug 14, 2018

Currently it is an optional dependency, and that's great. But it seems like it would be better to make it a peer dependency.

I am running a containerized node app, and I'm trying to keep my image as small as possible. I don't need dtrace, and I don't want to install python. Currently I get non-fatal errors when when building my image. As they are non-fatal, it is not a show stopper, but it still takes up time and is inelegant to have the errors show up in the log.

If I do install python, it still takes time to install the dtrace dependency, even if I have no use for it.

It seems like the best way to go would be to allow consumers to manually install the dtrace provider module if they need it, as it behaves differently on different platforms and not every consumer of Bunyan will want it.

I recognize that this could be construed as a breaking change as it would likely cause dtrace to stop being installed automatically for some folks.

@hulbert
Copy link

hulbert commented Nov 20, 2018

@kbirger FYI we are maintaining a fork with dtrace removed (as well as a few other optional dependencies)

https://github.com/takescoop/node-bunyan
https://www.npmjs.com/package/@scoop/bunyan

@voxpelli
Copy link

Great @hulbert, I'll probably start using that one as well. Been thinking about doing the same, but no need for multiple forks 👍

@thom-nic
Copy link

I don't even use bunyan directly however there are still a ton of packages that depend on "the real" bunyan, hence pull in this package along with dtrace-provider and cause headaches. And despite bunyan putting a try/catch around the require, it can still fail in a way that kills the node process. See: chrisa/node-dtrace-provider#118

It would be nice if we could point all transitive bunyan dependencies to the forked version, however I don't think there's a way to get npm to do such a thing.

Given the number of issues caused by dtrace - especially vs the number of ppl who I imagine are actually using it - I think it would really make lives easier to do something about dtrace-provider.

Actually, doesn't npm always complain loudly if peer dependencies are missing? Would a peerDependency actually fix anything w/r/t npm install?

It's possible, the best solution might be to treat dtrace-provider like a plugin - don't list it as a dependency at all. Then require() in a try/catch and fail silently if it's not found. Pretty much current behavior just without any explicit dependency in package.json.

@Paladinium
Copy link

As @thom-nic says: the fork does not help if all other packages still refer to the 'real one'. With dtrace-provider being incompatible with Node 14 (see chrisa/node-dtrace-provider#132) and at the same time not being maintained, bunyan should consider replacing dtrace-provider.

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

5 participants