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

pg.connect is undocumented #1238

Closed
seishun opened this issue Mar 7, 2017 · 10 comments
Closed

pg.connect is undocumented #1238

seishun opened this issue Mar 7, 2017 · 10 comments
Milestone

Comments

@seishun
Copy link

seishun commented Mar 7, 2017

The pg.connect function is not mentioned in the README or the wiki. But the function exists and it's present in a lot of example code on the Internet.

Is it deprecated? Is using new pg.Pool() preferred to pg.connect? Or are they both fully supported? Either way, it would be nice to have this function mentioned somewhere, so that people know which way to write their own code.

@charmander
Copy link
Collaborator

Yes, pg.Pool instances are preferred to pg.connect.

@brianc
Copy link
Owner

brianc commented Mar 9, 2017

Yeah pg.connect is soft deprecated. I'm probably not going to remove it for a long time, but I'd recommend using an instance of a pool instead of pg.connect - global singletons like pg.connect are generally a nasty design pattern. My self from 6 years ago didn't think that one all the way through. :p SO the thinking goes don't document it anymore, mark it deprecated in a future version, and eventually remove it and roll out a monkey-patch into a separate node-module so if you absolutely need backwards compatibility you can install it still. I hate breaking backwards compatibility.

@seishun
Copy link
Author

seishun commented Mar 9, 2017

Thank you for the clarification.

SO the thinking goes don't document it anymore

I think it would be better to document it. A simple mention like "pg.connect is an old thing, don't use it" would be enough to avoid confusion by newcomers.

@charmander
Copy link
Collaborator

SO the thinking goes don't document it anymore, mark it deprecated in a future version, and eventually remove it and roll out a monkey-patch into a separate node-module so if you absolutely need backwards compatibility you can install it still.

Can that future version be the next version? =)

@pstef
Copy link

pstef commented May 8, 2017

I prefer to have a single connection from the application's perspective. This is because I already use a connection pooler that is external from this perspective. It seems to me that using pg's built-in connection pooler would buy me nothing at the expense of introducing concepts that my application doesn't need.

@charmander
Copy link
Collaborator

@pstef You would want pg.Client, then. pg.connect uses a singleton pool.

@brianc brianc added this to the docs milestone May 24, 2017
@brianc
Copy link
Owner

brianc commented May 24, 2017

I added this to the docs milestone so I'll include it as deprecated when I write them up.

@Yitzi2
Copy link

Yitzi2 commented Jul 18, 2017

Question: What would then be the proper equivalent to the code shown at https://devcenter.heroku.com/articles/heroku-postgresql#connecting-in-node-js?
EDIT: It looks like the connection syntax at https://node-postgres.com/features/connecting#connection-uri explains things. You may want to contact Heroku and suggest they edit their guide to use non-deprecated features, though (I figure it'll count for more if coming from the library author).

@charmander
Copy link
Collaborator

pg.connect has been removed; is this okay to close?

@brianc
Copy link
Owner

brianc commented Jul 24, 2017

👍 definitely good to close

@brianc brianc closed this as completed Jul 24, 2017
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