diff --git a/lib/esm-transformer.ts b/lib/esm-transformer.ts index fa331ad3..b0dc1355 100644 --- a/lib/esm-transformer.ts +++ b/lib/esm-transformer.ts @@ -225,6 +225,76 @@ export function rewriteMjsRequirePaths(code: string): string { ); } +// CommonJS-resolvable conditions, in the order Node's require() resolver +// honours them. If a conditions object exposes none of these, require() cannot +// resolve it (Node throws ERR_PACKAGE_PATH_NOT_EXPORTED). +const CJS_EXPORT_CONDITIONS = ['require', 'node', 'node-addons', 'default']; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type ExportsValue = any; + +/** + * Make a package.json "exports" value resolvable through CommonJS `require()` + * after pkg has transformed the package's ESM sources to CJS. + * + * pkg compiles every ESM module to CommonJS and renames the package's `.mjs` + * files to `.js` in the snapshot. The "exports" field, however, is left as the + * package author wrote it, so at runtime Node's CJS resolver (which always + * prefers "exports" over "main") still sees `import`-only conditions and/or + * targets pointing at the now-renamed `.mjs` files. That produces + * ERR_PACKAGE_PATH_NOT_EXPORTED (import-only packages) or MODULE_NOT_FOUND + * (targets pointing at a `.mjs` file that no longer exists). + * + * This rewrites the field to mirror the transformation pkg already applied: + * - every string target has its `.mjs` extension rewritten to `.js`, and + * - any conditions object that lacks a CJS-resolvable condition gets a + * synthetic `require` condition mirroring its `import` (or `default`) target, + * so `require()` resolves to the transformed file. + * + * @param exportsField - The raw "exports" value from package.json + * @returns A new "exports" value resolvable via `require()` + */ +export function normalizeExportsForCJS( + exportsField: ExportsValue, +): ExportsValue { + if (typeof exportsField === 'string') { + return exportsField.replace(/\.mjs$/, '.js'); + } + + if (Array.isArray(exportsField)) { + return exportsField.map((entry) => normalizeExportsForCJS(entry)); + } + + if (exportsField && typeof exportsField === 'object') { + const keys = Object.keys(exportsField); + // A conditions object uses condition names as keys (e.g. "import", + // "require"); a subpath map uses "." / "./sub" keys. They never mix, so the + // presence of any condition key means this is a conditions object. + const isConditions = keys.some((key) => !key.startsWith('.')); + + const result: ExportsValue = {}; + for (const key of keys) { + result[key] = normalizeExportsForCJS(exportsField[key]); + } + + if ( + isConditions && + !keys.some((key) => CJS_EXPORT_CONDITIONS.includes(key)) + ) { + // Import-only conditions object: add a `require` target mirroring the ESM + // one so the transformed file is reachable through require(). + const fallback = result.import ?? result.default; + if (fallback !== undefined) { + return { require: fallback, ...result }; + } + } + + return result; + } + + return exportsField; +} + /** * Transform ESM code to CommonJS using esbuild * This allows ESM modules to be compiled to bytecode via vm.Script diff --git a/lib/walker.ts b/lib/walker.ts index 1c2f276b..403e9fff 100644 --- a/lib/walker.ts +++ b/lib/walker.ts @@ -26,7 +26,11 @@ import { pc } from './colors'; import { follow } from './follow'; import { log, wasReported } from './log'; import * as detector from './detector'; -import { transformESMtoCJS, rewriteMjsRequirePaths } from './esm-transformer'; +import { + transformESMtoCJS, + rewriteMjsRequirePaths, + normalizeExportsForCJS, +} from './esm-transformer'; import { ConfigDictionary, FileRecord, @@ -1081,6 +1085,24 @@ class Walker { } } + // If package has an "exports" field, rewrite it so the snapshot is + // resolvable through CommonJS require(). pkg transforms ESM to CJS and + // renames .mjs files to .js, but Node's CJS resolver always prefers + // "exports" over "main"; without this, import-only or .mjs-targeted + // exports fail at runtime (ERR_PACKAGE_PATH_NOT_EXPORTED / MODULE_NOT_FOUND). + if (pkgContent.exports && !this.params.seaMode) { + const normalizedExports = normalizeExportsForCJS( + pkgContent.exports, + ); + if ( + JSON.stringify(normalizedExports) !== + JSON.stringify(pkgContent.exports) + ) { + pkgContent.exports = normalizedExports; + modified = true; + } + } + // If package has "type": "module", we need to change it to "commonjs" // because we transform all ESM files to CJS before bytecode compilation if (pkgContent.type === 'module' && !this.params.seaMode) { diff --git a/test/test-54-esm-exports-conditions/app/app-a.mjs b/test/test-54-esm-exports-conditions/app/app-a.mjs new file mode 100644 index 00000000..7b51c7b7 --- /dev/null +++ b/test/test-54-esm-exports-conditions/app/app-a.mjs @@ -0,0 +1,3 @@ +import { a } from 'esm-only'; + +console.log(a); diff --git a/test/test-54-esm-exports-conditions/app/app-b.mjs b/test/test-54-esm-exports-conditions/app/app-b.mjs new file mode 100644 index 00000000..cc31b02c --- /dev/null +++ b/test/test-54-esm-exports-conditions/app/app-b.mjs @@ -0,0 +1,3 @@ +import { b } from 'req-mjs'; + +console.log(b); diff --git a/test/test-54-esm-exports-conditions/app/node_modules/esm-only/index.mjs b/test/test-54-esm-exports-conditions/app/node_modules/esm-only/index.mjs new file mode 100644 index 00000000..2bf789c1 --- /dev/null +++ b/test/test-54-esm-exports-conditions/app/node_modules/esm-only/index.mjs @@ -0,0 +1 @@ +export const a = 'esm-only ok'; diff --git a/test/test-54-esm-exports-conditions/app/node_modules/esm-only/package.json b/test/test-54-esm-exports-conditions/app/node_modules/esm-only/package.json new file mode 100644 index 00000000..8163067d --- /dev/null +++ b/test/test-54-esm-exports-conditions/app/node_modules/esm-only/package.json @@ -0,0 +1,9 @@ +{ + "name": "esm-only", + "version": "1.0.0", + "exports": { + ".": { + "import": "./index.mjs" + } + } +} diff --git a/test/test-54-esm-exports-conditions/app/node_modules/req-mjs/index.mjs b/test/test-54-esm-exports-conditions/app/node_modules/req-mjs/index.mjs new file mode 100644 index 00000000..25be604d --- /dev/null +++ b/test/test-54-esm-exports-conditions/app/node_modules/req-mjs/index.mjs @@ -0,0 +1 @@ +export const b = 'req-mjs ok'; diff --git a/test/test-54-esm-exports-conditions/app/node_modules/req-mjs/package.json b/test/test-54-esm-exports-conditions/app/node_modules/req-mjs/package.json new file mode 100644 index 00000000..323f4f37 --- /dev/null +++ b/test/test-54-esm-exports-conditions/app/node_modules/req-mjs/package.json @@ -0,0 +1,10 @@ +{ + "name": "req-mjs", + "version": "1.0.0", + "exports": { + ".": { + "require": "./index.mjs", + "import": "./index.mjs" + } + } +} diff --git a/test/test-54-esm-exports-conditions/main.js b/test/test-54-esm-exports-conditions/main.js new file mode 100644 index 00000000..9bd0c1a5 --- /dev/null +++ b/test/test-54-esm-exports-conditions/main.js @@ -0,0 +1,59 @@ +#!/usr/bin/env node + +'use strict'; + +// Regression test for https://github.com/yao-pkg/pkg/issues/281 +// +// A packaged ESM app crashes at startup when it imports a dependency whose +// package.json "exports" map is valid for `import` but not resolvable through +// CommonJS `require()`. pkg transforms ESM to CJS and renames .mjs -> .js, but +// Node's CJS resolver always prefers "exports" over "main", so without +// rewriting the "exports" field the binary fails at runtime with either: +// - case A (import-only): ERR_PACKAGE_PATH_NOT_EXPORTED +// - case B (.mjs targets): MODULE_NOT_FOUND (.../index.mjs) + +const path = require('path'); +const assert = require('assert'); +const utils = require('../utils.js'); + +assert(!module.parent); +assert(__dirname === process.cwd()); + +const target = process.argv[2] || 'host'; + +const cases = [ + { id: 'a', entry: './app/app-a.mjs', expected: 'esm-only ok' }, + { id: 'b', entry: './app/app-b.mjs', expected: 'req-mjs ok' }, +]; + +for (const testCase of cases) { + console.log(`Testing ESM exports conditions (case ${testCase.id})...`); + + const output = `./run-time/test-output-${testCase.id}.exe`; + utils.mkdirp.sync(path.dirname(output)); + + // Expected output from plain node. + const left = utils.spawn.sync('node', [path.basename(testCase.entry)], { + cwd: path.dirname(testCase.entry), + }); + assert.strictEqual(left.trim(), testCase.expected); + + // Package with pkg. + utils.pkg.sync(['--target', target, '--output', output, testCase.entry], { + stdio: 'inherit', + }); + + // The packaged binary must run and produce the same output (no startup crash). + const right = utils.spawn.sync('./' + path.basename(output), [], { + cwd: path.dirname(output), + }); + assert.strictEqual( + left.trim(), + right.trim(), + `Outputs should match between node and pkg for case ${testCase.id}`, + ); + + console.log(`Test passed: case ${testCase.id}`); +} + +utils.vacuum.sync('./run-time'); diff --git a/test/unit/esm-transformer.test.ts b/test/unit/esm-transformer.test.ts index 2325f916..e0354ffc 100644 --- a/test/unit/esm-transformer.test.ts +++ b/test/unit/esm-transformer.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { + normalizeExportsForCJS, rewriteMjsRequirePaths, transformESMtoCJS, } from '../../lib/esm-transformer'; @@ -147,3 +148,69 @@ describe('transformESMtoCJS', () => { assert.equal(res.code, 'let x = ;'); }); }); + +describe('normalizeExportsForCJS', () => { + it('rewrites a string target .mjs to .js', () => { + assert.equal(normalizeExportsForCJS('./index.mjs'), './index.js'); + }); + + it('adds a require condition for an import-only exports map (#281 case A)', () => { + // esm-only: { exports: { ".": { import: "./index.mjs" } } } + // Without a CJS-resolvable condition, require() throws + // ERR_PACKAGE_PATH_NOT_EXPORTED in the packaged binary. + assert.deepEqual( + normalizeExportsForCJS({ '.': { import: './index.mjs' } }), + { + '.': { require: './index.js', import: './index.js' }, + }, + ); + }); + + it('rewrites .mjs require targets that no longer exist (#281 case B)', () => { + // req-mjs: { exports: { ".": { require: "./index.mjs", import: "./index.mjs" } } } + // The .mjs file is renamed to .js in the snapshot, so the target must follow. + assert.deepEqual( + normalizeExportsForCJS({ + '.': { require: './index.mjs', import: './index.mjs' }, + }), + { '.': { require: './index.js', import: './index.js' } }, + ); + }); + + it('falls back to the default condition when there is no import', () => { + assert.deepEqual( + normalizeExportsForCJS({ '.': { default: './index.mjs' } }), + { + '.': { default: './index.js' }, + }, + ); + }); + + it('does not add require when a CJS-resolvable condition already exists', () => { + // `default` is resolvable by require(), so the map is left as-is (extensions aside). + assert.deepEqual( + normalizeExportsForCJS({ import: './e.mjs', default: './d.mjs' }), + { import: './e.js', default: './d.js' }, + ); + }); + + it('recurses through subpath maps and nested conditions', () => { + assert.deepEqual( + normalizeExportsForCJS({ + '.': { import: './i.mjs' }, + './sub': { node: { import: './sub.mjs' } }, + }), + { + '.': { require: './i.js', import: './i.js' }, + './sub': { node: { require: './sub.js', import: './sub.js' } }, + }, + ); + }); + + it('handles array targets', () => { + assert.deepEqual(normalizeExportsForCJS(['./a.mjs', './b.mjs']), [ + './a.js', + './b.js', + ]); + }); +});