Skip to content

fix: CJS detection regression for packages with "export" in comments#3793

Merged
bartlomieju merged 2 commits intomainfrom
fix/cjs-detection-regression
Apr 28, 2026
Merged

fix: CJS detection regression for packages with "export" in comments#3793
bartlomieju merged 2 commits intomainfrom
fix/cjs-detection-regression

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

@bartlomieju bartlomieju commented Apr 27, 2026

Closes #3794

Fixes a regression from #3767 where CJS npm packages fail with exports is not defined in dev mode.

  • The old heuristic (code.includes("export ")) was fooled by the word "export" appearing in comments of CJS files — e.g. @opentelemetry/api has // TODO: Remove ProxyTracerProvider export in the next major version. which caused the CJS shim to be skipped entirely
  • Replaces the naive string check with two-layer detection:
    1. Package.json type field (Node.js semantics): .cjs → always CJS, .js → CJS unless nearest package.json has "type": "module". Results cached per directory.
    2. ESM statement regex fallback: for dual CJS/ESM packages that ship ESM in .js without "type": "module" (e.g. @opentelemetry/api, preact-render-to-string), detects actual ESM syntax including minified forms like import{ and export*from
  • Adds a test importing from a real CJS npm package (qs) in node_modules — the existing "CJS test" fixtures were local ESM files that never exercised the shim path

The content-based heuristic (`code.includes("export ")`) was fooled by
CJS files containing the word "export" in comments — e.g.
`@opentelemetry/api`'s CJS entry has a comment:
  // TODO: Remove ProxyTracerProvider export in the next major version.

This caused the CJS shim to be skipped, leaving `exports` undefined
when Vite's ESModulesEvaluator ran the file.

Replace the naive string check with two-layer detection:

1. Package.json "type" field (Node.js semantics): .cjs is always CJS,
   .js is CJS unless the nearest package.json has "type": "module".
   Results are cached per directory.

2. ESM statement regex fallback: for dual CJS/ESM packages that ship
   ESM in .js without "type": "module" (e.g. @opentelemetry/api),
   detect actual ESM syntax (export/import statements) including
   minified forms like `import{` and `export*from`.

Also adds a test that imports from a real CJS npm package (qs) in
node_modules, since the existing CJS test fixtures were local ESM
files that never exercised the shim path.
@bartlomieju bartlomieju merged commit 838f41a into main Apr 28, 2026
9 checks passed
@bartlomieju bartlomieju deleted the fix/cjs-detection-regression branch April 28, 2026 07:02
bartlomieju added a commit that referenced this pull request Apr 28, 2026
## Summary

- Bump `@fresh/core` 2.3.1 → 2.3.2
- Bump `@fresh/plugin-vite` 1.1.0 → 1.1.1
- Bump `@fresh/init` 2.3.1 → 2.3.2
- Bump `@fresh/update` 2.3.1 → 2.3.2

Patch release for the CJS detection regression fix (#3793).
bartlomieju added a commit that referenced this pull request Apr 28, 2026
## Summary

Reverts two commits that broke CJS package handling in dev mode:

- Reverts #3793 (CJS detection regex fix)
- Reverts #3767 (removal of Babel CJS→ESM transform)

The simplified CJS shim introduced in #3767 has two fundamental issues:
1. **Detection**: the heuristic (`code.includes("export ")`) is fooled
by comments containing "export"
2. **Named exports**: the shim only provides `export default
module.exports`, so `import { trace } from "@opentelemetry/api"` returns
`undefined` (#3797)

The Babel-based transform handled both correctly. Bringing it back until
we can replace it with a proper solution using `deno_ast`'s CJS parser.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[vite] Internal server error: exports is not defined

1 participant