Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 7, 2025

Problem

The cjsModuleRunnerPlugin was incorrectly transforming CommonJS require() calls by always accessing .default on the imported module:

// Before: Broken transformation
const x = (await import("some-package")).default;

This assumption fails when Vite's import analysis resolves the import to an ESM variant of a package. For example, when importing @swc/helpers/_/_interop_require_default, Vite may resolve it to @swc/helpers/esm/_interop_require_default.js. Pure ESM modules with only named exports don't have a .default property, causing .default to return undefined and breaking code that depends on these imports.

This issue was discovered when trying to load Next.js CommonJS modules that import from packages like @swc/helpers, resulting in errors like:

Cannot read properties of undefined (reading '_')

Solution

Introduced a runtime interop helper __cjs_interop__ that uses an explicit marker-based approach to distinguish transformed CJS modules from genuine ESM modules:

function __cjs_interop__(m) { 
  return m.__cjs_module_runner_transform ? m.default : m; 
}

All modules transformed by cjsModuleRunnerPlugin now export an explicit marker:

export const __cjs_module_runner_transform = true;

The helper only unwraps .default when the __cjs_module_runner_transform marker is present, ensuring we don't incorrectly unwrap .default on genuine ESM modules. This explicit approach is safer and more predictable than heuristic-based detection.

// After: Correct transformation
const x = __cjs_interop__(await import("some-package"));

Changes

  • Updated transformCjsToEsm() to inject the interop helper and wrap all dynamic imports
  • Added __cjs_module_runner_transform marker export to all transformed CJS modules
  • Works for both top-level and hoisted require() calls
  • Helper is only injected when require() calls are present in the code
  • Updated test snapshots to reflect new transformation output

Testing

  • ✅ All 6 unit tests pass with updated snapshots
  • ✅ All 107 e2e tests pass
  • ✅ Verified the fix handles CJS modules and doesn't incorrectly transform genuine ESM modules

Fixes #904

Original prompt

Fix @vitejs/vite-plugin-react/issues/904


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

- Add __cjs_interop__ runtime helper to handle ESM modules correctly
- Check for __esModule flag and default export before accessing .default
- Update all tests to reflect new transformation output
- Fixes issue where require("esm-package") would fail when transformed

Co-authored-by: hi-ogawa <[email protected]>
@Copilot Copilot AI changed the title [WIP] Fix @vitejs/vite-plugin-react/issues/904 fix(plugin-rsc): add CJS/ESM interop helper for require() transformation Oct 7, 2025
@Copilot Copilot AI requested a review from hi-ogawa October 7, 2025 13:44
Copilot finished work on behalf of hi-ogawa October 7, 2025 13:44
Copy link
Contributor

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot What about #904 (comment)?

  • Or we check during runtime checkIfItShouldPickDefaultOrNot(await import("...")) based on some heuristics.

I think this works. Let's do:

  • inject some marker export like export const __cjs_module_runner_transform = true for modules transformed by cjsModuleRunnerPlugin.
  • then checkIfItShouldPickDefaultOrNot can unwrap default only when __cjs_module_runner_transform exists.

…rop helper

- Change interop helper to check for __cjs_module_runner_transform marker
- Add marker export to all transformed CJS modules
- Update tests to include marker in transformed modules
- More explicit approach than heuristic-based detection

Co-authored-by: hi-ogawa <[email protected]>
Copy link
Contributor Author

Copilot AI commented Oct 7, 2025

@copilot What about #904 (comment)?

  • Or we check during runtime checkIfItShouldPickDefaultOrNot(await import("...")) based on so...

Updated the implementation to use the explicit marker approach. The interop helper now checks for __cjs_module_runner_transform marker instead of heuristics. All transformed CJS modules export this marker, and the helper only unwraps .default when the marker is present. (commit 5f18644 -> current)

@Copilot Copilot AI requested a review from hi-ogawa October 7, 2025 13:58
Copilot finished work on behalf of hi-ogawa October 7, 2025 13:58
@hi-ogawa hi-ogawa changed the title fix(plugin-rsc): add CJS/ESM interop helper for require() transformation fix(rsc/cjs): unwrap default based on __cjs_module_runner_transform marker Oct 7, 2025
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.

cjsModuleRunnerPlugin transforms cjs with require("some-package") incorrectly when "some-package" exports esm
2 participants