fix: align @gsd/native module type with compiled output (#3253)
* fix: align @gsd/native module type with compiled output (#2861) The package declared "type": "module" and used "import"-only export conditions, but the addon loader used import.meta.url which is incompatible when the parent package enforces ESM resolution on Node.js v24. Switch to "type": "commonjs" with "default" export conditions and remove the import.meta.url/__dirname shim (CJS provides both natively). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: restore dual CJS/ESM compat for native addon loader The ESM-to-CJS conversion removed import.meta.url polyfills, but the CI test loader (dist-redirect.mjs) transpiles this file to ESM via ts.transpileModule — making __dirname and require unavailable at test time. Add runtime typeof guards that use the CJS globals when available (compiled output) and fall back to import.meta.url in ESM (test runner). Use @ts-expect-error to suppress TS1470 for the import.meta branches that are unreachable in the compiled CJS output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use indirect eval for import.meta.url to avoid CJS parse-time error import.meta is a parse-time syntax error in CJS — typeof guards don't help because Node.js rejects the syntax before executing any code. Wrapping in new Function("return import.meta.url") hides the syntax from the CJS parser while still working when executed as ESM (test runner). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: replace new Function(import.meta.url) with loader-injected CJS globals import.meta is static syntax unavailable in new Function() and eval() scopes, causing rtk-portability CI failures across all platforms. Instead of trying to access import.meta.url indirectly, the test loader (dist-redirect.mjs) now injects __dirname, __filename, and require as a preamble when transpiling workspace packages to ESM. This lets native.ts use __dirname/require directly in both CJS (production) and ESM (CI test) contexts without any import.meta.url fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
GitHub
parent
8917cc75be
commit
abd9be24e9
4 changed files with 139 additions and 28 deletions
|
|
@ -2,7 +2,7 @@
|
|||
"name": "@gsd/native",
|
||||
"version": "0.1.0",
|
||||
"description": "Native Rust bindings for GSD \u2014 high-performance native modules via N-API",
|
||||
"type": "module",
|
||||
"type": "commonjs",
|
||||
"main": "./dist/index.js",
|
||||
"types": "./dist/index.d.ts",
|
||||
"scripts": {
|
||||
|
|
@ -14,75 +14,75 @@
|
|||
"exports": {
|
||||
".": {
|
||||
"types": "./dist/index.d.ts",
|
||||
"import": "./dist/index.js"
|
||||
"default": "./dist/index.js"
|
||||
},
|
||||
"./grep": {
|
||||
"types": "./dist/grep/index.d.ts",
|
||||
"import": "./dist/grep/index.js"
|
||||
"default": "./dist/grep/index.js"
|
||||
},
|
||||
"./ps": {
|
||||
"types": "./dist/ps/index.d.ts",
|
||||
"import": "./dist/ps/index.js"
|
||||
"default": "./dist/ps/index.js"
|
||||
},
|
||||
"./glob": {
|
||||
"types": "./dist/glob/index.d.ts",
|
||||
"import": "./dist/glob/index.js"
|
||||
"default": "./dist/glob/index.js"
|
||||
},
|
||||
"./clipboard": {
|
||||
"types": "./dist/clipboard/index.d.ts",
|
||||
"import": "./dist/clipboard/index.js"
|
||||
"default": "./dist/clipboard/index.js"
|
||||
},
|
||||
"./ast": {
|
||||
"types": "./dist/ast/index.d.ts",
|
||||
"import": "./dist/ast/index.js"
|
||||
"default": "./dist/ast/index.js"
|
||||
},
|
||||
"./html": {
|
||||
"types": "./dist/html/index.d.ts",
|
||||
"import": "./dist/html/index.js"
|
||||
"default": "./dist/html/index.js"
|
||||
},
|
||||
"./text": {
|
||||
"types": "./dist/text/index.d.ts",
|
||||
"import": "./dist/text/index.js"
|
||||
"default": "./dist/text/index.js"
|
||||
},
|
||||
"./fd": {
|
||||
"types": "./dist/fd/index.d.ts",
|
||||
"import": "./dist/fd/index.js"
|
||||
"default": "./dist/fd/index.js"
|
||||
},
|
||||
"./image": {
|
||||
"types": "./dist/image/index.d.ts",
|
||||
"import": "./dist/image/index.js"
|
||||
"default": "./dist/image/index.js"
|
||||
},
|
||||
"./xxhash": {
|
||||
"types": "./dist/xxhash/index.d.ts",
|
||||
"import": "./dist/xxhash/index.js"
|
||||
"default": "./dist/xxhash/index.js"
|
||||
},
|
||||
"./diff": {
|
||||
"types": "./dist/diff/index.d.ts",
|
||||
"import": "./dist/diff/index.js"
|
||||
"default": "./dist/diff/index.js"
|
||||
},
|
||||
"./gsd-parser": {
|
||||
"types": "./dist/gsd-parser/index.d.ts",
|
||||
"import": "./dist/gsd-parser/index.js"
|
||||
"default": "./dist/gsd-parser/index.js"
|
||||
},
|
||||
"./highlight": {
|
||||
"types": "./dist/highlight/index.d.ts",
|
||||
"import": "./dist/highlight/index.js"
|
||||
"default": "./dist/highlight/index.js"
|
||||
},
|
||||
"./json-parse": {
|
||||
"types": "./dist/json-parse/index.d.ts",
|
||||
"import": "./dist/json-parse/index.js"
|
||||
"default": "./dist/json-parse/index.js"
|
||||
},
|
||||
"./stream-process": {
|
||||
"types": "./dist/stream-process/index.d.ts",
|
||||
"import": "./dist/stream-process/index.js"
|
||||
"default": "./dist/stream-process/index.js"
|
||||
},
|
||||
"./truncate": {
|
||||
"types": "./dist/truncate/index.d.ts",
|
||||
"import": "./dist/truncate/index.js"
|
||||
"default": "./dist/truncate/index.js"
|
||||
},
|
||||
"./ttsr": {
|
||||
"types": "./dist/ttsr/index.d.ts",
|
||||
"import": "./dist/ttsr/index.js"
|
||||
"default": "./dist/ttsr/index.js"
|
||||
}
|
||||
},
|
||||
"files": [
|
||||
|
|
|
|||
91
packages/native/src/__tests__/module-compat.test.mjs
Normal file
91
packages/native/src/__tests__/module-compat.test.mjs
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
/**
|
||||
* Tests that the @gsd/native package.json is correctly configured
|
||||
* for Node.js module resolution (ESM/CJS compatibility).
|
||||
*
|
||||
* Regression test for #2861: "type": "module" + "import"-only export
|
||||
* conditions caused crashes on Node.js v24 when the parent package also
|
||||
* declared "type": "module" and strict ESM resolution was enforced.
|
||||
*/
|
||||
|
||||
import { test, describe } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
const pkgPath = path.resolve(__dirname, "..", "..", "package.json");
|
||||
const pkg = JSON.parse(readFileSync(pkgPath, "utf8"));
|
||||
|
||||
describe("@gsd/native module compatibility (#2861)", () => {
|
||||
test("package.json must not declare type: module (compiled output is CJS-compatible)", () => {
|
||||
// The compiled output uses createRequire() to load .node addons.
|
||||
// Declaring "type": "module" forces Node.js to treat .js files as ESM,
|
||||
// but the package needs "type": "commonjs" to override the parent
|
||||
// package's "type": "module" and ensure correct CJS semantics.
|
||||
assert.notEqual(
|
||||
pkg.type,
|
||||
"module",
|
||||
'package.json must not set "type": "module" — this causes crashes on Node.js v24 ' +
|
||||
"when the parent package also declares ESM (see #2861)",
|
||||
);
|
||||
});
|
||||
|
||||
test("package.json should explicitly declare type: commonjs", () => {
|
||||
// When installed as a dependency under a parent with "type": "module"
|
||||
// (e.g. gsd-pi), an absent "type" field would inherit the parent's
|
||||
// ESM setting. Explicit "commonjs" overrides this.
|
||||
assert.equal(
|
||||
pkg.type,
|
||||
"commonjs",
|
||||
'package.json must explicitly set "type": "commonjs" to override ' +
|
||||
"the parent package's ESM declaration",
|
||||
);
|
||||
});
|
||||
|
||||
test("all export conditions must use 'default' (not 'import'-only)", () => {
|
||||
// The "import" condition key restricts resolution to ESM import
|
||||
// statements only. Using "default" ensures the export works for both
|
||||
// require() and import, which is essential for a CJS package that may
|
||||
// be consumed from ESM code via Node's CJS interop.
|
||||
const exportsMap = pkg.exports;
|
||||
assert.ok(exportsMap, "package.json must have an exports map");
|
||||
|
||||
for (const [subpath, conditions] of Object.entries(exportsMap)) {
|
||||
assert.ok(
|
||||
!conditions.import || conditions.default,
|
||||
`exports["${subpath}"] uses "import" condition without "default" — ` +
|
||||
`this breaks CJS consumers and Node.js v24 strict resolution`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("native.ts source must not use bare import.meta.url (parse-time error in CJS)", () => {
|
||||
// When compiled to CJS, import.meta is a *parse-time* syntax error --
|
||||
// typeof guards don't help because Node rejects the syntax before
|
||||
// executing any code. The source must wrap import.meta access in
|
||||
// an indirect eval so the CJS parser never sees the bare syntax.
|
||||
const nativeSrc = readFileSync(
|
||||
path.resolve(__dirname, "..", "native.ts"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
// Bare import.meta.url (NOT wrapped) would crash at parse time in CJS.
|
||||
// These regexes match direct usage like fileURLToPath(import.meta.url)
|
||||
// and createRequire(import.meta.url), but NOT indirect patterns that
|
||||
// hide import.meta from the CJS parser.
|
||||
const hasBareImportMetaDirname = /path\.dirname\(.*fileURLToPath\(import\.meta\.url\)\)/.test(nativeSrc);
|
||||
const hasBareImportMetaRequire = /createRequire\(import\.meta\.url\)/.test(nativeSrc);
|
||||
|
||||
assert.ok(
|
||||
!hasBareImportMetaDirname,
|
||||
"native.ts must not use bare import.meta.url in fileURLToPath() -- " +
|
||||
"this is a parse-time syntax error in CJS; use indirect eval",
|
||||
);
|
||||
assert.ok(
|
||||
!hasBareImportMetaRequire,
|
||||
"native.ts must not use bare import.meta.url in createRequire() -- " +
|
||||
"this is a parse-time syntax error in CJS; use indirect eval",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -8,14 +8,15 @@
|
|||
* 3. native/addon/gsd_engine.dev.node (local debug build)
|
||||
*/
|
||||
|
||||
import { createRequire } from "node:module";
|
||||
import * as path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
const require = createRequire(import.meta.url);
|
||||
// __dirname and require are available in both execution contexts:
|
||||
// - CJS (production build via tsc): provided natively by Node
|
||||
// - ESM (CI test loader): injected by the dist-redirect.mjs preamble
|
||||
const _dirname = __dirname;
|
||||
const _require = require;
|
||||
|
||||
const addonDir = path.resolve(__dirname, "..", "..", "..", "native", "addon");
|
||||
const addonDir = path.resolve(_dirname, "..", "..", "..", "native", "addon");
|
||||
const platformTag = `${process.platform}-${process.arch}`;
|
||||
|
||||
/** Map Node.js platform/arch to the npm package suffix */
|
||||
|
|
@ -36,7 +37,7 @@ function loadNative(): Record<string, unknown> {
|
|||
const packageSuffix = platformPackageMap[platformTag];
|
||||
if (packageSuffix) {
|
||||
try {
|
||||
_loadedSuccessfully = true; return require(`@gsd-build/engine-${packageSuffix}`) as Record<string, unknown>;
|
||||
_loadedSuccessfully = true; return _require(`@gsd-build/engine-${packageSuffix}`) as Record<string, unknown>;
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
errors.push(`@gsd-build/engine-${packageSuffix}: ${message}`);
|
||||
|
|
@ -46,7 +47,7 @@ function loadNative(): Record<string, unknown> {
|
|||
// 2. Try local release build (native/addon/gsd_engine.{platform}.node)
|
||||
const releasePath = path.join(addonDir, `gsd_engine.${platformTag}.node`);
|
||||
try {
|
||||
_loadedSuccessfully = true; return require(releasePath) as Record<string, unknown>;
|
||||
_loadedSuccessfully = true; return _require(releasePath) as Record<string, unknown>;
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
errors.push(`${releasePath}: ${message}`);
|
||||
|
|
@ -55,7 +56,7 @@ function loadNative(): Record<string, unknown> {
|
|||
// 3. Try local dev build (native/addon/gsd_engine.dev.node)
|
||||
const devPath = path.join(addonDir, "gsd_engine.dev.node");
|
||||
try {
|
||||
_loadedSuccessfully = true; return require(devPath) as Record<string, unknown>;
|
||||
_loadedSuccessfully = true; return _require(devPath) as Record<string, unknown>;
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
errors.push(`${devPath}: ${message}`);
|
||||
|
|
|
|||
|
|
@ -87,7 +87,26 @@ export function load(url, context, nextLoad) {
|
|||
emitDecoratorMetadata: true,
|
||||
},
|
||||
});
|
||||
return { format: 'module', source: outputText, shortCircuit: true };
|
||||
// Inject CJS-compatible globals (__dirname, __filename, require) so that
|
||||
// workspace packages compiled as ESM can still use them. This avoids the
|
||||
// need for import.meta.url behind indirect invocation patterns that fail in
|
||||
// CJS and in dynamically-created scopes.
|
||||
// Only inject globals that the source file doesn't already declare itself.
|
||||
const preambleLines = [
|
||||
'import { fileURLToPath as __preamble_fUTP } from "node:url";',
|
||||
'import { dirname as __preamble_dn } from "node:path";',
|
||||
'import { createRequire as __preamble_cR } from "node:module";',
|
||||
];
|
||||
if (!outputText.includes('const __filename') && !outputText.includes('let __filename')) {
|
||||
preambleLines.push('const __filename = __preamble_fUTP(import.meta.url);');
|
||||
}
|
||||
if (!outputText.includes('const __dirname') && !outputText.includes('let __dirname')) {
|
||||
preambleLines.push('const __dirname = __preamble_dn(__preamble_fUTP(import.meta.url));');
|
||||
}
|
||||
if (!outputText.includes('const require') && !outputText.includes('let require')) {
|
||||
preambleLines.push('const require = __preamble_cR(import.meta.url);');
|
||||
}
|
||||
return { format: 'module', source: preambleLines.join('\n') + '\n' + outputText, shortCircuit: true };
|
||||
}
|
||||
return nextLoad(url, context);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue