Repository navigation
Conversation
darcyrush
force-pushed
the
fix/type-only-commonjs
branch
from
November 6, 2026 16:28
9e55fb1 to
bda8cec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI generated, but ran the fix against a dummy repo locally.
Fixes #909
Summary
Type erasure removes
import typeand the other type-only declarations whole, so Node.js' own typestripping runs a file whose only module syntax they are as CommonJS. oxc-node ran it as an ES module,
without
require,moduleorexports."type": "commonjs"unless noted)import type+require, as the entry point,require()d, or imported from an.mtsfilerequire is not defined in ES module scopeexport type { … }(±from),export type *,export type X =,export interface,export default interface,export declare …"type"ReferenceErrorimport type+export =TS(1203);require():module is not defined in ES module scopeimport { type X }, a value export,import.meta, top-level await,export {}.mts,"type": "module"Fix
parsed_as_esmandhas_module_declarationno longer count animport/exportdeclaration thattype erasure removes whole (
is_type_only_module_declaration):import type,export type { … }with or without
from,export type *, and the export of a type alias, an interface or an ambientdeclaration.
import { type X } from "x"still counts, because it leavesimport {} from "x"behind,which is module syntax to Node.js.
oxc_transformcompiles such a file the way a.ctsfile is already compiled. The emptyexport {}the transform appends is dropped, and helpers arerequire()d. This also changes the publictransform()API, whose output for such a file no longer ends inexport {};.On the #759 test
This replaces the test
a file whose only module syntax is a type-only import runs as an ES modulefrom #759, which says:
That held while the output kept the injected
export {}. Compiled as a.ctsfile is, the output hasno module syntax left, so the CommonJS path runs it. The file then gets
require,moduleandexports. That matches what Node.js' type stripping andtscgive it, and a file written as CommonJSneeds them: today it fails with
require is not defined in ES module scope.A file with module syntax left after erasure still runs as an ES module, exactly as #759 made it. The
new guard tests pin that.
Tests
In
cjs-esm-syntax.spec.ts:"type": "commonjs"package, stays CommonJS as an entrypoint, when imported from an
.mtsfile, and whenrequire()d."type".import typebesideexport =stays CommonJS.import typebeside an inlinetypespecifier, a value export,import.metaor top-levelawait still runs as an ES module.
With the
src/lib.rschange reverted, the 5 new tests fail. The 4 guards pass either way.Verification
Node.js v26.12.1, linux-x64:
vp run test(599 passed, 8 skipped),cargo test --all-features,cargo clippy --all-targets --all-features -- -D warningsandcargo fmt --checkall pass. In adummy repo, the 7 type-only forms run as CommonJS, as
node file.tsruns them, and the 5 guards stillrun as ES modules.
Not covered
module: node16,node18ornodenext, the resolve hook reports every.tsfile as an ESmodule before the load hook sees it, so this fix needs fix(core): let package.json "type" decide .ts format under node* modules #908 there.
import { x } from "./file.ts") of such a file still fails withdoes not provide an export named, as it does on main. The load hook hands CommonJS files back toNode.js without a source, and
cjs-module-lexerthen reads the untransformed.ts, where theimport typeline stops it. A default import andrequire()work.export namespace N { export type T = number }) still counts asmodule syntax, although Node.js strips it.