[pyramation]

Summary

@pgsql/parser's import entry points are ES modules in .js files, but the package has no "type": "module". Node 18, and Node 20 before 20.19, therefore load them as CommonJS, so import '@pgsql/parser' and every @pgsql/parser/v13…v18 subpath fail with SyntaxError: Unexpected token 'export'. Newer Node versions only work because of module syntax detection. require was never affected, and neither are the six libpg-query packages (they have no exports map, so import resolves to main, which is .cjs).

Repro with 1.6.2 on Node 18.20.8:

node --input-type=module -e "await import('@pgsql/parser/v18')"
SyntaxError: Unexpected token 'export'

Changes, all in parser/:

• prepare.js writes the ESM files as .mjs: wasm/index.mjs, wasm/v<n>.mjs and wasm/v<n>/index.mjs, which was copied from versions/<n>/wasm/index.js. The export * as v<n> and dynamic import() specifiers now point at index.mjs.
• package.json: the import conditions and module now point at .mjs. types is listed first in each condition so TypeScript keeps resolving index.d.ts (checked with tsc --module nodenext against the packed tarball).
• test/esm.test.mjs: imports the root and each subpath in a child node --no-experimental-detect-module. That flag reproduces the Node 18 behaviour on current Node.

Evidence:

• Before the fix, the 7 new tests fail on Node 24 (ERR_REQUIRE_CYCLE_MODULE), and on Node 18 all 14 ESM tests fail.
• After the fix, the parser suite passes (40/40) on Node 18.20.8, 20.20.2, 22.23.4 and 24.22.1.
• An npm pack tarball works with ESM import and CJS require for the root Parser and all six subpaths on all four Node versions.

Link to Devin session: https://app.devin.ai/sessions/68876ce060664609ab6b63c8bed91e78
Open in Devin Desktop: https://app.devin.ai/desktop/session/68876ce060664609ab6b63c8bed91e78?variant=devin
Requested by: @pyramation

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

• Disable automatic comment, CI, and merge conflict monitoring

[tenki-reviewer]

Review complete. 🟠 1 high · 🟡 1 medium

💬 Inline comments (1)

• 🟡 spawnSync spawn failures surface as null == 0 — esm.test.mjs:42

📍 Findings outside the diff (1) — 🟠 1 high — defects on lines GitHub can't attach comments to

🟠 High — Renamed index.mjs skips the @pgsql/types rewrite · prepare.js:125–128 · unchanged line

// parser/scripts/prepare.js
125	      if (file.endsWith('.js') || file.endsWith('.cjs') || file.endsWith('.d.ts')) {
126	        let content = fs.readFileSync(destPath, 'utf8');
127	        // Node's ESM resolver rejects directory imports, so ESM files need the full path.
128	        const typesPath = file.endsWith('.js') ? './types/index.js' : './types';

The copy step now renames each version's ESM entry to index.mjs (parser/scripts/prepare.js:118), but the @pgsql/types rewrite only runs for .js/.cjs/.d.ts files (parser/scripts/prepare.js:125), so the renamed file is copied verbatim. The built entry point really contains export * from "@pgsql/types" (versions/13/src/index.ts:8), and no such rewrite means the published ESM entry references an unresolvable package specifier. Net effect: import '@pgsql/parser' and import '@pgsql/parser/vN' fail at module resolution — the exact failure this PR set out to fix.

Even if the rewrite condition is widened, the path selection ternary at parser/scripts/prepare.js:128 must treat the renamed file as ESM (./types/index.js), not the directory form ./types that Node's ESM resolver rejects.

---

This PR switches the per-version ESM entry point to index.mjs and adds a spawnSync-based smoke test that exercises the installed package under --no-experimental-detect-module for Node version parity.

Files	Change
parser/package.json	Repoints the ESM export/type entries at the new index.mjs files.
parser/scripts/prepare.js	Renames generated ESM entries to index.mjs during prepare; rewrite logic only covers .js/.cjs/.d.ts.
parser/test/esm.test.mjs	Adds a child-process ESM import test with Node-version flag handling.

Risk concentrates on published package resolution: the renamed ESM entry bypasses the existing @pgsql/types import rewrite, and the test harness discards child-process error details.

_{Reviewed commit: eee3f00}

[tenki-reviewer]

Renames the generated ESM entry to index.mjs and adds an ESM smoke test, but the renamed entry skips the @pgsql/types rewrite and the test masks spawn failures.

Key findings

• 🟠 Renamed index.mjs skips the @pgsql/types rewrite — prepare.js:125
• 🟡 spawnSync spawn failures surface as null == 0 — esm.test.mjs:42

[tenki-reviewer]

🟡 bug · medium

spawnSync spawn failures surface as null == 0

spawnSync reports spawn failures (ENOENT, EACCES) and signal kills as status: null plus an error field, which the test destructures away (parser/test/esm.test.mjs:42). assert.equal(status, 0, stderr) at parser/test/esm.test.mjs:46 then fails as null == 0 with an empty or misleading message, turning an infrastructure failure into an opaque one. The child's stdout is also discarded and the inline script throws a bare 'bad parse', so a failing matrix entry may give no actionable output.

📋 Prompt for AI Agents

In parser/test/esm.test.mjs lines 42-46, destructure error and stdout from the spawnSync result; before assert.equal(status, 0, stderr) add assert.ifError(error), and change the assertion message to include the specifier and both streams, e.g. assert.equal(status, 0, import ${specifier} failed\nstdout: ${stdout}\nstderr: ${stderr}), so spawn failures (ENOENT, signal kills) and parse failures produce actionable CI output instead of null == 0 with an empty stderr.

[devin-ai-integration]

Folded into #190