New exports condition breaks require('pg/lib/type-overrides')

[mmkal]

Hi! I got a bug report in pgkit which uses pg/lib/type-overrides: https://github.com/mmkal/pgkit/issues/443

It seems to be because of the new exports definition on package.json

Here's an easy repro you can run in another project (empty directory after npm init -y, say):

npm i pg@~8.14.0 && node -e "require('pg/lib/type-overrides')" # succeeds
npm i pg@~8.15.0 && node -e "require('pg/lib/type-overrides')" # fails
npm i pg@~8.16.0 && node -e "require('pg/lib/type-overrides')" # fails

Error for the latter two looks like:

Error: Cannot find module '/Users/mmkal/src/scratch/node_modules/pg/lib/type-overrides'
    at createEsmNotFoundErr (node:internal/modules/cjs/loader:1441:15)
    at finalizeEsmResolution (node:internal/modules/cjs/loader:1430:15)
    at resolveExports (node:internal/modules/cjs/loader:661:14)
    at Function._findPath (node:internal/modules/cjs/loader:753:31)
    at Function._resolveFilename (node:internal/modules/cjs/loader:1391:27)
    at defaultResolveImpl (node:internal/modules/cjs/loader:1061:19)
    at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1066:22)
    at Function._load (node:internal/modules/cjs/loader:1215:37)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:235:24) {
  code: 'MODULE_NOT_FOUND',
  path: '/Users/mmkal/src/scratch/node_modules/pg'
}

If you go into node_modules and delete the exports prop on package.json, it works ok again.

It also works if you add ./lib/type-overrides explicitly, so one option might be to explicitly write out every supported import like this:

  "exports": {
    ".": {
      "import": "./esm/index.mjs",
      "require": "./lib/index.js",
      "default": "./lib/index.js"
    },
    "./lib/type-overrides": {
      "import": "./esm/type-overrides.mjs",
      "require": "./lib/type-overrides.js",
      "default": "./lib/type-overrides.js"
    },
    "./lib/*": {
      "import": "./lib/*",
      "require": "./lib/*",
      "default": "./lib/*"
    }
  },

Possibly related to https://github.com/brianc/node-postgres/issues/3457 but arguably this is more serious since it affects runtime.

[pmalouin]

We're also affected by this issue.

It looks like the fix in https://github.com/brianc/node-postgres/pull/3428 solved the same issue just for extensioned require()s like require('pg/lib/type-overrides.js'), but not for extensionless expressions like require('pg/lib/type-overrides').

Sharing some of my findings while tinkering...

I noticed that adding .js at the end of the exports fixes calls to require('pg/lib/type-overrides'):

  "exports": {
    ".": {
      "import": "./esm/index.mjs",
      "require": "./lib/index.js",
      "default": "./lib/index.js"
    },
    "./lib/*": {
      "import": "./lib/*.js",
      "require": "./lib/*.js",
      "default": "./lib/*.js"
    }
  },

...but that breaks support for extensioned paths, so to maintain compatibility you'd also need two entries like:

  "exports": {
    ".": {
      "import": "./esm/index.mjs",
      "require": "./lib/index.js",
      "default": "./lib/index.js"
    },
    "./lib/*": {
      "import": "./lib/*.js",
      "require": "./lib/*.js",
      "default": "./lib/*.js"
    },
    "./lib/*.js": {
      "import": "./lib/*.js",
      "require": "./lib/*.js",
      "default": "./lib/*.js"
    }
  },

As an aside, I think this can actually be simplified to:

  "exports": {
    ".": {
      "import": "./esm/index.mjs",
      "require": "./lib/index.js",
      "default": "./lib/index.js"
    },
    "./lib/*": "./lib/*.js",
    "./lib/*.js": "./lib/*.js"
  },

[brianc]

Sorry for the delay here & ultimately for breaking import compatibility. I didn't understand what a nightmare upgrading to esm exports while still maintaining commonjs exports would be & what a cost/impact it would have on consumers. I've just submitted a PR which hopefully addresses this (tests included) - I would love it if you could take a look. Also, if there are other files which aren't importing correctly both with and without the .js extension please let me know. I'd love to put this behind us and move into the glorious esm future without accidentally shotgunning thousands of code-bases with accidental incompatible breaking exports!

[brianc]

also shout out to @pmalouin for supplying the soultion! Very very very helpful for me & it's greatly appreciated! ❤