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

Define prom-client as Peer Dependency instead of Direct Dependency #126

Closed
n0v1 opened this issue Jan 22, 2021 · 2 comments · Fixed by #127
Closed

Define prom-client as Peer Dependency instead of Direct Dependency #126

n0v1 opened this issue Jan 22, 2021 · 2 comments · Fixed by #127
Assignees

Comments

@n0v1
Copy link
Contributor

n0v1 commented Jan 22, 2021

We use swagger-stats together with prometheus-gc-stats. Currently swagger-stats defines prom-client as a direct dependency with version range "^11.5.3". prometheus-gc-stats has a peer dependency on prom-client with version range ">= 10 <= 12".

Because it's a peer dependency of prometheus-gc-stats and because we need to add some custom metrics, we must add a direct dependency on prom-client to our own code ("my-app"). This has version range "^12.0.0".

We end up with a dependency tree like this:

my-app/
  node_modules/
    prom-client@12.0.0/
    prometheus-gc-stats/
    swagger-stats/
      node_modules/
        prom-client@11.5.3/

This means swagger-stats is using it's own copy of prom-client and we're not able (at least not easily) to return all metrics at once. The problem is that it's quite hard to find out what is going on here. Most people will just notice that they are missing some metrics.

One reason to workaround this would be to downgrade our version of prom-client to v11. Then we would end up with one single version of prom-client in the tree (because of hoisting) and everything would work. But this is a rather fragile solution because every developer has to know that we must not upgrade it. When there is a new release of swagger-stats with a new major version of prom-client we have to remember to also upgrade our version...

The solution to this would be for swagger-stats to add prom-client as a peer dependency. Then the host package ("my-app") has to provide a compatible version of prom-client to swagger-stats. npm/yarn will make sure this exists and warn if not.

I can provide a pull request with the necessary changes. This change would require a major release I guess.

This related to #6 and #114.

@n0v1
Copy link
Contributor Author

n0v1 commented Jan 25, 2021

Thanks for accepting this, @sv2. I created another pr with some docs changes: #128.

@ssbarbee
Copy link

This should of been a major release ^_^

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

Successfully merging a pull request may close this issue.

3 participants