Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions crates/perry/src/commands/compile/cjs_wrap/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ pub(in crate::commands::compile) fn is_commonjs(source: &str) -> bool {
}
if stripped.contains("module.exports")
|| stripped.contains("exports.")
// Issue #5275: bracket / computed-string-literal CJS export forms —
// `module['exports'] = …` / `module["exports"] = …` (default) and
// `exports['name'] = …` / `exports["name"] = …` (named). These are
// semantically identical to the dot forms (@colors/colors's
// `lib/custom/trap.js` does `module['exports'] = function runTheTrap`).
// Without recognizing them the file falls through to the ESM pipeline
// and the bare `module`/`exports` identifiers throw at module init.
//
// NOTE: `strip_comments_and_strings` blanks the `'exports'` STRING
// CONTENT (and its quotes) to spaces, leaving `module[ ]`, so
// we can't match the quoted token against `stripped`. Scan for the
// bracket-export shape on the original source via a regex that allows
// whitespace where the stripper would have written spaces. A genuinely
// dynamic `module[k]` (non-string-literal key) does NOT match.
|| has_bracket_cjs_export(source)
// Issue #4872: tsc-compiled type-only modules (nestjs dist
// `*.interface.js`) contain ONLY the interop marker
// `Object.defineProperty(exports, "__esModule", { value: true });`
Expand All @@ -57,6 +72,23 @@ pub(in crate::commands::compile) fn is_commonjs(source: &str) -> bool {
stripped.contains("require(") && !stripped.contains("import ")
}

