fix(stub): handle arbitrary module namespace identifier names (#533)#563
fix(stub): handle arbitrary module namespace identifier names (#533)#563guoyangzhen wants to merge 1 commit intounjs:mainfrom
Conversation
Modules using arbitrary module namespace identifiers (e.g. export const 'module.exports') generated invalid stub files like: export const module.exports = _module.module.exports; Now uses bracket notation for non-standard identifiers: export const "module.exports" = _module["module.exports"]; Fixes unjs#533
📝 WalkthroughWalkthroughThe PR fixes stub file generation for modules with non-identifier export names. Previously, all exports used dot notation which generated invalid syntax for names like Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/builders/rollup/stub.ts`:
- Around line 161-169: The current code emits invalid ESM like export const
"module.exports" = ..., so change the mapping to create a safe local identifier
for non-identifier names (e.g. __export_<hash>), emit a const binding from
_module (const __export_x = _module[<stringified name>];) and then re-export it
using an export clause that supports string-literal export names (export {
__export_x as <stringified name> };), using the existing name/_module values to
build the two statements; update the map block that handles name to produce
either the simple export const for valid identifiers or the two-line local
binding + export { ... as ... } for arbitrary names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 915ed282-c3af-4c98-bb6c-ddaa9c8cb48c
📒 Files selected for processing (1)
src/builders/rollup/stub.ts
| .map((name) => { | ||
| // Check if name is a valid JS identifier | ||
| // Arbitrary module namespace identifiers (e.g. 'module.exports') | ||
| // need bracket notation for member access | ||
| if (/^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(name)) { | ||
| return `export const ${name} = _module.${name};`; | ||
| } | ||
| return `export const ${JSON.stringify(name)} = _module[${JSON.stringify(name)}];`; | ||
| }), |
There was a problem hiding this comment.
export const ${JSON.stringify(name)} generates invalid ESM syntax for arbitrary export names.
Line 168 emits export const "module.exports" = ..., which is a parse error and still breaks stub generation for the target case.
💡 Proposed fix
...namedExports
.filter((name) => name !== "default")
- .map((name) => {
+ .flatMap((name, i) => {
// Check if name is a valid JS identifier
// Arbitrary module namespace identifiers (e.g. 'module.exports')
// need bracket notation for member access
if (/^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(name)) {
- return `export const ${name} = _module.${name};`;
+ return [`export const ${name} = _module.${name};`];
}
- return `export const ${JSON.stringify(name)} = _module[${JSON.stringify(name)}];`;
+ const key = JSON.stringify(name);
+ const local = `_reExport_${i}`;
+ return [
+ `const ${local} = _module[${key}];`,
+ `export { ${local} as ${key} };`,
+ ];
}),Is `export const "module.exports" = _module["module.exports"];` valid JavaScript (ESM) syntax?
What is the correct ESM syntax to export a string-literal export name (e.g. `"module.exports"`) from a local binding?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/builders/rollup/stub.ts` around lines 161 - 169, The current code emits
invalid ESM like export const "module.exports" = ..., so change the mapping to
create a safe local identifier for non-identifier names (e.g. __export_<hash>),
emit a const binding from _module (const __export_x = _module[<stringified
name>];) and then re-export it using an export clause that supports
string-literal export names (export { __export_x as <stringified name> };),
using the existing name/_module values to build the two statements; update the
map block that handles name to produce either the simple export const for valid
identifiers or the two-line local binding + export { ... as ... } for arbitrary
names.
Fixes #533
Problem
Modules using arbitrary module namespace identifiers (e.g.
export const 'module.exports') generated invalid stub files:Fix
Check if the export name is a valid JS identifier. If not, use bracket notation with
JSON.stringify()for proper escaping:This supports the
require(esm)feature where modules export'module.exports'as an arbitrary identifier name.Summary by CodeRabbit