search

timgit / pg-boss

Queueing jobs in Postgres from Node.js like a boss
MIT License
2.13k stars 158 forks source link

Move `pg` to `peerDependencies`? #109

Closed elmigranto closed 5 years ago

elmigranto commented 5 years ago

npm has peerDependencies which are to be provided by the app. Would it be better to specify library's (soft) need for pg there? I have three main arguments for this:

Right now, my yarn.lock file has two copies of pg very closely versioned with app's one being newer:

yarn list v1.13.0
├─ pg-boss@3.1.4
│  └─ pg@7.8.0
├─ pg-connection-string@0.1.3
├─ pg-int8@1.0.1
├─ pg-pool@2.0.6
├─ pg-types@2.0.0
├─ pg@7.8.1
└─ pgpass@1.0.2
pg@^7.4.1:
  version "7.8.0"
  resolved "https://registry.yarnpkg.com/pg/-/pg-7.8.0.tgz#541c25b3323d85f67ce7d4501a77470976868ce9"
  integrity sha512-yS3C9YD+ft0H7G47uU0eKajgTieggCXdA+Fxhm5G+wionY6kPBa8BEVDwPLMxQvkRkv3/LXiFEqjZm9gfxdW+g==
  dependencies:
    buffer-writer "2.0.0"
    packet-reader "0.3.1"
    pg-connection-string "0.1.3"
    pg-pool "^2.0.4"
    pg-types "~2.0.0"
    pgpass "1.x"
    semver "4.3.2"

pg@^7.8.1:
  version "7.8.1"
  resolved "https://registry.yarnpkg.com/pg/-/pg-7.8.1.tgz#327e25411784cffe067f1cd1ba275d506e92a82b"
  integrity sha512-m9aIrOV4mgfo+1Ze+eNoJwaWZDvpeBz8Kzwi0zzqLC+tQBsQgIuu+FGPqzyRv9HFlS7tHO1I33LKp9gP5g7U4Q==
  dependencies:
    buffer-writer "2.0.0"
    packet-reader "1.0.0"
    pg-connection-string "0.1.3"
    pg-pool "^2.0.4"
    pg-types "~2.0.0"
    pgpass "1.x"
    semver "4.3.2"
elmigranto commented 5 years ago

I see that version specified in package.json for pg-boss is ^7.4.1, which should be resolvable to 7.8.1 used by my app. Guess duplicates are the issue with yarn or something like that…

(upd: I was in the middle of git rebase, after I finished with conflicts and did some other yarn things, duplicates were gone.)

Still, I think the original questions is still a valid one. What's your take, Tim? @timgit

timgit commented 5 years ago

The reference dependency list you included already has pg, so if that's the case pg is already there. What's the problem that you're trying to solve with this? Did you receive any errors, or is this an optimization request? I use pg-boss both with and without a shared pool from another framework, but ultimately it's all pg at the end of the day. What's the harm in having pg located in multiple places in node_modules? This is quite common and allows each package version isolation just in case it's needed.

elmigranto commented 5 years ago

I noticed that I had 2 pg versions, but it was due to my setup and it went away after I was done with a rebase. Both, app and pg-boss eventually resolved to the same up-to-date pg version. This was the main reason I filed this issue, but since it ultimately had nothing to do with pg-boss, I think we can close this.

There are some other arguments that can be made for the case of having pg in peerDependencies for pg-boss beyond that, but I don't think they warrant doing anything at this point. (E.g. having something else other than Postgres that supports its wire protocol, or having some kind of proxy, or different pg version, etc.)

Anyhow, thanks for responding, I think I mostly agree with you here. Cheers ♥️