You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In #3767 we attempted to replace the 960-line Babel CJS→ESM transform with a simpler approach: a content-based heuristic for CJS detection + a lightweight ESM shim (export default module.exports). This was reverted in #3798 because of two fundamental issues.
Problems with the simplified approach
1. CJS detection heuristic is fragile
The heuristic used code.includes("export ") to detect ESM files and skip CJS wrapping. This is fooled by comments — e.g. @opentelemetry/api's CJS entry has:
// TODO: Remove ProxyTracerProvider export in the next major version.
This caused the CJS shim to be skipped entirely, resulting in exports is not defined at runtime.
In #3793 we tried fixing this with a two-layer approach:
Check nearest package.json"type" field (Node.js semantics)
Fall back to a regex detecting actual ESM statements (export\s*[{*], import\s*[{*"'], etc.)
This only provides a default export. Named imports like import { trace } from "@opentelemetry/api" get undefined, causing #3797:
Cannot read properties of undefined (reading 'getTracer')
The old Babel transform properly converted exports.foo = ... and Object.defineProperty(exports, "foo", ...) patterns into ESM named exports. The simplified shim cannot do this without analyzing the CJS code structure.
Proposed solution
Replace the Babel CJS transform with deno_ast's CJS parser (or cjs-module-lexer which Node.js uses internally). This would:
Correctly detect CJS vs ESM — no heuristics needed, the parser knows
Extract named exports — the parser identifies all exports.xxx and Object.defineProperty(exports, "xxx", ...) patterns
Be much smaller than the Babel transform — purpose-built for this exact task
Match Node.js behavior — same approach Node.js uses for import { foo } from "cjs-package" interop
Context
In #3767 we attempted to replace the 960-line Babel CJS→ESM transform with a simpler approach: a content-based heuristic for CJS detection + a lightweight ESM shim (
export default module.exports). This was reverted in #3798 because of two fundamental issues.Problems with the simplified approach
1. CJS detection heuristic is fragile
The heuristic used
code.includes("export ")to detect ESM files and skip CJS wrapping. This is fooled by comments — e.g.@opentelemetry/api's CJS entry has:// TODO: Remove ProxyTracerProvider export in the next major version.This caused the CJS shim to be skipped entirely, resulting in
exports is not definedat runtime.In #3793 we tried fixing this with a two-layer approach:
package.json"type"field (Node.js semantics)export\s*[{*],import\s*[{*"'], etc.)This fixed detection but revealed problem 2.
2. The shim doesn't support named exports
The CJS shim wraps modules as:
This only provides a
defaultexport. Named imports likeimport { trace } from "@opentelemetry/api"getundefined, causing #3797:The old Babel transform properly converted
exports.foo = ...andObject.defineProperty(exports, "foo", ...)patterns into ESM named exports. The simplified shim cannot do this without analyzing the CJS code structure.Proposed solution
Replace the Babel CJS transform with
deno_ast's CJS parser (orcjs-module-lexerwhich Node.js uses internally). This would:exports.xxxandObject.defineProperty(exports, "xxx", ...)patternsimport { foo } from "cjs-package"interopWhat didn't work
code.includes("export ")package.jsontype + ESM regexTest coverage gaps found
commonjs_mod.js,maxmind.js) were changed to ESM in refactor: remove CJS and env var Babel transforms, let Vite handle natively #3767, so the "CJS tests" weren't testing CJS at allimport { foo }vsimport foo)node_modules(fixtures were local files that bypass thenode_modulesdetection path)@opentelemetry/apito its ESM entry, masking the CJS issue in CI