/// Issue #5275: detect a bracket / computed-string-literal CJS export
/// assignment — `module['exports'] = …`, `module["exports"] = …`,
/// `exports['name'] = …`, `exports["name"] = …`, and the
/// `module.exports['name'] = …` variant. Requires the `=` (an assignment) so
/// a bare `module['exports']` read or a comment mention doesn't trip it, and
/// requires a string-literal key so a genuinely dynamic `module[k] = …` is
/// not matched. Matched on the ORIGINAL source because
/// `strip_comments_and_strings` blanks the quoted key.
fn has_bracket_cjs_export(source: &str) -> bool {
// `module['exports'] = …` / `module["exports"] = …` (default export).
let module_default = regex::Regex::new(r#"\bmodule\[\s*['"]exports['"]\s*\]\s*="#).unwrap();
// `exports['name'] = …` / `module.exports['name'] = …` (named export).
let named =
regex::Regex::new(r#"\bexports\[\s*['"][A-Za-z_$][A-Za-z0-9_$]*['"]\s*\]\s*="#).unwrap();
module_default.is_match(source) || named.is_match(source)
}
Comment on lines +83 to +90

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Bracket CJS detection can false-positive on comment/string text.

The new matcher runs on raw source, so text like // module['exports'] = x (or a string literal containing that snippet) can classify the file as CJS and force wrapping even when no real export assignment exists.

Please run the bracket matcher on a comment-stripped view (while preserving string-literal text needed for quoted-key matching), or add a lightweight lexical guard before accepting regex hits.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry/src/commands/compile/cjs_wrap/detect.rs` around lines 83 - 91,
The has_bracket_cjs_export function applies regex patterns directly to raw
source code, causing false positives when the patterns appear in comments or
strings. Modify the function to preprocess the source code to remove comments
(while preserving string literals for accurate quoted-key matching), or add a
lexical guard that validates regex matches are not within comment blocks before
returning true. This ensures that patterns like module['exports'] in comments
like // module['exports'] = x are not incorrectly classified as real CommonJS
exports.


/// Replace comment bodies and string/template-literal contents with spaces
/// so token scans (`require(`, `import `) only see real code. Same scanner
/// shape as `looks_like_es_module` in perry-parser, including the
Expand Down
27 changes: 26 additions & 1 deletion crates/perry/src/commands/compile/cjs_wrap/extract_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ use super::*;
/// literal, call, member expression, etc.) — those cases need the IIFE's
/// `module.exports` machinery to resolve correctly.
pub fn extract_single_module_exports_assignment(source: &str) -> Option<String> {
let re = regex::Regex::new(r#"(?m)^\s*module\.exports\s*=\s*([^;\n]+?)\s*;?\s*$"#).ok()?;
// Issue #5275: also accept the bracket/computed-string-literal form
// `module['exports'] = X` / `module["exports"] = X`, equivalent to the
// dot form. A genuinely dynamic `module[k] = X` (non-string-literal key)
// is NOT matched and stays on the runtime `_cjs` path.
let re = regex::Regex::new(
r#"(?m)^\s*module(?:\.exports|\[\s*'exports'\s*\]|\[\s*"exports"\s*\])\s*=\s*([^;\n]+?)\s*;?\s*$"#,
)
.ok()?;
let ident_re = regex::Regex::new(r#"^[A-Za-z_$][A-Za-z0-9_$]*$"#).ok()?;
let mut found: Option<String> = None;
for cap in re.captures_iter(source) {
Expand Down Expand Up @@ -372,6 +379,24 @@ pub fn extract_exports_from_source(source: &str) -> Vec<String> {
}
}

// Issue #5275: bracket / computed-string-literal named exports —
// `exports['name'] = …` / `exports["name"] = …` (and the
// `module.exports['name'] = …` variant). Equivalent to the dot form.
// The leading boundary class excludes `.` so `e.exports['X'] = …` (an
// inner webpack/ncc module's own exports param) is not mistaken for a
// named export of the outer bundle — mirroring the dot matcher above. A
// genuinely dynamic `exports[k] = …` (non-string-literal key) does not
// match and stays on the `_cjs` runtime path.
let bracket_re = regex::Regex::new(
r#"(?:^|[^A-Za-z0-9_$.])(?:module\.)?exports\[\s*['"]([A-Za-z_$][A-Za-z0-9_$]*)['"]\s*\]\s*="#,
)
.unwrap();
for cap in bracket_re.captures_iter(source) {
if let Some(m) = cap.get(1) {
push_unique(&mut names, m.as_str());
}
}

// Shape 2: `module.exports = { ... }` — extract every key from the
// object literal body. Brace-balanced scan because the body may contain
// nested braces (`module.exports = { fn: function() {} }`). Two key
Expand Down
79 changes: 79 additions & 0 deletions crates/perry/src/commands/compile/cjs_wrap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,85 @@ module.exports = inner;
assert_eq!(names, vec!["foo".to_string(), "bar".to_string()]);
}

#[test]
fn issue_5275_detects_bracket_module_exports() {
// @colors/colors/lib/custom/trap.js shape: bracket default export.
assert!(is_commonjs("module['exports'] = function runTheTrap() {};"));
assert!(is_commonjs(
"module[\"exports\"] = function runTheTrap() {};"
));
}

#[test]
fn issue_5275_detects_bracket_named_exports() {
assert!(is_commonjs("exports['foo'] = 1;"));
assert!(is_commonjs("exports[\"foo\"] = 1;"));
}

#[test]
fn issue_5275_dynamic_bracket_key_is_not_cjs_on_its_own() {
// A genuinely dynamic `module[k] = …` (non-literal key) is not a CJS
// export signal — without other CJS tokens this stays ESM.
assert!(!is_commonjs("const k = 'x';\nmodule[k] = 1;\n"));
}

#[test]
fn issue_5275_extracts_bracket_named_exports() {
let src = "exports['foo'] = 1;\nexports[\"bar\"] = function(){};";
let names = extract_exports_from_source(src);
assert_eq!(names, vec!["foo".to_string(), "bar".to_string()]);
}

#[test]
fn issue_5275_extracts_bracket_module_exports_dot_named() {
let src = "module.exports['foo'] = 1;";
let names = extract_exports_from_source(src);
assert_eq!(names, vec!["foo".to_string()]);
}

#[test]
fn issue_5275_does_not_extract_dynamic_bracket_key() {
// `exports[k] = …` with a non-string-literal key must not surface a
// named export.
let src = "const k = 'x';\nexports[k] = 1;";
let names = extract_exports_from_source(src);
assert!(names.is_empty(), "expected no names, got {:?}", names);
}

#[test]
fn issue_5275_single_module_exports_accepts_bracket_form() {
let src = "class Child {}\nmodule['exports'] = Child;";
assert_eq!(
extract_single_module_exports_assignment(src),
Some("Child".to_string())
);
let src2 = "class Child {}\nmodule[\"exports\"] = Child;";
assert_eq!(
extract_single_module_exports_assignment(src2),
Some("Child".to_string())
);
}

#[test]
fn issue_5275_wrap_default_export_for_bracket_module_exports() {
// The mb repro: `module['exports'] = function greet(){}`. The IIFE
// runs the bracket assignment, so `export default _cjs;` resolves to
// the function — but the file MUST be wrapped first (detection).
let src = "module['exports'] = function greet(n) { return n; };";
assert!(is_commonjs(src));
let wrapped = wrap_commonjs(src, &PathBuf::from("/tmp/mb/index.js"));
assert!(
wrapped.contains("export default _cjs;"),
"expected default export through _cjs, got:\n{}",
wrapped
);
assert!(
perry_parser::parse_typescript(&wrapped, "mb/index.js").is_ok(),
"wrapped bracket-export module must parse, got:\n{}",
wrapped
);
}

#[test]
fn extracts_module_exports_object_literal_shorthand() {
// Issue #624: `module.exports = { createContext }`
Expand Down
Loading