brianc / node-postgres

PostgreSQL client for node.js.
https://node-postgres.com
MIT License
12.21k stars 1.22k forks source link

Add support for native ESM modules #2137

Open Urigo opened 4 years ago

Urigo commented 4 years ago

Hi and thank you for the great library and endless support!

It would be great to have this library export native named exports so we could import it natively on new Node versions.

Here is an example of how you would do it now, without that support: https://github.com/Urigo/typescript-node-es-modules-example/commit/98304173e964713955be3a92b4e355a857376dca#diff-f41e9d04a45c83f3b6f6e630f10117feR10-R11

sebiniemann commented 4 years ago

This would be great 👍 Especially now that NodeJS also supports ESM.

Switching from require/module.exports to import/export and the generation of a CommonJS variant from it is certainly the most future-oriented solution, but seems to involve some adjustments, due to some (unconventional) module management.

I see many require('...') calls inside of functions, which are assumed to run synchronously. One could replace these with dynamic imports (in case the delayed import was really necessary -- like with process.env.NODE_PG_FORCE_NATIVE), but this will also introduce asynchronous behaviour, that needs to be addressed.

In some cases, this also introduced circular dependencies, like pg\lib\index.js -> pg-pool\index.js -> pg\lib\index.js.

A more complicated case is something like

if (...) {
// ...
} else {
  Object.defineProperty(module.exports, 'native', {
    // ...
    get() {
      Object.defineProperty(module.exports, 'native', {
        // ...
      })
      // ...
    },
  })
}

where the export definition is not only delayed, but also later overwritten.

mattbishop commented 1 year ago

I am converting a TypeScript project to ESM now (too many libs are deprecating CJS this year, need to move ahead), and node-postgres is wonky for sure. I have to import classes to a PG namespace and then use it explicitly:

import PG from "pg"
// TS declarations are fine
import {PoolConfig} from "pg"

// classes are not, can't be found via import. Needs to be namespaced.
const pool = new PG.Pool();...
vladkrasn commented 5 months ago

Yup, would be nice to see ESM