add pg option, a simple test and some doc
#30
Conversation
|
While I think this does not belong to the scope of this plugin, it also enables extensibility. @mcollina what do you think? |
|
yes
Il giorno mer 10 ott 2018 alle 07:33 Cemre Mengu <[email protected]>
ha scritto:
… Merged #30 <#30> into
master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADL4y_XT_6iesE_Crbxd7W-x0sqpGFnks5ujgUsgaJpZM4XUz3J>
.
|
|
|
||
| function fastifyPostgres (fastify, options, next) { | ||
| let pg = defaultPg | ||
| if (options.pg) { |
There was a problem hiding this comment.
While this seems legit to be able to pass pg into it, but I don't think there's a good use case.
At this line pg === defaultPg is actually true even if you pass the pg instance after patching it with pg-range, so not sure if there's any benefit in passing it altogether.
All pg-range does is patch the pg, it doesn't return a new/different object. So just requireing pg-range before fastify-postgres should do the trick.
Or am I missing something?
There was a problem hiding this comment.
While I am not a continuous postgres user, it looks like require('pg') returns a factory and plugins are installed to that factory instance only so you need to pass that factory instance in?
@temsa do you mind trying the suggestion here to see it works?
There was a problem hiding this comment.
I agree @cemremengu.
Also, the main reason I'm skeptical about this is that fastify-postgres is a dedicated library for pg. We do not support any other similar postgres module. Which is why I think allowing the user to pass a pg instance is not so intuitive because the user is free to pass any object and we have no control over it.
There was a problem hiding this comment.
This also enables mocking, which might be a good feature to support.
4 participants
should fix #28
The test of the option is very simple and could probably be better