fix: improve CJS-to-ESM transform for better npm compatibility#3697
Merged
Conversation
…bility Fixes several issues in the CommonJS Babel transform that caused npm packages to break during builds while working in dev mode: - Fix .mts/.cts extension detection (`.cts` was in the ESM branch) - Handle `require.resolve()` by injecting `createRequire` polyfill - Polyfill `__dirname` and `__filename` CJS globals in transformed modules - Use `var` instead of `const` for `_default` to avoid TDZ errors in Rollup bundles - Guard namespace property spreading against primitive default exports - Track `module.exports.X` patterns as named exports (not just `exports.X`) Addresses: #3492, #3619, #3653, #3449, #3505 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
NOTE: When I tried to migrate Vite to v8 and rollup to rolldown for |
Contributor
Author
|
Closing in favor of #3734 |
3 tasks
# Conflicts: # packages/plugin-vite/src/plugins/patches/commonjs.ts # packages/plugin-vite/src/plugins/patches/commonjs_test.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju
changed the title
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Summary
Fixes several issues in the Fresh Vite plugin's CommonJS-to-ESM Babel transform (
commonjs.ts) that caused npm packages to break during builds while working fine in dev mode..mts/.ctsextension detection —.ctswas incorrectly in the ESM branch (should be.mts), causing TypeScript CJS files to skip the transformrequire.resolve()— injectscreateRequire(import.meta.url)polyfill sorequire.resolve()calls work in ESM output (require or exports is not defined #3619)__dirname/__filename— injectsfileURLToPath/dirnamebased polyfill when these CJS globals are usedvarinstead ofletfor_default— avoids TDZ errors when Rollup reorders declarations in bundled output (CJS dependencies cause fatal issues when building/running that go unnoticed in dev mode #3653)module.exports.Xas named exports — the MemberExpression visitor now handles bothexports.Xandmodule.exports.XpatternsAddresses: #3619, #3653
Test plan
commonjs_test.tstests pass (42 existing + 7 new)require.resolveinjectscreateRequirerequire.resolve+require()combined (single injection).mtsfiles treated as ESM.ctsfiles treated as CJS__dirname/__filenamepolyfill injectionmodule.exports.Xtracked as named exportmodule.exportswith namespace re-export guardNote on remaining CJS issues
This PR does not fix CJS issues caused by
require()-to-importhoisting changing package.json conditional export resolution (#3505, #3478, #3449). Those need resolver-level changes. See #3593 for native addon externalization (also out of scope).🤖 Generated with Claude Code