brianc
fix: build `pg-cloudflare` as a CommonJS module
Hey! 👋 We're currently working on an integration between Cloudflare Workers and Vitest, allowing you to run your Vitest tests inside the Workers runtime. A community member tried out an early version of this integration and uncovered an issue with pg/pg-cloudflare (https://github.com/cloudflare/workers-sdk/issues/5127#issuecomment-1989115175).
In pg/lib/stream.js, pg-cloudflare is require()ed...
https://github.com/brianc/node-postgres/blob/b4bfd63f6337018b312bf5a9a70887660954cbfa/packages/pg/lib/stream.js#L10
...but pg-cloudflare is compiled to an ES module. ES modules can't be require()d so this shouldn't work. Fortunately, Workers are usually bundled with esbuild which papers over differences between the module formats and allowed this to work. Our Vitest integration uses a "bundle-less" approach that imports modules at runtime from disk more like Node. This meant pg-cloudflare couldn't be require()d by pg, failing any Workers tests using pg.
This PR updates pg-cloudflare's tsconfig.json to compile to CommonJS instead, fixing this issue. 👍
/cc @petebacondarwin
The problem with converting this module to CommonJS is this line:
https://github.com/brianc/node-postgres/blob/07553428e9c0eacf761a5d4541a3300ff7859578/packages/pg-cloudflare/src/index.ts#L40
If we compile this code to CommonJS this becomes a dynamic require, which the Wrangler bundler then complains about, since the esbuild output results in a dynamic `require("cloudflare:sockets") call.
Could we solve this instead by marking the package as an ESM module (i.e. "type": "module" in its package.json)?
Surely Vite(st) can handle packages that are declared ESM?
Surely Vite(st) can handle packages that are declared ESM?
Not in this case. These files don't get transformed by Vite, and are imported using the workerd module fallback service. This means they have strict module semantics, and require() can't be used to import an ES module. We should be able to keep the dynamic import() in the output rather than converting this to a require(). Not quite sure what the tsconfig.json incantation is though, will investigate more tomorrow. 👍
Looks like the magic incantation is "module": "Node16" and "moduleResolution": "Node16". This compiles to a CommonJS module, but keeps dynamic import()s in the output. 👍