-
-
Notifications
You must be signed in to change notification settings - Fork 626
fix(commonjs): recognize named export #1962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,80 @@ | ||||||||||
| import { init, parse } from 'cjs-module-lexer'; | ||||||||||
|
|
||||||||||
| let initialized = false; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Ensure cjs-module-lexer WASM is initialized. | ||||||||||
| * Safe to call multiple times — will only init once. | ||||||||||
| */ | ||||||||||
| export async function ensureInit() { | ||||||||||
| if (!initialized) { | ||||||||||
| await init(); | ||||||||||
| initialized = true; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Analyze a CommonJS module source to detect named exports. | ||||||||||
| * | ||||||||||
| * @param {string} code — The raw CJS source code. | ||||||||||
| * @param {string} id — The module ID (for error reporting). | ||||||||||
| * @returns {Promise<{ | ||||||||||
| * exports: string[] | ||||||||||
| * reexports: string[] | ||||||||||
| * hasDefaultExport: boolean | ||||||||||
| * }>} | ||||||||||
| */ | ||||||||||
| export async function analyzeExports(code, id) { | ||||||||||
| await ensureInit(); | ||||||||||
| try { | ||||||||||
|
Comment on lines
+28
to
+29
|
||||||||||
| await ensureInit(); | |
| try { | |
| try { | |
| await ensureInit(); |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
analyzeExports uses this.warn(...), but this function is not called with a Rollup plugin context (it’s invoked as analyzeExports(code, id)), so this will be undefined and this path will throw. Pass a warn callback/context into analyzeExports (or move the warning to the caller) and ensure the fallback return does not incorrectly set hasDefaultExport: true when lexing fails.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { createFilter } from '@rollup/pluginutils'; | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import { peerDependencies, version } from '../package.json'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import { analyzeExports, ensureInit as ensureLexerInit } from './analyze-exports-lexer'; | ||||||||||||||||||||||||
| import analyzeTopLevelStatements from './analyze-top-level-statements'; | ||||||||||||||||||||||||
| import { getDynamicModuleRegistry, getDynamicRequireModules } from './dynamic-modules'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -113,7 +114,7 @@ export default function commonjs(options = {}) { | |||||||||||||||||||||||
| // Initialized in buildStart | ||||||||||||||||||||||||
| let requireResolver; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function transformAndCheckExports(code, id) { | ||||||||||||||||||||||||
| async function transformAndCheckExports(code, id) { | ||||||||||||||||||||||||
| const normalizedId = normalizePathSlashes(id); | ||||||||||||||||||||||||
| const { isEsModule, hasDefaultExport, hasNamedExports, ast } = analyzeTopLevelStatements( | ||||||||||||||||||||||||
| this.parse, | ||||||||||||||||||||||||
|
|
@@ -138,6 +139,16 @@ export default function commonjs(options = {}) { | |||||||||||||||||||||||
| return { meta: { commonjs: commonjsMeta } }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Use cjs-module-lexer for named export detection on CJS modules | ||||||||||||||||||||||||
| if (!isEsModule) { | ||||||||||||||||||||||||
| const lexerResult = await analyzeExports(code, id); | ||||||||||||||||||||||||
| commonjsMeta.lexerExports = lexerResult.exports; | ||||||||||||||||||||||||
| commonjsMeta.lexerReexports = lexerResult.reexports; | ||||||||||||||||||||||||
| if (lexerResult.hasDefaultExport && !commonjsMeta.hasDefaultExport) { | ||||||||||||||||||||||||
| commonjsMeta.hasDefaultExport = true; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const needsRequireWrapper = | ||||||||||||||||||||||||
| !isEsModule && (dynamicRequireModules.has(normalizedId) || strictRequiresFilter(id)); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -202,8 +213,9 @@ export default function commonjs(options = {}) { | |||||||||||||||||||||||
| return { ...rawOptions, plugins }; | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| buildStart({ plugins }) { | ||||||||||||||||||||||||
| async buildStart({ plugins }) { | ||||||||||||||||||||||||
| validateVersion(this.meta.rollupVersion, peerDependencies.rollup, 'rollup'); | ||||||||||||||||||||||||
| await ensureLexerInit(); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| await ensureLexerInit(); | |
| try { | |
| await ensureLexerInit(); | |
| } catch (error) { | |
| this.warn({ | |
| code: 'CJS_LEXER_INIT_FAILED', | |
| message: | |
| 'Failed to initialize cjs-module-lexer. Falling back to AST-based export detection.' + | |
| (error && error.message ? ` Cause: ${error.message}` : '') | |
| }); | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| exports.$Enums = {}; | ||
| exports.AccessLevel = exports.$Enums.AccessLevel = { | ||
| TEST: 'TEST', | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -294,6 +294,15 @@ test('import CommonJS module with esm property should get default export ', asyn | |||||||||||||||||||||||||||||||||
| const result2 = await executeBundle(bundle2, avaAssertions); | ||||||||||||||||||||||||||||||||||
| expect(result2.error.message).toBe('libExports is not a function'); | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| test('test named exports from cjs', async (t) => { | ||||||||||||||||||||||||||||||||||
| const bundle = await rollup({ | ||||||||||||||||||||||||||||||||||
| input: 'fixtures/samples/named-cjs-exports/main.cjs', | ||||||||||||||||||||||||||||||||||
| plugins: [commonjs()] | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+297
to
+301
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| t.plan(3); | ||||||||||||||||||||||||||||||||||
| await testBundle(t, bundle); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+297
to
+304
|
||||||||||||||||||||||||||||||||||
| test('test named exports from cjs', async (t) => { | |
| const bundle = await rollup({ | |
| input: 'fixtures/samples/named-cjs-exports/main.cjs', | |
| plugins: [commonjs()] | |
| }); | |
| t.plan(3); | |
| await testBundle(t, bundle); | |
| test('test named exports from cjs', async () => { | |
| const bundle = await rollup({ | |
| input: 'fixtures/samples/named-cjs-exports/main.cjs', | |
| plugins: [commonjs()] | |
| }); | |
| expect.assertions(3); | |
| await testBundle(avaAssertions, bundle); |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ensureInitis not concurrency-safe: if two async callers hit it beforeinitializedflips totrue,init()can run multiple times. Consider caching the in-flight init promise (e.g.let initPromise) or setting the flag before awaiting to ensure only one initialization occurs per process.