v2 externalization treats ESM-looking CommonJS comments as executable syntax
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 11.2k
- Forks
- 899
- Avg merge
- 2d 24m
- Merged PRs (30d)
- 40
Description
Environment
- Nitro 2.13.4 (
v2ate6ed0aa35a94e873e2e42487aec149238846225a) - mlly 1.8.2; Node 24.15.0; Linux
Minimal reproduction
Create a Node dependency named comment-syntax with package.json:
{ "name": "comment-syntax", "version": "1.0.0", "main": "index.js" }
Its index.js is valid CommonJS:
/* export default example */
module.exports = 42;
Import that dependency from a Nitro server route. Both the default and experimental.legacyExternals resolvers decline to externalize it because isValidNodeImport() counts the comment as ESM syntax by default. Direct Node loading succeeds.
The underlying validator behavior can be checked independently:
import { createRequire } from "node:module";
import { isValidNodeImport } from "mlly";
const require = createRequire(import.meta.url);
const entry = require.resolve("comment-syntax");
console.log(await isValidNodeImport(entry)); // false
console.log(await isValidNodeImport(entry, { stripComments: true })); // true
Expected behavior
Node externalization should examine executable syntax, ignoring comments while preserving explicit inlining and rejection of real mixed CommonJS/ESM code.
Proposed fix and TypeScript dependency
Pass { stripComments: true } in both v2 externals implementations. Regression tests exercise real CommonJS files with the real validator, preserving explicit inline rules and real mixed-syntax bundling. The two comment cases fail before this change and pass afterward.
This is one part of the TypeScript server-import failure reported in analogjs/analog#2457, currently worked around in analogjs/analog#2548. TypeScript 6.0.3 additionally contains ESM-looking diagnostic strings and identifier prefixes. Those require the separate mlly fix tracked in unjs/mlly#369.
The Nitro change alone is not claimed to fix TypeScript. Combined qualification uses a locally packed mlly build; the final TypeScript fix needs a released mlly version with token-aware detection. Nitro v3 is being qualified separately.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the two v2 externals implementations that call mlly's isValidNodeImport(), then inspect the regression tests for real CommonJS files and comment cases. Verify that comments are ignored while explicit inline rules and genuine mixed CommonJS/ESM behavior remain unchanged, and run the relevant externalization test suite.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- backend, build-system, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 72/100