holdfenytolvaj / pogi

Javascript library for PostgreSQL and node.js
MIT License
140 stars 15 forks source link

Allow passing a Pool to the constructor #20

Open ab-pm opened 5 years ago

ab-pm commented 5 years ago

We're not using Pogi as the only library for connecting to our database, and for that we instantiate a pg.Pool ourselves which we share between different services. It would be dope if Pogi could be instantiated to use such a pre-existing pool, instead of getting passed connection options only and managing the pool (or actually, all the pools for each connection string).

If you still want to pool the PgDB instances, I guess it should be possible to get the (undocumented) .options from the Pool instance to create your connection string key.

holdfenytolvaj commented 5 years ago

It is not officially documented, but I don't see why https://github.com/holdfenytolvaj/pogi/blob/master/src/pgDb.ts#L94

let pogi = new PgDb({pool});
await pogi.reload();

wouldn't work. Only the setLogger is missing in this way, https://github.com/holdfenytolvaj/pogi/blob/master/src/pgDb.ts#L176

ab-pm commented 5 years ago

Thanks, I tried and it seems to work okay. I did have an issue with the options though, which although documented as optional in that line https://github.com/holdfenytolvaj/pogi/blob/7d987e0ef81e4870ca2046ec52486f87e721c69f/src/pgDb.ts#L94

is used unconditionally on this.db.options at https://github.com/holdfenytolvaj/pogi/blob/0a5712fe428500963223805a8fef4df40fef86aa/src/pgTable.ts#L244 (and elsewhere in a similar pattern). The PgDb constructor should use this.config = pgdb.config || {};.

So I ended up with

// @ts-ignore constructor is private
pgdb = new PgDb({pool: staticPool, config: {}});
ab-pm commented 5 years ago

I faced quite a big problem with a non-matching dependency of pg. I was using the following code to get my enum array type parsed as expected:

import { Pool, types } from 'pg';
import { PgDb } from 'pogi';

…
const myPool = new Pool(…);
const pgdb = new PgDb({pool: myPool, config: {}});
…
await pgdb.setTypeParser('_myenum', types.getTypeParser(1009)); // text array

and it just didn't work. Took me some time to figure out that pogi was using a separate pg instance (local package with different version), and it called types.setTypeParser on that internal one which didn't get used by the connection from myPool.

So please use peerDependencies to specify the pg dependency, and (while I personally don't have a need for it) I guess it would be good also for moment.

holdfenytolvaj commented 5 years ago

Sorry, for being late, if it is urgent please give me a pull request, Otherwise it will take a while for me.