Improve Deno compatibility: config-first and safe env access

[Jobians]

This PR introduces three changes to make the pg package more compatible with Deno while
keeping full Node.js functionality:

1. Default user value

   • Replace user: process.platform === 'win32' ? process.env.USERNAME : process.env.USER
     with user: 'postgres' in defaults.
   • Avoids requiring environment variables in Deno.

2. Config-first parameter resolution

   • Update val() in connection-parameters.js to return config[key] first, before
     checking environment variables.
   • Prevents Deno errors when --allow-env is not granted.

3. Safe NODE_PG_FORCE_NATIVE check

   • Wrap the NODE_PG_FORCE_NATIVE check in a try/catch.
   • Ensures process.env access in Deno doesn’t throw, while preserving Node.js behavior.

These changes maintain Node.js compatibility, preserve the lazy-loading of the native module,
and allow using the package in Deno without requiring --allow-env.

[Jobians]

@charmander

[Jobians]

??? I will close this is you don't respond again

[NickCrews]

I think the biggest downside of the old pattern, and this suggested implementation, is that they bind too early. We don't need to, and therefore shouldn't decide which backend to use (either the js backend or the native backend) until the user actually tries to create a Client. If we defer this decision until then, then we open up the possibility for the user to request a different backend using a different API beside env vars (because if you decide at import time, this package has no APIs available yet for the user to inform us what they want).

So what about something like the following, which then provides an actual, non-env way to configure which backend the user wants? I don't love the use of a global variable to define the the behavior, but it is the least api-disruptive

const PG = function (clientConstructor) {
  this.defaults = defaults
  this.Client = clientConstructor
  this.Query = this.Client.Query
  this.Pool = poolFactory(this.Client)
  this._pools = []
  this.Connection = Connection
  this.types = require('pg-types')
  this.DatabaseError = DatabaseError
  this.TypeOverrides = TypeOverrides
  this.escapeIdentifier = escapeIdentifier
  this.escapeLiteral = escapeLiteral
  this.Result = Result
  this.utils = utils
  // A new property/config item we add? Not needed if you don't want to
  // increase your API surface area, or maybe requires some more thought.
  this.forceNative = false
}

function makeClient(config) {
    // start with the current config setting
    let useNative = module.exports.forceNative
    try {
        useNative = typeof process.env.NODE_PG_FORCE_NATIVE !== 'undefined'
    } catch {
        // ignore, eg in Deno sandbox without --allow-env
    }
    const actualClient = useNative ? require('./native') : Client
    return actualClient(config)
}

module.exports = new PG(makeClient)

// lazy require native module...the native module may not have installed
  Object.defineProperty(module.exports, 'native', {
...

eg user does

import {Client, forceNative} from 'pg'

forceNative = true
const nativeClient = Client(...)
forceNative = false
const jsClient = Client(...)

[NickCrews]

If I am reading this right, if you are in a deno environment with no --allow-env, then the above will always error, and forceNative will always remain in its default of false, and so it will be impossible to use the native driver. I'm not sure how much of a dealbreaker this actually is (eg does the native driver actually run in deno???).

[charmander]

I think the environment variable was always a mistake (same with the property), and breaking it in a way that’s backwards-compatible is fine. If a future API of the pg package even includes pg-native at all, I suggest it should be imported from pg/native and that there should be only the one way to do it. (I also recommend never using pg-native, since it has extra bugs and less maintenance. The inherent performance difference is minimal, though both pg and pg-native are both missing key optimizations right now.)

[NickCrews]

OK, I also agree that the best API would be to import from pg/native. Then we can skip any new APIs here to select the backend. How about this then?

const PG = ... // same as before

function makeClient(config) {
    let useNative = false;
    try {
        useNative = typeof process.env.NODE_PG_FORCE_NATIVE !== 'undefined'
    } catch {
        // ignore, eg in Deno sandbox without --allow-env
    }
    const actualClient = useNative ? require('./native') : Client
    return actualClient(config)
}

module.exports = new PG(makeClient)

// lazy require native module...the native module may not have installed
  Object.defineProperty(module.exports, 'native', {
...

Is there anything else you need to merge this PR? With that change I'm happy. Honestly, the current suggested implementation is similar enough, I would be fine merging that as well.

[NickCrews]

maybe say explicitly let user = undefined? I know they behave the same, but semantically this bare let user (incorrectly) implies to me that we expect to always set it to a value.