From a082a1b8726c428f58f13362980717a5456e0dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 21:55:54 +0200 Subject: [PATCH] fix(hir): late-bind `new X()` to a class declared later; name the ReferenceError Coop's Next.js App Route fixture died at module init on 0.5.1519 with the nameless `ReferenceError: identifier is not defined`. The identifier is `SentinelNode` in next/dist/server/lib/lru-cache.js: the CJS wrap hoists `LRUCache` out of the module IIFE but never sees `SentinelNode` (its doc comment closes on the `class` line, and the textual hoister anchors on `class ` at column 0), so the hoisted constructor's `new SentinelNode()` is lowered before the `__perry_cjs_factory` body registers the class. The unresolved-`new` guard from #8643 (905017b1c, inside the 1516..1519 window) turned that lowering-time miss into an unconditional nameless throw; before it, the by-name `Expr::New` bound at codegen through the module class table, which is why 0.5.1516 loaded. - `pre_scan_class_decl_names` records every class DECLARATION name in the module at any depth; the guard keeps the late-bound by-name construction for those. - Any other unresolved constructor is read off `globalThis` when the `new` executes (`js_global_get_or_throw_unresolved`, shared with the bare-identifier arm via `unresolved_global_get_expr`), so a runtime-created global constructs and a true miss throws `ReferenceError: is not defined` -- with the identifier, as #8730 and #8882 asked. The compile log names it too, with the same "unknown identifier" warning the bare-identifier arm prints. Regression tests: a hoisted class constructing a sibling declared inside a later closure keeps `New { class_name }` (fails without the new guard clause, verified); a `typeof`-guarded `new IntersectionObserver()` lowers to the named runtime lookup; the #8739 positive control now expects the named form. Fixes #8882. Refs #8730. Claude-Session: https://claude.ai/code/session_01UZJbhb2FTuakurTHPAKQgd --- Cargo.lock | 1 + changelog.d/8882-late-bound-class-new.md | 1 + crates/perry-hir/Cargo.toml | 1 + crates/perry-hir/src/lower/context.rs | 1 + crates/perry-hir/src/lower/expr_new.rs | 31 ++++++- crates/perry-hir/src/lower/lower_expr.rs | 4 +- .../src/lower/lower_expr/arm_ident.rs | 18 ++-- .../perry-hir/src/lower/lower_expr/helpers.rs | 23 +++++ crates/perry-hir/src/lower/lower_module_fn.rs | 4 + .../perry-hir/src/lower/lowering_context.rs | 9 ++ crates/perry-hir/src/lower/mod.rs | 4 +- crates/perry-hir/src/lower/pre_scan.rs | 2 + .../src/lower/pre_scan/class_decl_names.rs | 44 ++++++++++ crates/perry-hir/src/lower/tests.rs | 86 +++++++++++++++++++ .../tests/aliased_native_new_resolution.rs | 14 ++- 15 files changed, 223 insertions(+), 20 deletions(-) create mode 100644 changelog.d/8882-late-bound-class-new.md create mode 100644 crates/perry-hir/src/lower/pre_scan/class_decl_names.rs diff --git a/Cargo.lock b/Cargo.lock index e25c5c14ec..32127e3e0a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6291,6 +6291,7 @@ dependencies = [ "stacker", "swc_common", "swc_ecma_ast", + "swc_ecma_visit", "thiserror 1.0.69", ] diff --git a/changelog.d/8882-late-bound-class-new.md b/changelog.d/8882-late-bound-class-new.md new file mode 100644 index 0000000000..f1c9e318fb --- /dev/null +++ b/changelog.d/8882-late-bound-class-new.md @@ -0,0 +1 @@ +Fix a `new ()` whose constructor is not statically resolvable at lowering time throwing a nameless `ReferenceError: identifier is not defined` at module init (#8882, the second instance of #8730's class). The unresolved-`new` guard added in #8643 decided the miss at compile time, but JS binds the constructor reference when the `new` executes, and two shapes are invisible to the lowering-time lookups: a class declared in a function body lowered LATER, and a global that exists only at runtime. The CJS wrap produces the first one routinely — it hoists top-level classes out of the module IIFE but leaves some inside (here Next's `server/lib/lru-cache.js` `SentinelNode`, whose doc comment closes on the `class` line so the textual hoister never sees it), so the hoisted `LRUCache` constructor's `new SentinelNode()` was lowered before the IIFE body registered `SentinelNode`, and the whole Next.js App Route application died at init under Coop. A module-wide pre-scan now records every class declaration name at any depth; a name in that set keeps the late-bound by-name construction codegen resolves through the module class table (the pre-#8643 behaviour), and any other name is read off `globalThis` when the `new` runs via `js_global_get_or_throw_unresolved`, so a runtime-created constructor works and a true miss throws the spec `ReferenceError: is not defined` — with the identifier, as #8730 asked. The bare-identifier arm shares the same helper so the two cannot drift. diff --git a/crates/perry-hir/Cargo.toml b/crates/perry-hir/Cargo.toml index 08f72b1e49..b505dab798 100644 --- a/crates/perry-hir/Cargo.toml +++ b/crates/perry-hir/Cargo.toml @@ -20,6 +20,7 @@ perry-ui-model.workspace = true perry-parser.workspace = true swc_ecma_ast.workspace = true swc_common.workspace = true +swc_ecma_visit.workspace = true thiserror.workspace = true anyhow.workspace = true diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 9c1d6d559e..e55402add8 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -205,6 +205,7 @@ impl LoweringContext { class_renames: std::collections::HashMap::new(), next_class_rename_id: 0, module_class_decl_names: std::collections::HashSet::new(), + class_decl_names_any_depth: std::collections::HashSet::new(), next_anon_shape_id: 0, class_method_return_types: Vec::new(), class_captures: Vec::new(), diff --git a/crates/perry-hir/src/lower/expr_new.rs b/crates/perry-hir/src/lower/expr_new.rs index 65c5014dc5..8f361db3b3 100644 --- a/crates/perry-hir/src/lower/expr_new.rs +++ b/crates/perry-hir/src/lower/expr_new.rs @@ -1572,6 +1572,24 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R // (`new Missing()`), distinct from the TypeError produced when a // present binding's value is non-constructable. // + // #8882: the failure must be decided when the `new` EXECUTES, not + // here. Two shapes the lowering-time lookups cannot see: (1) a + // class declared in a function body that is lowered LATER — the + // CJS wrap leaves some top-level classes inside the module IIFE + // while hoisting their siblings, so a hoisted `LRUCache` + // constructor's `new SentinelNode()` (Next's `lru-cache.js`) is + // lowered before the `__perry_cjs_factory` body registers + // `SentinelNode`; (2) a global that exists only at runtime + // (`typeof IntersectionObserver === "function" && new + // IntersectionObserver(…)`). The #8643 guard lowered both to an + // unconditional, NAMELESS `ReferenceError: identifier is not + // defined`, which killed the whole application at init. Now a + // name declared as a class anywhere in the module keeps the + // late-bound by-name `Expr::New` below (codegen resolves it + // through the module class table, as before #8643), and any other + // name is read off `globalThis` when the `new` runs, throwing the + // spec `ReferenceError: is not defined` on a true miss. + // // Consult the native-module registry under BOTH the (possibly // rewritten) `class_name` AND the original `source_class_name`. // The alias-rewrite block just above replaces `class_name` with a @@ -1592,11 +1610,20 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R && ctx.lookup_native_module(&class_name).is_none() && ctx.lookup_native_module(source_class_name).is_none() && !ctx.forward_class_names.contains(source_class_name) + && !ctx.class_decl_names_any_depth.contains(source_class_name) && !is_reified_global_builtin_constructor(&class_name) { + // Same wording as the bare-identifier arm so one grep over the + // compile log lists every name that will be resolved at + // runtime — #8882 could not be attributed from the log because + // the `new` path never said which identifier it gave up on. + eprintln!( + " Warning: unknown identifier '{source_class_name}' — assuming global; `new {source_class_name}()` resolves it by name on globalThis at runtime (ReferenceError on a miss)" + ); return Ok(Expr::NewDynamic { - callee: Box::new(super::throw_reference_error_expr( - "js_throw_reference_error_unresolved_get", + callee: Box::new(super::unresolved_global_get_expr( + source_class_name.to_string(), + new_byte_offset, )), args, byte_offset: new_byte_offset, diff --git a/crates/perry-hir/src/lower/lower_expr.rs b/crates/perry-hir/src/lower/lower_expr.rs index b2bbdf6ad8..76acf0aa96 100644 --- a/crates/perry-hir/src/lower/lower_expr.rs +++ b/crates/perry-hir/src/lower/lower_expr.rs @@ -42,8 +42,8 @@ pub(crate) use helpers::{ global_script_this_enabled, is_fetch_global_value_name, is_known_global_identifier_name, lower_expr_with_json_parse_type_hint, native_module_binding_value, opt_call_func_nullish_guard, opt_call_receiver_repeatable, relower_trace, strict_global_assign_existing_or_throw, - throw_reference_error_expr, with_implicit_unset_let, with_set_fallback_for_ident, - wrap_with_gets, + throw_reference_error_expr, unresolved_global_get_expr, with_implicit_unset_let, + with_set_fallback_for_ident, wrap_with_gets, }; pub(crate) use reactive_text::try_desugar_reactive_text; diff --git a/crates/perry-hir/src/lower/lower_expr/arm_ident.rs b/crates/perry-hir/src/lower/lower_expr/arm_ident.rs index 6dcec4ec7f..1d0819a152 100644 --- a/crates/perry-hir/src/lower/lower_expr/arm_ident.rs +++ b/crates/perry-hir/src/lower/lower_expr/arm_ident.rs @@ -288,18 +288,12 @@ pub(crate) fn lower_ident_expr(ctx: &mut LoweringContext, ident: &ast::Ident) -> name ); } - return Ok(Expr::Call { - callee: Box::new(Expr::ExternFuncRef { - name: "js_global_get_or_throw_unresolved".to_string(), - param_types: vec![Type::Any], - return_type: Type::Any, - }), - args: vec![Expr::String(name.clone())], - type_args: Vec::new(), - // #5253: localize the `X is not defined` ReferenceError to - // this identifier's source position (winston `module`). - byte_offset: ident.span.lo.0, - }); + // #5253: localize the `X is not defined` ReferenceError to + // this identifier's source position (winston `module`). + return Ok(super::helpers::unresolved_global_get_expr( + name.clone(), + ident.span.lo.0, + )); } // Bare built-in constructor identifiers (`Date`, `Array`, // `Object`, ...) used as VALUES (not method receivers / diff --git a/crates/perry-hir/src/lower/lower_expr/helpers.rs b/crates/perry-hir/src/lower/lower_expr/helpers.rs index 5a95f9ef15..db8b60f754 100644 --- a/crates/perry-hir/src/lower/lower_expr/helpers.rs +++ b/crates/perry-hir/src/lower/lower_expr/helpers.rs @@ -44,6 +44,29 @@ pub(crate) fn throw_reference_error_expr(helper_name: &str) -> Expr { } } +/// Read a compile-time-unresolved identifier off `globalThis` at runtime, +/// throwing the spec `ReferenceError: is not defined` when no such +/// global exists (`js_global_get_or_throw_unresolved`). A global created at +/// RUNTIME (`Function("this.y = 2")()`, a `typeof IntersectionObserver === +/// "function"`-guarded browser API) is invisible to compile-time resolution, +/// so the miss must be decided when the read executes — and the message +/// must carry the identifier (#8730, #8882). `byte_offset` localizes the +/// error to the identifier's source position (#5253). Shared by the bare +/// identifier arm (`arm_ident.rs`) and `lower_new`'s unresolved-constructor +/// fallback so the two cannot drift. +pub(crate) fn unresolved_global_get_expr(name: String, byte_offset: u32) -> Expr { + Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: "js_global_get_or_throw_unresolved".to_string(), + param_types: vec![Type::Any], + return_type: Type::Any, + }), + args: vec![Expr::String(name)], + type_args: Vec::new(), + byte_offset, + } +} + /// #5989: lower a strict-mode assignment to an identifier with no lexical /// binding. Per spec (PutValue on a reference that resolves to the global /// environment), an EXISTING global property is a normal property write — diff --git a/crates/perry-hir/src/lower/lower_module_fn.rs b/crates/perry-hir/src/lower/lower_module_fn.rs index c7a1c4fd82..718e1d30a5 100644 --- a/crates/perry-hir/src/lower/lower_module_fn.rs +++ b/crates/perry-hir/src/lower/lower_module_fn.rs @@ -908,6 +908,10 @@ pub fn lower_module_full( // literals, and counter vars (see `fn_ctor_env`). ctx.fn_ctor_env = super::fn_ctor_env::build_fn_ctor_env(ast_module); + // #8882: every class DECLARATION name at any depth, for `lower_new`'s + // unresolved-constructor guard (see `pre_scan/class_decl_names.rs`). + pre_scan_class_decl_names(ast_module, &mut ctx); + // Pre-scan for WeakRef/FinalizationRegistry variable declarations so subsequent // method-call lowering (`x.deref()`, `x.register(...)`, `x.unregister(...)`) can // route via the dedicated HIR variants without relying on type inference. diff --git a/crates/perry-hir/src/lower/lowering_context.rs b/crates/perry-hir/src/lower/lowering_context.rs index 8edf6ba83e..f2da7d7f38 100644 --- a/crates/perry-hir/src/lower/lowering_context.rs +++ b/crates/perry-hir/src/lower/lowering_context.rs @@ -816,6 +816,15 @@ pub struct LoweringContext { /// `lower_class_from_ast` detect that collision and allocate a fresh, /// uniquely-named class instead. pub(crate) module_class_decl_names: std::collections::HashSet, + /// #8882: names of `class X { … }` DECLARATIONS anywhere in the module, + /// at any nesting depth (populated by `pre_scan_class_decl_names`). + /// Consulted by `lower_new`'s unresolved-constructor guard: a name that + /// is declared as a class somewhere in the module — typically inside a + /// function body lowered later, such as the CJS wrap's IIFE — keeps the + /// late-bound by-name construction instead of a compile-time + /// `ReferenceError`. Unlike `module_class_decl_names` this is NOT limited + /// to top level and is never used for ClassId (re)allocation. + pub(crate) class_decl_names_any_depth: std::collections::HashSet, /// Counter for generating anon-class names (`__AnonShape_N`). // #854: initialized in `new` but unread — anon-shape classes are now named // by content-addressed FNV hash (see `synthesize_anon_shape_class`), not by diff --git a/crates/perry-hir/src/lower/mod.rs b/crates/perry-hir/src/lower/mod.rs index d6956269a5..6ce65947ac 100644 --- a/crates/perry-hir/src/lower/mod.rs +++ b/crates/perry-hir/src/lower/mod.rs @@ -116,8 +116,8 @@ pub use lower_module_fn::{ mod lower_expr; pub(crate) use lower_expr::{ lower_expr, lower_expr_assignment, strict_global_assign_existing_or_throw, - throw_reference_error_expr, try_desugar_reactive_text, with_implicit_unset_let, - with_set_fallback_for_ident, + throw_reference_error_expr, try_desugar_reactive_text, unresolved_global_get_expr, + with_implicit_unset_let, with_set_fallback_for_ident, }; // Re-export extracted module functions diff --git a/crates/perry-hir/src/lower/pre_scan.rs b/crates/perry-hir/src/lower/pre_scan.rs index fc3335e171..ff378daba8 100644 --- a/crates/perry-hir/src/lower/pre_scan.rs +++ b/crates/perry-hir/src/lower/pre_scan.rs @@ -10,8 +10,10 @@ use swc_ecma_ast as ast; use super::*; use crate::ir::*; +mod class_decl_names; mod weakref_locals; +pub(crate) use class_decl_names::pre_scan_class_decl_names; pub(crate) use weakref_locals::pre_scan_weakref_locals; /// Pre-scan top-level function declarations for the standard TypeScript diff --git a/crates/perry-hir/src/lower/pre_scan/class_decl_names.rs b/crates/perry-hir/src/lower/pre_scan/class_decl_names.rs new file mode 100644 index 0000000000..a61637558d --- /dev/null +++ b/crates/perry-hir/src/lower/pre_scan/class_decl_names.rs @@ -0,0 +1,44 @@ +//! Pre-scan for every `class X { … }` DECLARATION name in the module, at any +//! nesting depth. +//! +//! #8882: `lower_new`'s unresolved-constructor guard (#8643) decides at +//! lowering time whether `new X()` can bind at all. Its lookups only see +//! bindings registered so far, but a class declared inside a function body +//! that is lowered LATER is still a legitimate late-bound target — JS +//! resolves the constructor reference when the `new` executes, not when the +//! enclosing method is compiled. The CJS wrap makes this shape common: it +//! hoists most top-level classes out of the module IIFE but leaves some +//! inside (a class it did not recognise textually, or one that reads an +//! IIFE-local), so a hoisted class's constructor can `new` a sibling that is +//! now nested in the `__perry_cjs_factory` closure and registered only when +//! that closure body is lowered. Next's `server/lib/lru-cache.js` has exactly +//! this: `LRUCache` (hoisted) constructs `SentinelNode` (left in the IIFE +//! because its doc comment closes on the `class` line). +//! +//! The guard consults this set: a name declared as a class anywhere in the +//! module keeps the by-name `Expr::New` lowering that codegen resolves through +//! the module class table (the pre-#8643 behaviour); anything else is a +//! runtime `globalThis` lookup that throws the spec `ReferenceError: X is not +//! defined`. Only DECLARATIONS count — a named class EXPRESSION's name binds +//! inside its own body alone. + +use swc_ecma_ast as ast; +use swc_ecma_visit::{Visit, VisitWith}; + +use crate::lower::*; + +pub(crate) fn pre_scan_class_decl_names(ast_module: &ast::Module, ctx: &mut LoweringContext) { + struct Collector<'a> { + names: &'a mut std::collections::HashSet, + } + impl Visit for Collector<'_> { + fn visit_class_decl(&mut self, class_decl: &ast::ClassDecl) { + self.names.insert(class_decl.ident.sym.to_string()); + class_decl.visit_children_with(self); + } + } + let mut collector = Collector { + names: &mut ctx.class_decl_names_any_depth, + }; + ast_module.visit_with(&mut collector); +} diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 3ac5e84935..89f113ae34 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1525,3 +1525,89 @@ fn typescript_transpile_subset_lowers_to_native_dispatch_and_enums() { "diagnostic flattening must use TypeScript native dispatch: {dump}" ); } + +/// #8882: a module-level class constructing a sibling class that is declared +/// inside a function body lowered LATER. This is the shape the CJS wrap +/// produces for Next's `server/lib/lru-cache.js`: `LRUCache` is hoisted out of +/// the module IIFE while `SentinelNode` (whose doc comment closes on the +/// `class` line, so the textual hoister never sees it) stays inside the +/// `__perry_cjs_factory` closure. JS binds the constructor reference when the +/// `new` executes; the #8643 guard instead lowered it to an unconditional, +/// nameless `ReferenceError` that killed the application at init. +#[test] +fn hoisted_class_constructs_sibling_declared_inside_a_later_closure() { + let source = r#" + class LRUCache { + constructor() { + this.head = new SentinelNode(); + this.tail = new SentinelNode(); + } + } + const _cjs = (function () { + class SentinelNode { + constructor() { + this.prev = null; + this.next = null; + } + } + return { SentinelNode }; + })(); + "#; + let module = perry_parser::parse_typescript(source, "lru-cache.js").expect("source parses"); + let hir = super::lower_module(&module, "lru-cache", "lru-cache.js").expect("source lowers"); + let lru_cache = hir + .classes + .iter() + .find(|class| class.name == "LRUCache") + .expect("LRUCache class is lowered"); + let debug = format!("{lru_cache:?}"); + + assert!( + !debug.contains("js_throw_reference_error_unresolved_get") + && !debug.contains("js_global_get_or_throw_unresolved"), + "a sibling class declared later in the module must not lower to a \ + compile-time ReferenceError:\n{debug}" + ); + assert_eq!( + debug.matches(r#"New { class_name: "SentinelNode""#).count(), + 2, + "both `new SentinelNode()` sites must stay late-bound by-name constructs:\n{debug}" + ); +} + +/// #8882 / #8730: a constructor name that resolves to nothing in the module +/// is read off `globalThis` when the `new` executes — exactly like a bare +/// identifier read — so a runtime-created global constructs and a true miss +/// throws `ReferenceError: is not defined` WITH the identifier. The +/// `typeof`-guarded browser-API shape is the one Next's `app-page` runtime +/// carries; it previously lowered to the nameless throw even though the guard +/// makes the branch dead on a server. +#[test] +fn unresolved_new_names_the_identifier_and_defers_to_a_runtime_global_lookup() { + let source = r#" + function observe(cb: any): any { + return typeof IntersectionObserver === "function" + ? new IntersectionObserver(cb) + : null; + } + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + let observe = hir + .functions + .iter() + .find(|function| function.name == "observe") + .expect("observe is lowered"); + let debug = format!("{observe:?}"); + + assert!( + !debug.contains("js_throw_reference_error_unresolved_get"), + "the nameless ReferenceError helper must not be emitted for `new ()`:\n{debug}" + ); + assert!( + debug.contains( + r#"NewDynamic { callee: Call { callee: ExternFuncRef { name: "js_global_get_or_throw_unresolved", param_types: [Any], return_type: Any }, args: [String("IntersectionObserver")]"# + ), + "an unresolved constructor must be a runtime globalThis lookup carrying its name:\n{debug}" + ); +} diff --git a/crates/perry-hir/tests/aliased_native_new_resolution.rs b/crates/perry-hir/tests/aliased_native_new_resolution.rs index 1705e79353..fe6e879533 100644 --- a/crates/perry-hir/tests/aliased_native_new_resolution.rs +++ b/crates/perry-hir/tests/aliased_native_new_resolution.rs @@ -17,6 +17,7 @@ use perry_hir::lower_module; use perry_parser::parse_typescript_with_cache; const THROW_HELPER: &str = "js_throw_reference_error_unresolved_get"; +const GLOBAL_LOOKUP_HELPER: &str = "js_global_get_or_throw_unresolved"; fn lower_debug(src: &str) -> String { let src = src.to_string(); @@ -92,12 +93,21 @@ fn unaliased_native_class_import_still_constructs() { fn genuinely_unresolved_new_still_throws() { // Positive control: the guard must still fire for a `new` on an identifier // that resolves to no binding at all — the fix must not blanket-suppress it. + // #8882: it now defers to a runtime `globalThis` lookup that carries the + // identifier (`ReferenceError: Totally_Undefined_Constructor_Xyz is not + // defined` on a miss) instead of the nameless throw. let debug = lower_debug(r#"const x = new Totally_Undefined_Constructor_Xyz();"#); assert!( - debug.contains(THROW_HELPER), - "a genuinely unresolved `new` must still lower to the nameless \ + !debug.contains(THROW_HELPER), + "a genuinely unresolved `new` must no longer lower to the nameless \ ReferenceError throw:\n{debug}" ); + assert!( + debug.contains(GLOBAL_LOOKUP_HELPER) + && debug.contains("\"Totally_Undefined_Constructor_Xyz\""), + "a genuinely unresolved `new` must lower to a named runtime global \ + lookup:\n{debug}" + ); } #[test]