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
28 changes: 28 additions & 0 deletions crates/perry-hir/src/lower_decl/body_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,34 @@ pub fn lower_body_stmt(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result<Ve
}
}
ctx.pending_classes.push(class);
// #5251 follow-up — a function-nested `class X { … }` whose
// name collides with an OUTER same-named local must SHADOW
// that local within this scope, exactly as a nested
// `function X(){}` does (see `lower_nested_fn_decl`, which
// `define_local`s the function name + emits a `Stmt::Let`).
// Without a scope-local binding, a later in-scope reference to
// `X` (e.g. the cjs_wrap `exports.X = X` that records the
// module's named export) falls through `lookup_local` to the
// enclosing module-scope `const X = _cjs.X` re-export binding —
// a circular self-read that resolves to `undefined`. That is
// the class-vs-function export asymmetry that left
// `require('./code').Name` undefined for ajv's `codegen/code.js`
// (`class Name`) while an identically-shaped `function Name`
// exported fine. Bind the name to a `ClassRef` value (the same
// shape `var C = class {…}` lowers to, recorded by codegen in
// `local_class_aliases`) so the in-scope read resolves to the
// class. Gated on a pre-existing outer binding so working
// packages (no collision) are byte-for-byte unaffected.
if ctx.lookup_local(&class_name).is_some() {
let class_local = ctx.define_local(class_name.clone(), Type::Any);
result.push(Stmt::Let {
id: class_local,
name: class_name.clone(),
ty: Type::Any,
init: Some(Expr::ClassRef(class_name.clone())),
mutable: false,
});
}
} else {
// Duplicate same-named class: still evaluate its computed
// member keys for their spec-mandated side effects. See
Expand Down
113 changes: 113 additions & 0 deletions tests/test_cjs_nested_class_named_export.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
#!/bin/bash
# Regression (fix/ajv-not-constructor): a compiled CJS package whose class is
# kept inside the cjs_wrap IIFE (#5251 hoist guard — the class body references
# the injected `exports`) must still surface its `exports.X = X` named export to
# `require('./code').X`. The HIR lowering for a function-nested `class X {}` now
# defines a scope-local binding shadowing the outer same-named re-export const,
# matching how a nested `function X(){}` already behaves.
#
# Pre-fix: `require('./code').Name` was `undefined` (the in-IIFE `exports.Name =
# Name` resolved `Name` to the circular module-scope `const Name = _cjs.Name`),
# so ajv's `new code_1.Name(...)` threw "undefined is not a constructor".
# The function variant in the identical shape always worked — this guards both.

set -e

SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
PERRY="$SCRIPT_DIR/../target/release/perry"
[ ! -f "$PERRY" ] && PERRY="$SCRIPT_DIR/../target/debug/perry"
if [ ! -f "$PERRY" ]; then
echo "SKIP: perry binary not found (build with cargo build --release)"
exit 0
fi
if ! command -v cc >/dev/null 2>&1; then
echo "SKIP: cc not available"
exit 0
fi

TMPDIR=$(mktemp -d)
trap "rm -rf $TMPDIR" EXIT

PKG="$TMPDIR/node_modules/pk"
mkdir -p "$PKG"

# code.js — the kept-in-IIFE shape: `class Name` whose ctor reads the injected
# `exports`, then `exports.Name = Name`. The #5251 guard keeps the class inside
# the IIFE; the named export must still reach the module's external exports.
cat > "$PKG/code.js" << 'EOF'
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Name = void 0;
exports.IDENTIFIER = /^[a-z]+$/i;
class Name { constructor(s){ if(!exports.IDENTIFIER.test(s)) throw new Error("bad"); this.str = s; } }
exports.Name = Name;
EOF

# func.js — the FUNCTION variant of the identical shape (the asymmetry to guard
# against re-breaking): a function kept in the IIFE exported the same way.
cat > "$PKG/func.js" << 'EOF'
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Make = void 0;
exports.PREFIX = "v";
function Make(s){ this.tag = exports.PREFIX + s; }
exports.Make = Make;
EOF

# scope.js — cross-module consumer that constructs the kept-in-IIFE class at
# module-init time (mirrors ajv's scope.js `new code_1.Name("const")`).
cat > "$PKG/scope.js" << 'EOF'
"use strict";
const code_1 = require("./code");
const func_1 = require("./func");
exports.varKinds = { const: new code_1.Name("const") };
exports.made = new func_1.Make("x");
EOF

cat > "$PKG/index.js" << 'EOF'
"use strict";
const code_1 = require("./code");
const func_1 = require("./func");
const scope_1 = require("./scope");
module.exports = function(){
return "Name=" + typeof code_1.Name
+ " Make=" + typeof func_1.Make
+ " kind=" + String(scope_1.varKinds.const)
+ " made=" + scope_1.made.tag;
};
EOF

cat > "$PKG/package.json" << 'EOF'
{ "name":"pk","version":"1.0.0","main":"index.js" }
EOF

cat > "$TMPDIR/package.json" << 'EOF'
{ "type":"module","private":true,"perry":{"compilePackages":["*"],"allow":{"compilePackages":["*"]}} }
EOF

cat > "$TMPDIR/main.ts" << 'EOF'
import pk from "pk";
console.log(pk());
EOF

cd "$TMPDIR"
COMPILE_OUTPUT=$(PERRY_NO_AUTO_OPTIMIZE=1 "$PERRY" main.ts -o test_bin --no-cache 2>&1) || {
echo "FAIL: compile error"
echo "$COMPILE_OUTPUT" | tail -20
exit 1
}

RUN_OUTPUT=$(./test_bin 2>&1)
# Matches `node main.ts`: the class & function both export as functions, and the
# cross-module `new code_1.Name(...)` / `new func_1.Make(...)` succeed.
EXPECTED="Name=function Make=function kind=[object Object] made=vx"

if [ "$RUN_OUTPUT" = "$EXPECTED" ]; then
echo "PASS"
exit 0
fi

echo "FAIL: kept-in-IIFE class named export not visible on require()"
echo "Expected: $EXPECTED"
echo "Got: $RUN_OUTPUT"
exit 1
Comment on lines +100 to +113

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 | 🟡 Minor | ⚡ Quick win

Guard runtime execution so failures still print diagnostics.

At Line 100, set -e can terminate the script before the Expected/Got failure output runs. Add explicit handling around ./test_bin so runtime regressions remain debuggable.

Proposed fix
-RUN_OUTPUT=$(./test_bin 2>&1)
+RUN_OUTPUT=$(./test_bin 2>&1) || {
+  echo "FAIL: runtime error"
+  echo "$RUN_OUTPUT" | tail -20
+  exit 1
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN_OUTPUT=$(./test_bin 2>&1)
# Matches `node main.ts`: the class & function both export as functions, and the
# cross-module `new code_1.Name(...)` / `new func_1.Make(...)` succeed.
EXPECTED="Name=function Make=function kind=[object Object] made=vx"
if [ "$RUN_OUTPUT" = "$EXPECTED" ]; then
echo "PASS"
exit 0
fi
echo "FAIL: kept-in-IIFE class named export not visible on require()"
echo "Expected: $EXPECTED"
echo "Got: $RUN_OUTPUT"
exit 1
RUN_OUTPUT=$(./test_bin 2>&1) || {
echo "FAIL: runtime error"
echo "$RUN_OUTPUT" | tail -20
exit 1
}
# Matches `node main.ts`: the class & function both export as functions, and the
# cross-module `new code_1.Name(...)` / `new func_1.Make(...)` succeed.
EXPECTED="Name=function Make=function kind=[object Object] made=vx"
if [ "$RUN_OUTPUT" = "$EXPECTED" ]; then
echo "PASS"
exit 0
fi
echo "FAIL: kept-in-IIFE class named export not visible on require()"
echo "Expected: $EXPECTED"
echo "Got: $RUN_OUTPUT"
exit 1
🤖 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 `@tests/test_cjs_nested_class_named_export.sh` around lines 100 - 113, The
script may exit prematurely due to `set -e` when `./test_bin` exits with a
non-zero status, preventing the failure diagnostics from being printed. Modify
the RUN_OUTPUT command execution to gracefully handle non-zero exit codes from
`./test_bin` so the script continues to execute and print the Expected/Got
output for debugging purposes. This can be done by either temporarily disabling
the exit-on-error behavior before running the test command, or by appending `||
true` to the command to suppress the exit code propagation.

Loading