Skip to content

feat!: remove support for a custom Promise implementation - #3733

Open
pri12ya871 wants to merge 1 commit into
brianc:9.0from
pri12ya871:remove-byo-promise
Open

feat!: remove support for a custom Promise implementation#3733
pri12ya871 wants to merge 1 commit into
brianc:9.0from
pri12ya871:remove-byo-promise

Conversation

@pri12ya871

Copy link
Copy Markdown

Closes #3631.

Removes the Promise constructor option. All four packages now use the global Promise.

package change
pg/lib/client.js drop byoPromiseDeprecationNotice, its trigger and the _Promise field; 5 call sites
pg/lib/native/client.js drop the field; 3 call sites
pg-cursor/index.js drop the field; 2 call sites
pg-pool/index.js drop this.Promise; 6 call sites, plus the first parameter of the private promisify() helper, which existed only to carry it

Open question — scope

I asked this on the issue and went ahead with the full removal so there's something concrete to look at, but I'm happy to cut it down.

pg-pool and pg-cursor are versioned separately from pg, so dropping their Promise option is a breaking change for those packages in their own right, not only for pg@9.0. If you'd rather they went out with a coordinated major of their own, say so and I'll drop those two files — it's a clean partial revert and the pg half stands on its own.

Notes

  • An existing test asserted the old behaviour. test/integration/client/query-as-promise-tests.js did new pg.Client({ Promise: bluebird }) and asserted the returned promise was a bluebird. I inverted it to assert a supplied type is now ignored, using a local class NotAPromise extends Promise so the test no longer needs bluebird at all.
  • Pool#_promiseTry keeps its Promise.try fast path. That method exists on Node 23+ but not on 16–22, which CI still covers, so the fallback has to stay. Now that the constructor is always the global one this could arguably be simplified when the minimum Node version rises.
  • bluebird is left in devDependencies for pg and pg-pool, where it's now unused. Removing it desyncs yarn.lock and CI installs with --frozen-lockfile, so it seemed better as a follow-up than bundled into this. Happy to include it if you'd prefer, with the lockfile regenerated.
  • The docs never documented the option, so there's nothing to update in docs/.

Testing

make test-unit passes locally. I can't run test-integration or test-native here — Windows, no Postgres, and libpq won't build — so I leaned on CI for those. I did exercise the pg-pool paths directly, since pool.end() needs no database: end() returns a native promise, a second end() rejects through the Promise.reject path, _promiseTry resolves, and a supplied Promise is ignored.

Passing `Promise` to a Client, Pool or Cursor has been deprecated since the notice
added for pg@9.0; this removes it. All four packages now use the global Promise.

- pg/lib/client.js: drop `byoPromiseDeprecationNotice` and the `_Promise` field
- pg/lib/native/client.js, pg-cursor/index.js: drop the same field
- pg-pool/index.js: drop `this.Promise`, and with it the first parameter of the
  private `promisify()` helper, which existed only to carry it

`Pool#_promiseTry` keeps its `Promise.try` fast path. That method exists on Node 23
and later but not on 16 through 22, which CI still covers, so the fallback stays.

The integration test that asserted a supplied promise type was honoured now asserts
the opposite, using a local Promise subclass so it no longer needs bluebird. bluebird
is left in devDependencies for now because removing it would desync yarn.lock, and
CI installs with --frozen-lockfile.

Closes brianc#3631
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

Successfully merging this pull request may close these issues.

1 participant