Pg-promise: Suggestion - simplify documentation for newcomers

[jabooth]

I've been (very happily!) using pg-promise for around 18 months now. Whilst the library itself has been very solid, I have to admit I found it a little confusing at the start going through the documentation to get a grasp of what is the canonical way to achieve a task in pg-promise, largely due to the fact that README.md contains sections which I don't feel are very relevant to new users.

I'm revisiting the docs now to see what's changed in the last 6 months or so since I last looked. Whilst things are definitely improved, there's a few things there that I would humbly suggest you change to streamline on boarding of new users.

1. Don't describe Shared Connections

You strongly advise against using this feature in favour of tasks and even go as far as saying it's obsolete, but you still explain and give examples. As a new user reading down the README, I don't think you should even introduce this concept at this stage. This really simplifies the mental model for connections in pg-promise in my opinion. This whole section can be simplified nicely, to something along the lines of:

1. By default queries in pg-promise are independent of each other - every query executed by default gets it's own dedicated connection to operate in. These connections come out of a pool pg-promise maintains for efficiency, but you don't need to worry about that.

2. Of course sometimes we don't want independent queries - we often need to execute a series of queries together in a transaction, where we can control the behaviour of the set of queries (we might for instance, want to roll back all changes made if any one subquery fails). For this pg-promise provides tasks.

   Note: pg-promise also has another type of connection called a shared connection, but this was superseded by tasks. If you have legacy code that uses this feature, go here [link to Wiki] for information. If you don't have legacy code using this feature, don't worry about it.

   2. Don't promote the fact that you can take the task object as an input parameter (t) or this in a task callback function

This really flummoxed me when getting started with pg-promise. I know it's simple, but I just couldn't get why there would be two options. In examples often both options are show simultaneously, e.g.:

db.tx(function (t) {
    // t = this;
    var q1 = this.none('update users set active=$1 where id=$2', [true, 123]);
    ...
})

call me stupid, but all of the following went through my head:

1. Why is that assignment to t from this commented out? Is it a suggestion, that I should consider performing that assignment at the start? If so why would I assign to function argument?
2. Wait, t is provided to the function but isn't used...why is it there then? If I remove the t parameter would everything still work, or is there some magic stuff running that means I still have to pass t even if I choose to use this?
3. Why would a library have a callback interface based on assigning to this? Surely an input parameter is far more natural and explicit?
4. How would the this assignment be impacted by all the crazy Javascript rules around this? Can I use a fat arrow function here, or would that screw up the this setting?

I really don't feel the this option is remotely worth the confusion it causes, but I get that there is no way you will likely remove the feature from the library given that there are so many users out there who likely use that style. All I would argue is that you don't mention it to new users. Just go with the function argument - it's very similar to the Promise .then((res) => {}) API, and avoids a whole bunch of confusion. Have all the examples show the argument option, then right at the end give the this usage in one clean example (without the t argument).

Anyway, there's my 2 cents. I would be more than happy to prepare a PR where I implement these changes in the README, if that is something you would be receptive to?

[vitaly-t]

I agree on most of what's been said here, just wish I had the time to rewrite the main readme file...

I would be more than happy to prepare a PR where I implement these changes in the README, if that is something you would be receptive to?

Perhaps if you could do it in smaller parts :wink:

[jabooth]

Sure thing! I'll just chip away at little corners of it when I get chance 👍

[vitaly-t]

I have started making some improvements (all checked into the master):

• removed the whole Connection section, not to get into the shared connection stuff at all, even though it is not quite obsolete yet, see example for LISTEN/NOTIFY.
• simplified transaction chapters
• Replaced all t = this notes with t and this here are the same

[vitaly-t]

Closing it for now, due to inactivity.