From 4e8e6b2b4751ac426dd53c1a36eae66e9ea21d36 Mon Sep 17 00:00:00 2001 From: Ashley Hunter Date: Thu, 8 Oct 2026 23:33:39 +0100 Subject: [PATCH 1/5] fix(hmr): write the new template's fields and stop redefining the live definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The update module `compileForHmrSync` returns replaced a component's template like this: X.ɵcmp = i0.ɵɵdefineComponent({ ...X.ɵcmp, ...(X.ɵcmp.inputConfig ? { inputs: X.ɵcmp.inputConfig } : {}), template: function X_Template(rf, ctx) { ... }, }); Three things were wrong with it. - `decls`, `vars` and `ngContentSelectors` were not written, so the spread kept the old template's. A swap that added an element or a binding rebuilt a view sized for the old template and failed partway ("Index expected to be less than 30", "TNodes should be created before any bindings"). `consts` was written only when the new template had some, so a template that lost its last attribute kept the old ones. - The live definition's `outputs` went back through `ɵɵdefineComponent`, which had already converted them to `{ publicName: propertyName }` and converted them again. A component with `checked = model(false)` ended up with `outputs: { checked: 'checkedChange' }`. - The live definition has no `changeDetection`, so `onPush` was derived again as true: an Eager component became OnPush after a swap. `inputs` was already restored from `inputConfig` for the second reason. Rather than undo each conversion, the module no longer calls `ɵɵdefineComponent`. Only the template is compiled on this path, so the rest of the definition is the live one, copied as it is: X.ɵcmp = { ...X.ɵcmp, decls: 4, vars: 1, consts: null, ngContentSelectors: _c1, template: function X_Template(rf, ctx) { ... }, tView: null, }; Every field the template decides is written, including the ones the new template has no value for. `tView` is cleared so the runtime builds the view for the new template. `ɵɵreplaceMetadata` then merges this into the live definition and re-creates the views, as before. - `compile_template_for_hmr` returns `decls`, `vars` and the `ngContentSelectors` expression alongside the template. - `HmrUpdateModuleOptions` takes them as `HmrTemplateFields`. - `generate_hmr_update_module_from_js` (`generateHmrModule`), which is given only template JavaScript, emits the same shape without the counts it cannot know; its docs say so. Angular's own update module is a whole new `ɵɵdefineComponent({...})` built from the component class. That needs the class, which this template-only path does not have, so the shape differs: the definition is not rebuilt and the component id does not change. Tests: - `test/hmr-update-module.test.ts`: 18 tests that compile a component, load it with the real `@angular/core`, apply the update module through `ɵɵreplaceMetadata`, and compare the template's fields with a fresh compile of the new template and every other field with what it was. `@angular/core` is a dev dependency for this. - `e2e/tests/hmr-update-definition.spec.ts`: 6 browser tests that edit templates on disk and check what renders: elements and bindings gained and lost, `` gained, changed and lost, consts, outputs still reaching the parent after one and two swaps, the change detection strategy, and a component whose path contains `@`. --- .../src/component/transform.rs | 23 +- crates/oxc_angular_compiler/src/hmr/mod.rs | 3 +- .../src/hmr/update_module.rs | 342 +++++++++++------- crates/oxc_angular_compiler/src/lib.rs | 6 +- napi/angular-compiler/.gitignore | 1 + .../e2e/app/src/app/@scoped/at.component.ts | 10 + .../e2e/app/src/app/@scoped/at.html | 1 + .../e2e/app/src/app/app.component.ts | 8 +- .../angular-compiler/e2e/app/src/app/app.html | 12 + .../e2e/app/src/app/io.component.ts | 22 ++ napi/angular-compiler/e2e/app/src/app/io.html | 1 + .../e2e/app/src/app/lab.component.ts | 15 + .../angular-compiler/e2e/app/src/app/lab.html | 1 + .../e2e/tests/hmr-input-component.spec.ts | 6 +- .../e2e/tests/hmr-update-definition.spec.ts | 234 ++++++++++++ napi/angular-compiler/package.json | 2 + napi/angular-compiler/src/lib.rs | 13 +- .../test/hmr-update-module.test.ts | 325 +++++++++++++++++ pnpm-lock.yaml | 6 + 19 files changed, 879 insertions(+), 152 deletions(-) create mode 100644 napi/angular-compiler/e2e/app/src/app/@scoped/at.component.ts create mode 100644 napi/angular-compiler/e2e/app/src/app/@scoped/at.html create mode 100644 napi/angular-compiler/e2e/app/src/app/io.component.ts create mode 100644 napi/angular-compiler/e2e/app/src/app/io.html create mode 100644 napi/angular-compiler/e2e/app/src/app/lab.component.ts create mode 100644 napi/angular-compiler/e2e/app/src/app/lab.html create mode 100644 napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts create mode 100644 napi/angular-compiler/test/hmr-update-module.test.ts diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 934427735..1a9157b57 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -418,6 +418,16 @@ pub struct HmrTemplateCompileOutput { /// constant references. Without this, the template may reference indices that don't /// exist in the old component definition's consts array. pub consts_js: Option, + + /// The number of element, text and container slots the template creates. + pub decls: u32, + + /// The number of binding slots the template uses. + pub vars: u32, + + /// The `ngContentSelectors` array as JavaScript code, when the template projects + /// content. It refers to a constant declared in `declarations_js`. + pub ng_content_selectors_js: Option, } /// Compiled component information. @@ -5604,7 +5614,18 @@ pub fn compile_template_for_hmr<'a>( None }; - Ok(HmrTemplateCompileOutput { template_js, declarations_js, styles, consts_js }) + let ng_content_selectors_js = + job.content_selectors.take().map(|selectors| emitter.emit_expression(&selectors)); + + Ok(HmrTemplateCompileOutput { + template_js, + declarations_js, + styles, + consts_js, + decls: job.root.decl_count.unwrap_or(0), + vars: job.root.vars.unwrap_or(0), + ng_content_selectors_js, + }) } /// Generate component compilation output for HMR. diff --git a/crates/oxc_angular_compiler/src/hmr/mod.rs b/crates/oxc_angular_compiler/src/hmr/mod.rs index c9629de93..621e0c2cd 100644 --- a/crates/oxc_angular_compiler/src/hmr/mod.rs +++ b/crates/oxc_angular_compiler/src/hmr/mod.rs @@ -46,5 +46,6 @@ pub use dependencies::{ pub use initializer::{HmrDefinition, compile_hmr_initializer, compile_hmr_update_callback}; pub use styles::generate_style_update_module; pub use update_module::{ - HmrUpdateModuleOptions, generate_hmr_update_module, generate_hmr_update_module_from_js, + HmrTemplateFields, HmrUpdateModuleOptions, generate_hmr_update_module, + generate_hmr_update_module_from_js, }; diff --git a/crates/oxc_angular_compiler/src/hmr/update_module.rs b/crates/oxc_angular_compiler/src/hmr/update_module.rs index 77a230ddd..a393b9d71 100644 --- a/crates/oxc_angular_compiler/src/hmr/update_module.rs +++ b/crates/oxc_angular_compiler/src/hmr/update_module.rs @@ -6,6 +6,28 @@ //! The generated modules export a function that Angular's `ɵɵreplaceMetadata` //! can call to apply the updated component definition. +/// The fields of a component definition that its template decides, from a compile of +/// the new template. +#[derive(Debug, Clone, Copy)] +pub struct HmrTemplateFields<'a> { + /// The compiled template function as JavaScript code. + pub template_js: &'a str, + + /// The number of element, text and container slots the template creates. + pub decls: u32, + + /// The number of binding slots the template uses. + pub vars: u32, + + /// The consts array (or the function returning it) as JavaScript code, if the + /// template has any. + pub consts_js: Option<&'a str>, + + /// The `ngContentSelectors` array as JavaScript code, if the template projects + /// content. + pub ng_content_selectors_js: Option<&'a str>, +} + /// Options for generating HMR update modules. #[derive(Debug, Clone)] pub struct HmrUpdateModuleOptions<'a> { @@ -15,8 +37,8 @@ pub struct HmrUpdateModuleOptions<'a> { /// Component class name. pub class_name: &'a str, - /// Compiled template as JavaScript code. - pub template_js: Option<&'a str>, + /// The new template's fields, or `None` when only the styles changed. + pub template: Option>, /// Compiled styles (CSS strings). pub styles: Option<&'a [String]>, @@ -24,18 +46,14 @@ pub struct HmrUpdateModuleOptions<'a> { /// Constant declarations (child view functions, pooled constants) as JavaScript code. /// These are emitted before the component definition update. pub declarations_js: Option<&'a str>, - - /// The consts array as JavaScript code. - /// This must be included to ensure the template function's constant references - /// match the component definition's consts array. - pub consts_js: Option<&'a str>, } /// Generate an HMR update module for a component. /// /// This generates a JavaScript module that exports a function which updates /// the component's definition. The function is called by Angular's -/// `ɵɵreplaceMetadata` runtime function. +/// `ɵɵreplaceMetadata` runtime function, which merges the definition it leaves +/// on the class into the live one and re-creates the component's views. /// /// # Generated Format /// @@ -43,14 +61,31 @@ pub struct HmrUpdateModuleOptions<'a> { /// // HMR update for: path/to/component.ts@ComponentName /// export default function ComponentName_UpdateMetadata(ComponentName, ɵɵnamespaces) { /// const i0 = ɵɵnamespaces[0]; -/// ComponentName.ɵcmp = i0.ɵɵdefineComponent({ +/// ComponentName.ɵcmp = { /// ...ComponentName.ɵcmp, +/// decls: 2, +/// vars: 1, +/// consts: null, +/// ngContentSelectors: undefined, /// template: function ComponentName_Template(rf, ctx) { ... }, /// styles: ["..."], -/// }); +/// tView: null, +/// }; /// } /// ``` /// +/// Angular's own update module (`compileHmrUpdateCallback`) writes a whole new +/// `ɵɵdefineComponent({...})` from the component class. Only the template is +/// compiled here, so the rest of the definition is the live one, copied as it +/// is. It must not go back through `ɵɵdefineComponent`: that function converts +/// `inputs` and `outputs` and derives `onPush` from `changeDetection`, so a +/// definition it has already built comes out with its outputs reversed and its +/// change detection strategy lost. +/// +/// Every field the template decides is written, including the ones the new +/// template has no value for, because a key left out keeps the old template's. +/// `tView` is cleared so the runtime builds the view for the new template. +/// /// # Arguments /// /// * `options` - HMR update module options @@ -59,20 +94,35 @@ pub struct HmrUpdateModuleOptions<'a> { /// /// JavaScript code for the HMR update module. pub fn generate_hmr_update_module(options: &HmrUpdateModuleOptions<'_>) -> String { - generate_hmr_update_module_internal( + let mut fields = std::vec::Vec::new(); + if let Some(template) = &options.template { + fields.push(("decls", template.decls.to_string())); + fields.push(("vars", template.vars.to_string())); + fields.push(("consts", template.consts_js.unwrap_or("null").to_string())); + fields.push(( + "ngContentSelectors", + template.ng_content_selectors_js.unwrap_or("undefined").to_string(), + )); + fields.push(("template", template.template_js.to_string())); + } + emit_update_module( options.component_id, options.class_name, - options.template_js, + &fields, options.styles, options.declarations_js, - options.consts_js, ) } /// Generate an HMR update module from a compiled template string. /// /// This is a convenience function when you already have the template -/// compiled to JavaScript. +/// compiled to JavaScript. It knows only what it is given: the module replaces +/// the template function (and `consts`, when passed) and leaves `decls`, `vars` +/// and `ngContentSelectors` as the live definition has them, so it is only +/// correct for a template that needs the same ones. Use +/// [`generate_hmr_update_module`] with the output of a template compile to +/// write them all. /// /// # Arguments /// @@ -95,36 +145,32 @@ pub fn generate_hmr_update_module_from_js( // can contain `@` (e.g. `node_modules/@scope/...`) but a class name cannot. let class_name = component_id.rsplit_once('@').map_or("Component", |(_, name)| name); - generate_hmr_update_module_internal( - component_id, - class_name, - Some(template_js), - styles, - declarations_js, - consts_js, - ) + let mut fields = vec![("template", template_js.to_string())]; + if let Some(consts_js) = consts_js { + fields.push(("consts", consts_js.to_string())); + } + emit_update_module(component_id, class_name, &fields, styles, declarations_js) } -/// Internal function to generate HMR update module. -fn generate_hmr_update_module_internal( +/// Emits an update module that copies the live definition and overrides `fields` +/// (name and JavaScript value) and, when known, `styles`. +fn emit_update_module( component_id: &str, class_name: &str, - template_js: Option<&str>, + fields: &[(&str, String)], styles: Option<&[String]>, declarations_js: Option<&str>, - consts_js: Option<&str>, ) -> String { let mut output = String::new(); // Add comment with component ID - output.push_str(&format!("// HMR update for: {}\n", component_id)); + output.push_str(&format!("// HMR update for: {component_id}\n")); // Export a function that Angular's ɵɵreplaceMetadata will call // The function signature matches what compileHmrUpdateCallback generates: // function ClassName_UpdateMetadata(ClassName, ɵɵnamespaces, ...locals) output.push_str(&format!( - "export default function {}_UpdateMetadata({}, ɵɵnamespaces) {{\n", - class_name, class_name + "export default function {class_name}_UpdateMetadata({class_name}, ɵɵnamespaces) {{\n" )); // Destructure the Angular core namespace from namespaces array @@ -133,46 +179,19 @@ fn generate_hmr_update_module_internal( // Add constant declarations (child view functions, pooled constants) // These must be declared before the component definition since the template may reference them if let Some(declarations) = declarations_js { - if !declarations.is_empty() { - // Indent each line of declarations for consistent formatting - for line in declarations.lines() { - output.push_str(" "); - output.push_str(line); - output.push('\n'); - } + // Indent each line of declarations for consistent formatting + for line in declarations.lines() { + output.push_str(" "); + output.push_str(line); + output.push('\n'); } } - // Update the component definition using ɵɵdefineComponent - // We spread the existing definition and override template/styles. - // IMPORTANT: We must override `inputs` with `inputConfig` because the spread - // includes `inputs` in the already-processed format (output of - // `parseAndConvertInputsForDefinition`). If we don't override, ɵɵdefineComponent - // will process them again, producing corrupted input mappings. - // `inputConfig` stores the original unprocessed inputs format. - // Only override when inputConfig exists (components with inputs); otherwise - // setting `inputs: undefined` would corrupt the component definition. - output.push_str(&format!(" {}.ɵcmp = i0.ɵɵdefineComponent({{\n", class_name)); - output.push_str(&format!(" ...{}.ɵcmp,\n", class_name)); - output.push_str(&format!( - " ...({cn}.ɵcmp.inputConfig ? {{ inputs: {cn}.ɵcmp.inputConfig }} : {{}}),\n", - cn = class_name - )); + output.push_str(&format!(" {class_name}.ɵcmp = {{\n")); + output.push_str(&format!(" ...{class_name}.ɵcmp,\n")); - // Add template function if present - if let Some(template_js) = template_js { - output.push_str(" template: "); - output.push_str(template_js); - output.push_str(",\n"); - } - - // Add consts array if present - // This must be included to override the old consts from the spread, ensuring - // the template function's constant references match the actual consts array - if let Some(consts_js) = consts_js { - output.push_str(" consts: "); - output.push_str(consts_js); - output.push_str(",\n"); + for (name, value) in fields { + output.push_str(&format!(" {name}: {value},\n")); } // Add styles whenever the caller has an answer — an EMPTY list included. @@ -195,7 +214,8 @@ fn generate_hmr_update_module_internal( } } - output.push_str(" });\n"); + output.push_str(" tView: null,\n"); + output.push_str(" };\n"); output.push_str("}\n"); output @@ -205,34 +225,110 @@ fn generate_hmr_update_module_internal( mod tests { use super::*; + const TEMPLATE: HmrTemplateFields<'static> = HmrTemplateFields { + template_js: "function AppComponent_Template(rf, ctx) { }", + decls: 3, + vars: 2, + consts_js: None, + ng_content_selectors_js: None, + }; + + fn module( + component_id: &str, + template: Option>, + styles: Option<&[String]>, + declarations_js: Option<&str>, + ) -> String { + // The path can contain `@` (e.g. `node_modules/@scope/...`); a class name cannot. + let class_name = component_id.rsplit_once('@').map_or("Component", |(_, name)| name); + generate_hmr_update_module(&HmrUpdateModuleOptions { + component_id, + class_name, + template, + styles, + declarations_js, + }) + } + #[test] - fn test_generate_hmr_update_module_from_js() { - let result = generate_hmr_update_module_from_js( + fn test_generate_hmr_update_module() { + let result = module( "src/app/app.component.ts@AppComponent", - "function AppComponent_Template(rf, ctx) { }", + Some(TEMPLATE), Some(&["h1 { color: red; }".to_string()]), None, - None, ); assert!(result.contains("export default function AppComponent_UpdateMetadata")); assert!(result.contains("const i0 = ɵɵnamespaces[0]")); - assert!(result.contains("AppComponent.ɵcmp = i0.ɵɵdefineComponent")); - assert!(result.contains("template:")); + assert!(result.contains("template: function AppComponent_Template(rf, ctx) { },")); assert!(result.contains("styles:")); assert!(result.contains("color: red")); } + /// The live definition is copied, never handed back to `ɵɵdefineComponent`, which + /// would convert its `inputs` and `outputs` a second time. #[test] - fn test_generate_hmr_update_module_no_styles() { - let result = generate_hmr_update_module_from_js( + fn test_live_definition_is_copied_not_redefined() { + let result = module("src/app/app.component.ts@AppComponent", Some(TEMPLATE), None, None); + + assert!(result.contains("AppComponent.ɵcmp = {\n ...AppComponent.ɵcmp,\n")); + assert!(!result.contains("ɵɵdefineComponent")); + assert!(!result.contains("inputConfig")); + // The runtime builds the view for the new template only if none is cached. + assert!(result.contains(" tView: null,\n };")); + } + + /// Every field the template decides is written after the spread, so none keeps the + /// old template's value: not even one the new template has nothing for. + #[test] + fn test_template_fields_are_all_written() { + let result = module("src/app/app.component.ts@AppComponent", Some(TEMPLATE), None, None); + + let spread = result.find("...AppComponent.ɵcmp").unwrap(); + for field in ["decls: 3,", "vars: 2,", "consts: null,", "ngContentSelectors: undefined,"] { + let at = result.find(field).unwrap_or_else(|| panic!("missing `{field}`:\n{result}")); + assert!(at > spread, "`{field}` must override the spread"); + } + } + + #[test] + fn test_consts_and_content_selectors() { + let result = module( "src/app/app.component.ts@AppComponent", - "function AppComponent_Template(rf, ctx) { }", + Some(HmrTemplateFields { + consts_js: Some("[\"value1\", 42, [1, 2, 3]]"), + ng_content_selectors_js: Some("_c1"), + ..TEMPLATE + }), None, + Some("const _c0 = [[[\"\", \"x\", \"\"]]];\nconst _c1 = [\"[x]\"];"), + ); + + assert!(result.contains("consts: [\"value1\", 42, [1, 2, 3]],")); + assert!(result.contains("ngContentSelectors: _c1,")); + } + + /// A styles-only update leaves the template's fields, and the view built for it, alone. + #[test] + fn test_styles_only_update() { + let result = module( + "src/app/app.component.ts@AppComponent", None, + Some(&["h1 { color: red; }".to_string()]), None, ); + for field in ["decls:", "vars:", "consts:", "ngContentSelectors:", "template:"] { + assert!(!result.contains(field), "unexpected `{field}`:\n{result}"); + } + assert!(result.contains("color: red")); + } + + #[test] + fn test_generate_hmr_update_module_no_styles() { + let result = module("src/app/app.component.ts@AppComponent", Some(TEMPLATE), None, None); + assert!(result.contains("export default function AppComponent_UpdateMetadata")); assert!(result.contains("template:")); assert!(!result.contains("styles:")); @@ -244,43 +340,45 @@ mod tests { // with `...ClassName.ɵcmp`, so a key it does not emit keeps its old // value — omitting `styles` here would leave the component's last // stylesheet applied until a full reload. - let result = generate_hmr_update_module_from_js( - "src/app/app.component.ts@AppComponent", - "function AppComponent_Template(rf, ctx) { }", - Some(&[]), - None, - None, - ); + let result = + module("src/app/app.component.ts@AppComponent", Some(TEMPLATE), Some(&[]), None); assert!(result.contains("styles: [],")); } #[test] - fn test_class_name_extraction() { - let result = generate_hmr_update_module_from_js( + fn test_class_name_in_function_and_definition() { + for id in [ "path/to/my.component.ts@MyComponent", - "function MyComponent_Template(rf, ctx) { }", - None, - None, - None, - ); - - assert!(result.contains("function MyComponent_UpdateMetadata(MyComponent")); - } - - #[test] - fn test_class_name_extraction_path_with_at() { - for id in - ["node_modules/@scope/pkg/src/a.ts@MyComponent", "packages/@a/@b/x.ts@MyComponent"] - { - let result = generate_hmr_update_module_from_js(id, "", None, None, None); + "node_modules/@scope/pkg/src/a.ts@MyComponent", + "packages/@a/@b/x.ts@MyComponent", + ] { + let result = module(id, Some(TEMPLATE), None, None); assert!(result.contains("function MyComponent_UpdateMetadata(MyComponent"), "{id}"); assert!(result.contains("MyComponent.ɵcmp ="), "{id}"); + assert!(result.contains(&format!("// HMR update for: {id}\n")), "{id}"); } } + /// From template JavaScript alone the counts are unknown, so they are left as they are. #[test] - fn test_class_name_extraction_no_at_falls_back() { + fn test_generate_hmr_update_module_from_js() { + let result = generate_hmr_update_module_from_js( + "node_modules/@scope/pkg/src/a.ts@MyComponent", + "function MyComponent_Template(rf, ctx) { }", + Some(&["h1 { color: red; }".to_string()]), + None, + Some("[\"value1\"]"), + ); + + assert!(result.contains("function MyComponent_UpdateMetadata(MyComponent")); + assert!(result.contains("MyComponent.ɵcmp = {\n ...MyComponent.ɵcmp,\n")); + assert!(result.contains("template: function MyComponent_Template(rf, ctx) { },")); + assert!(result.contains("consts: [\"value1\"],")); + assert!(result.contains("color: red")); + assert!(!result.contains("ɵɵdefineComponent")); + assert!(!result.contains("decls:")); + let result = generate_hmr_update_module_from_js("MyComponent", "", None, None, None); assert!(result.contains("function Component_UpdateMetadata(Component")); } @@ -288,12 +386,14 @@ mod tests { #[test] fn test_generate_hmr_update_module_with_declarations() { let declarations = "function App_For_1(rf, ctx) { }\nconst _c0 = [1, 2, 3];"; - let result = generate_hmr_update_module_from_js( + let result = module( "src/app/app.component.ts@AppComponent", - "function AppComponent_Template(rf, ctx) { App_For_1(rf, ctx); }", + Some(HmrTemplateFields { + template_js: "function AppComponent_Template(rf, ctx) { App_For_1(rf, ctx); }", + ..TEMPLATE + }), None, Some(declarations), - None, ); assert!(result.contains("export default function AppComponent_UpdateMetadata")); @@ -304,40 +404,4 @@ mod tests { let cmp_pos = result.find("ɵcmp").unwrap(); assert!(decl_pos < cmp_pos); } - - #[test] - fn test_generate_hmr_update_module_uses_input_config() { - let result = generate_hmr_update_module_from_js( - "src/app/app.component.ts@AppComponent", - "function AppComponent_Template(rf, ctx) { }", - None, - None, - None, - ); - - // The HMR module must conditionally override `inputs` with `inputConfig` - // to avoid double-processing by `parseAndConvertInputsForDefinition`. - // It must only do so when inputConfig exists to avoid setting inputs to undefined. - assert!(result.contains("AppComponent.ɵcmp.inputConfig")); - assert!(result.contains("inputs:")); - - // `inputs` override must come AFTER the spread to take precedence - let spread_pos = result.find("...AppComponent.ɵcmp").unwrap(); - let inputs_pos = result.find("inputConfig").unwrap(); - assert!(inputs_pos > spread_pos); - } - - #[test] - fn test_generate_hmr_update_module_with_consts() { - let result = generate_hmr_update_module_from_js( - "src/app/app.component.ts@AppComponent", - "function AppComponent_Template(rf, ctx) { }", - None, - None, - Some("[\"value1\", 42, [1, 2, 3]]"), - ); - - assert!(result.contains("export default function AppComponent_UpdateMetadata")); - assert!(result.contains("consts: [\"value1\", 42, [1, 2, 3]]")); - } } diff --git a/crates/oxc_angular_compiler/src/lib.rs b/crates/oxc_angular_compiler/src/lib.rs index 50a3029ce..b985030ea 100644 --- a/crates/oxc_angular_compiler/src/lib.rs +++ b/crates/oxc_angular_compiler/src/lib.rs @@ -78,9 +78,9 @@ pub use component::CrossFileAnalyzer; // Re-export HMR types pub use hmr::{ HmrDefinition, HmrDependencies, HmrLocalDependency, HmrMetadata, HmrNamespaceDependency, - HmrUpdateModuleOptions, compile_hmr_initializer, compile_hmr_update_callback, - extract_hmr_dependencies, generate_hmr_update_module, generate_hmr_update_module_from_js, - generate_style_update_module, + HmrTemplateFields, HmrUpdateModuleOptions, compile_hmr_initializer, + compile_hmr_update_callback, extract_hmr_dependencies, generate_hmr_update_module, + generate_hmr_update_module_from_js, generate_style_update_module, }; // Re-export styles diff --git a/napi/angular-compiler/.gitignore b/napi/angular-compiler/.gitignore index 9744d17f2..ac27bae0f 100644 --- a/napi/angular-compiler/.gitignore +++ b/napi/angular-compiler/.gitignore @@ -10,3 +10,4 @@ /benchmarks/typedb-web/dist /e2e/app/dist /tsconfig.tsbuildinfo +/test/.hmr-update-module-tmp diff --git a/napi/angular-compiler/e2e/app/src/app/@scoped/at.component.ts b/napi/angular-compiler/e2e/app/src/app/@scoped/at.component.ts new file mode 100644 index 000000000..fbedcfa18 --- /dev/null +++ b/napi/angular-compiler/e2e/app/src/app/@scoped/at.component.ts @@ -0,0 +1,10 @@ +import { Component } from '@angular/core' + +/** A component whose file path contains `@`, like one under `node_modules/@scope`. */ +@Component({ + selector: 'app-at', + templateUrl: './at.html', +}) +export class At { + value = 'AT_VALUE' +} diff --git a/napi/angular-compiler/e2e/app/src/app/@scoped/at.html b/napi/angular-compiler/e2e/app/src/app/@scoped/at.html new file mode 100644 index 000000000..2298e1f65 --- /dev/null +++ b/napi/angular-compiler/e2e/app/src/app/@scoped/at.html @@ -0,0 +1 @@ +

AT_ONE

diff --git a/napi/angular-compiler/e2e/app/src/app/app.component.ts b/napi/angular-compiler/e2e/app/src/app/app.component.ts index ee2c28635..80d52f906 100644 --- a/napi/angular-compiler/e2e/app/src/app/app.component.ts +++ b/napi/angular-compiler/e2e/app/src/app/app.component.ts @@ -1,17 +1,23 @@ import { Component, signal } from '@angular/core' +import { At } from './@scoped/at.component' import { Card } from './card.component' import { DuoFirst, DuoSecond } from './duo.component' import { InlineCard } from './inline-card.component' +import { Io } from './io.component' +import { Lab } from './lab.component' import { UTIL_VALUE } from './util' @Component({ selector: 'app-root', templateUrl: './app.html', styleUrl: './app.css', - imports: [Card, InlineCard, DuoFirst, DuoSecond], + imports: [Card, InlineCard, DuoFirst, DuoSecond, Lab, Io, At], }) export class App { protected readonly title = signal('E2E_TITLE') protected readonly utilValue = UTIL_VALUE + protected readonly ioClicked = signal('') + protected readonly ioRenamed = signal(0) + protected readonly ioChecked = signal(false) } diff --git a/napi/angular-compiler/e2e/app/src/app/app.html b/napi/angular-compiler/e2e/app/src/app/app.html index 3e9d268ca..b4abca5af 100644 --- a/napi/angular-compiler/e2e/app/src/app/app.html +++ b/napi/angular-compiler/e2e/app/src/app/app.html @@ -6,4 +6,16 @@

{{ title() }}

+ PXPYREST + +

{{ ioClicked() }}|{{ ioRenamed() }}|{{ ioChecked() }}

+ diff --git a/napi/angular-compiler/e2e/app/src/app/io.component.ts b/napi/angular-compiler/e2e/app/src/app/io.component.ts new file mode 100644 index 000000000..c963a13cb --- /dev/null +++ b/napi/angular-compiler/e2e/app/src/app/io.component.ts @@ -0,0 +1,22 @@ +import { Component, input, model, output } from '@angular/core' + +/** + * A component with every kind of output and an aliased, transformed input, for + * checking that a template hot swap leaves them bound. + */ +@Component({ + selector: 'app-io', + templateUrl: './io.html', +}) +export class Io { + clicked = output() + renamed = output({ alias: 'renamedAlias' }) + checked = model(false) + label = input('', { alias: 'labelAlias', transform: (value: string) => value.toUpperCase() }) + + emit(): void { + this.clicked.emit('CLICKED') + this.renamed.emit(7) + this.checked.set(!this.checked()) + } +} diff --git a/napi/angular-compiler/e2e/app/src/app/io.html b/napi/angular-compiler/e2e/app/src/app/io.html new file mode 100644 index 000000000..515c3af9a --- /dev/null +++ b/napi/angular-compiler/e2e/app/src/app/io.html @@ -0,0 +1 @@ + diff --git a/napi/angular-compiler/e2e/app/src/app/lab.component.ts b/napi/angular-compiler/e2e/app/src/app/lab.component.ts new file mode 100644 index 000000000..6aebae044 --- /dev/null +++ b/napi/angular-compiler/e2e/app/src/app/lab.component.ts @@ -0,0 +1,15 @@ +import { ChangeDetectionStrategy, Component } from '@angular/core' + +/** + * A component whose template the HMR definition tests rewrite: it gains and + * loses elements, bindings, attributes and slots. + */ +@Component({ + selector: 'app-lab', + templateUrl: './lab.html', + changeDetection: ChangeDetectionStrategy.Eager, +}) +export class Lab { + a = 'LAB_A' + b = 'LAB_B' +} diff --git a/napi/angular-compiler/e2e/app/src/app/lab.html b/napi/angular-compiler/e2e/app/src/app/lab.html new file mode 100644 index 000000000..eee2bf29f --- /dev/null +++ b/napi/angular-compiler/e2e/app/src/app/lab.html @@ -0,0 +1 @@ +

one

diff --git a/napi/angular-compiler/e2e/tests/hmr-input-component.spec.ts b/napi/angular-compiler/e2e/tests/hmr-input-component.spec.ts index ade518ad1..08866f3d6 100644 --- a/napi/angular-compiler/e2e/tests/hmr-input-component.spec.ts +++ b/napi/angular-compiler/e2e/tests/hmr-input-component.spec.ts @@ -40,9 +40,9 @@ test.describe('Input Component HMR', () => { }) expect(titleColor).toBe('rgb(255, 0, 0)') - // Verify input bindings still work after HMR update - // This exercises the inputConfig conditional in update_module.rs: - // If inputs were corrupted, these values would be missing or wrong + // Verify input bindings still work after HMR update. The update module + // copies the live definition, so its inputs are never converted again: + // if they were corrupted, these values would be missing or wrong await expect(page.locator('.card-title')).toContainText('INPUT_TITLE') await expect(page.locator('.card-value')).toContainText('42') }) diff --git a/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts b/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts new file mode 100644 index 000000000..a6b715439 --- /dev/null +++ b/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts @@ -0,0 +1,234 @@ +import type { Page } from '@playwright/test' + +import { test, expect } from '../fixtures/test-fixture.js' + +/** + * A template hot swap must leave the component with the definition a fresh + * compile of the new template would give it: sized for the new template + * (`decls`, `vars`, `consts`, `ngContentSelectors`), and otherwise untouched + * (`inputs`, `outputs`, change detection). + * + * The update module used to spread the live definition back through + * `ɵɵdefineComponent`. That kept the old template's counts and selectors, so a + * swap that added an element or binding rebuilt a view of the wrong size, and + * it converted `outputs` a second time, turning them round. + */ + +/** The parts of a rendered component's definition these tests care about. */ +async function definition(page: Page, selector: string) { + return await page.evaluate((selector) => { + const element = document.querySelector(selector)! + const def = (window as any).ng.getComponent(element).constructor.ɵcmp + return { + decls: def.decls as number, + vars: def.vars as number, + consts: (def.consts?.length ?? null) as number | null, + ngContentSelectors: (def.ngContentSelectors ?? null) as string[] | null, + outputs: { ...def.outputs } as Record, + inputs: Object.fromEntries( + Object.entries(def.inputs as Record).map(([name, value]) => [ + name, + value.slice(0, 2), + ]), + ), + onPush: def.onPush as boolean, + } + }, selector) +} + +test.describe('HMR update definition', () => { + let errors: string[] + + test.beforeEach(async ({ page }) => { + errors = [] + page.on('pageerror', (error) => errors.push(error.message)) + page.on('console', (message) => { + if (message.type() === 'error') errors.push(message.text()) + }) + await page.goto('/') + await page.waitForLoadState('networkidle') + }) + + test('a template that gains an element and a binding, then loses both', async ({ + page, + fileModifier, + hmrDetector, + }) => { + const sentinelId = await hmrDetector.addSentinel() + await expect(page.locator('app-lab .lab-one')).toHaveText('one') + expect(await definition(page, 'app-lab')).toMatchObject({ decls: 2, vars: 0 }) + + await fileModifier.modifyFile( + 'lab.html', + () => + '

one

{{ a }}

three\n', + ) + + await expect(page.locator('app-lab .lab-two')).toHaveText('LAB_A') + await expect(page.locator('app-lab .lab-three')).toHaveAttribute('title', 'LAB_B') + await expect(page.locator('app-lab .lab-one')).toHaveText('one') + expect(await definition(page, 'app-lab')).toMatchObject({ decls: 6, vars: 2 }) + + // A second swap on the same component, down to less than it started with. + await fileModifier.modifyFile('lab.html', () => 'min\n') + + await expect(page.locator('app-lab .lab-min')).toHaveText('min') + await expect(page.locator('app-lab .lab-two')).toHaveCount(0) + await expect(page.locator('app-lab .lab-three')).toHaveCount(0) + expect(await definition(page, 'app-lab')).toMatchObject({ decls: 2, vars: 0 }) + + expect(await hmrDetector.sentinelExists(sentinelId)).toBe(true) + expect(errors).toEqual([]) + }) + + test('a template that gains, changes and loses an ', async ({ + page, + fileModifier, + hmrDetector, + }) => { + const sentinelId = await hmrDetector.addSentinel() + await expect(page.locator('app-lab .lab-px')).toHaveCount(0) + expect(await definition(page, 'app-lab')).toMatchObject({ ngContentSelectors: null }) + + await fileModifier.modifyFile( + 'lab.html', + () => '

one

\n', + ) + + await expect(page.locator('app-lab .lab-px')).toHaveText('PX') + await expect(page.locator('app-lab .lab-py')).toHaveCount(0) + expect(await definition(page, 'app-lab')).toMatchObject({ ngContentSelectors: ['[x]'] }) + + await fileModifier.modifyFile( + 'lab.html', + () => '

one

\n', + ) + + await expect(page.locator('app-lab .lab-py')).toHaveText('PY') + await expect(page.locator('app-lab .lab-px')).toHaveText('PX') + await expect(page.locator('app-lab .lab-rest')).toHaveText('REST') + await expect(page.locator('app-lab')).toHaveText('PYonePXREST') + expect(await definition(page, 'app-lab')).toMatchObject({ ngContentSelectors: ['[y]', '*'] }) + + await fileModifier.modifyFile('lab.html', () => '

one

\n') + + await expect(page.locator('app-lab')).toHaveText('one') + await expect(page.locator('app-lab .lab-py')).toHaveCount(0) + expect(await definition(page, 'app-lab')).toMatchObject({ ngContentSelectors: null }) + + expect(await hmrDetector.sentinelExists(sentinelId)).toBe(true) + expect(errors).toEqual([]) + }) + + test('a template whose consts change', async ({ page, fileModifier, hmrDetector }) => { + const sentinelId = await hmrDetector.addSentinel() + expect(await definition(page, 'app-lab')).toMatchObject({ consts: 1 }) + + await fileModifier.modifyFile( + 'lab.html', + () => + '

one

{{ ref.value }}\n', + ) + + await expect(page.locator('app-lab #lab-id')).toHaveAttribute('data-k', 'v') + await expect(page.locator('app-lab .lab-input')).toHaveAttribute('type', 'text') + await expect(page.locator('app-lab .lab-ref')).toHaveText('typed') + expect(await definition(page, 'app-lab')).toMatchObject({ consts: 4 }) + + // No attributes at all: the old consts must not linger. + await fileModifier.modifyFile('lab.html', () => '

plain

\n') + + await expect(page.locator('app-lab p')).toHaveText('plain') + await expect(page.locator('app-lab p')).not.toHaveAttribute('class') + expect(await definition(page, 'app-lab')).toMatchObject({ consts: null }) + + expect(await hmrDetector.sentinelExists(sentinelId)).toBe(true) + expect(errors).toEqual([]) + }) + + test('outputs and inputs survive a swap and still reach the parent', async ({ + page, + fileModifier, + hmrDetector, + }) => { + const sentinelId = await hmrDetector.addSentinel() + const before = await definition(page, 'app-io') + expect(before.outputs).toEqual({ + clicked: 'clicked', + renamedAlias: 'renamed', + checkedChange: 'checked', + }) + expect(before.inputs.labelAlias[0]).toBe('label') + await expect(page.locator('.io-emit')).toHaveText('IO') + await expect(page.locator('.io-result')).toHaveText('|0|false') + + await fileModifier.modifyFile( + 'io.html', + () => + '{{ checked() }}\n', + ) + + await expect(page.locator('.io-checked')).toHaveText('false') + // The aliased input kept its transform. + await expect(page.locator('.io-emit')).toHaveText('IO') + let after = await definition(page, 'app-io') + expect(after.outputs).toEqual(before.outputs) + expect(after.inputs).toEqual(before.inputs) + + // output(), the aliased output and the model's change event all reach the parent. + await page.locator('.io-emit').click() + await expect(page.locator('.io-result')).toHaveText('CLICKED|7|true') + await expect(page.locator('.io-checked')).toHaveText('true') + + // A second swap. + await fileModifier.modifyFile( + 'io.html', + () => '\n', + ) + + await expect(page.locator('.io-emit')).toHaveText('again IO') + after = await definition(page, 'app-io') + expect(after.outputs).toEqual(before.outputs) + expect(after.inputs).toEqual(before.inputs) + + await page.locator('.io-emit').click() + await expect(page.locator('.io-result')).toHaveText('CLICKED|7|false') + + expect(await hmrDetector.sentinelExists(sentinelId)).toBe(true) + expect(errors).toEqual([]) + }) + + test('the change detection strategy survives a swap', async ({ page, fileModifier }) => { + expect((await definition(page, 'app-lab')).onPush).toBe(false) + expect((await definition(page, 'app-io')).onPush).toBe(true) + + await fileModifier.modifyFile('lab.html', () => '

swapped

\n') + await fileModifier.modifyFile( + 'io.html', + () => '\n', + ) + + await expect(page.locator('app-lab .lab-one')).toHaveText('swapped') + await expect(page.locator('.io-emit')).toHaveText('swapped') + expect((await definition(page, 'app-lab')).onPush).toBe(false) + expect((await definition(page, 'app-io')).onPush).toBe(true) + expect(errors).toEqual([]) + }) + + test('a component whose path contains `@`', async ({ page, fileModifier, hmrDetector }) => { + const sentinelId = await hmrDetector.addSentinel() + await expect(page.locator('.at-text')).toHaveText('AT_ONE') + + await fileModifier.modifyFile( + '@scoped/at.html', + () => '

AT_TWO

{{ value }}\n', + ) + + await expect(page.locator('.at-text')).toHaveText('AT_TWO') + await expect(page.locator('.at-value')).toHaveText('AT_VALUE') + expect(await definition(page, 'app-at')).toMatchObject({ decls: 4, vars: 1 }) + + expect(await hmrDetector.sentinelExists(sentinelId)).toBe(true) + expect(errors).toEqual([]) + }) +}) diff --git a/napi/angular-compiler/package.json b/napi/angular-compiler/package.json index 4e0740f9d..5cbc749ca 100644 --- a/napi/angular-compiler/package.json +++ b/napi/angular-compiler/package.json @@ -70,11 +70,13 @@ "devDependencies": { "@angular/compiler": "^22.0.0-rc.2", "@angular/compiler-cli": "^22.0.0-rc.2", + "@angular/core": "^22.0.0-rc.2", "@napi-rs/cli": "catalog:", "@oxc-node/cli": "catalog:", "@playwright/test": "^1.58.0", "@types/node": "catalog:", "oxfmt": "catalog:", + "rxjs": "catalog:", "sass": "^1.93.2", "typescript": "catalog:", "vite": "catalog:", diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index 32930eec3..dcae33a1c 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -20,7 +20,7 @@ use napi_derive::napi; use oxc_allocator::Allocator; use oxc_angular_compiler::{ AngularVersion as RustAngularVersion, ChangeDetectionStrategy as RustChangeDetectionStrategy, - HmrUpdateModuleOptions, HostMetadataInput as RustHostMetadataInput, + HmrTemplateFields, HmrUpdateModuleOptions, HostMetadataInput as RustHostMetadataInput, TransformOptions as RustTransformOptions, ViewEncapsulation as RustViewEncapsulation, build_ctor_params_metadata_in as core_build_ctor_params_metadata, build_decorator_metadata_array as core_build_decorator_metadata_array, @@ -659,14 +659,19 @@ pub fn compile_for_hmr_sync( None }; - // Generate HMR module with declarations, encapsulated styles, and consts + // Generate the HMR module from everything the new template decided let hmr_module = generate_hmr_update_module(&HmrUpdateModuleOptions { component_id: &component_id, class_name: &component_name, - template_js: Some(&template_js), + template: Some(HmrTemplateFields { + template_js: &template_js, + decls: output.decls, + vars: output.vars, + consts_js: output.consts_js.as_deref(), + ng_content_selectors_js: output.ng_content_selectors_js.as_deref(), + }), styles: encapsulated_styles.as_deref(), declarations_js, - consts_js: output.consts_js.as_deref(), }); HmrCompileResult { hmr_module, component_id, template_js, errors: vec![] } diff --git a/napi/angular-compiler/test/hmr-update-module.test.ts b/napi/angular-compiler/test/hmr-update-module.test.ts new file mode 100644 index 000000000..3732ef2b1 --- /dev/null +++ b/napi/angular-compiler/test/hmr-update-module.test.ts @@ -0,0 +1,325 @@ +/** + * The HMR update module that `compileForHmrSync` returns, applied to a real + * compiled component with the real `@angular/core`. + * + * Each test compiles a component with this compiler, loads it, and hands the + * update module to Angular's own `ɵɵreplaceMetadata`. Two things must hold for + * the definition that results: + * + * - every field the template decides (`decls`, `vars`, `consts`, + * `ngContentSelectors`) is the new template's, checked against a fresh + * compile of the same component with that template; + * - nothing else changes. `ɵɵdefineComponent` converts `inputs` and `outputs` + * and derives `onPush`, so a definition passed through it a second time + * comes out wrong. + * + * Rendering after a swap is covered by `e2e/tests/hmr-update-definition.spec.ts`. + */ +import { mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' + +import * as core from '@angular/core' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' + +import { compileForHmrSync, transformAngularFileSync } from '../index.js' + +const TMP = join(fileURLToPath(new URL('.', import.meta.url)), '.hmr-update-module-tmp') +let nextFile = 0 + +beforeAll(() => { + mkdirSync(TMP, { recursive: true }) + // i18n constants call `$localize` when the definition's `consts` are read. + ;(globalThis as any).$localize ??= (strings: TemplateStringsArray, ...values: unknown[]) => + String.raw({ raw: strings }, ...values) +}) + +afterAll(() => { + rmSync(TMP, { recursive: true, force: true }) +}) + +interface ComponentSource { + /** Extra names to import from `@angular/core`. */ + imports?: string + /** Extra `@Component` metadata, e.g. `changeDetection: ...,`. */ + metadata?: string + /** The class body. */ + members?: string +} + +/** Compiles a component named `X` with `template` and loads the class. */ +async function component(template: string, source: ComponentSource = {}): Promise { + const code = `import { Component${source.imports ? `, ${source.imports}` : ''} } from '@angular/core'; + +@Component({ + selector: 'x-cmp', + ${source.metadata ?? ''} + template: ${JSON.stringify(template)}, +}) +export class X { + ${source.members ?? ''} +} +` + const result = transformAngularFileSync(code, '/x/a.ts', {}) + expect(result.errors).toEqual([]) + const file = join(TMP, `component-${nextFile++}.mjs`) + writeFileSync(file, result.code) + return (await import(/* @vite-ignore */ pathToFileURL(file).href)).X +} + +/** Loads an update module the way the browser does: as a module, by its default export. */ +async function loadUpdateModule(code: string): Promise<(...args: unknown[]) => void> { + const file = join(TMP, `update-${nextFile++}.mjs`) + writeFileSync(file, code) + return (await import(/* @vite-ignore */ pathToFileURL(file).href)).default +} + +/** Hot-swaps `template` into `type` the way the runtime does. */ +async function swap(type: any, template: string, id = '/x/a.ts'): Promise { + const result = compileForHmrSync(template, type.name, id, null, {}) + expect(result.errors).toEqual([]) + const applyMetadata = await loadUpdateModule(result.hmrModule) + ;(core as any).ɵɵreplaceMetadata(type, applyMetadata, [core], []) + return result.hmrModule +} + +/** The fields of a definition that its template decides. */ +function templateFields(type: any) { + const def = type.ɵcmp + return { + decls: def.decls, + vars: def.vars, + consts: typeof def.consts === 'function' ? def.consts() : def.consts, + ngContentSelectors: def.ngContentSelectors, + } +} + +/** Every other field of a definition, as a comparable snapshot. */ +function otherFields(type: any) { + const rest = { ...type.ɵcmp } + for (const field of ['decls', 'vars', 'consts', 'ngContentSelectors', 'template', 'tView']) { + delete rest[field] + } + return rest +} + +/** + * Swaps `to` into a component compiled with `from` and checks the result + * against a component compiled with `to` in the first place. + */ +async function expectSwapMatchesFreshCompile( + from: string, + to: string, + source: ComponentSource = {}, +): Promise { + const type = await component(from, source) + const before = otherFields(type) + await swap(type, to) + const fresh = await component(to, source) + expect(templateFields(type)).toEqual(templateFields(fresh)) + expect(otherFields(type)).toEqual(before) + expect(type.ɵcmp.tView).toBeNull() + return type +} + +describe('compileForHmrSync update module', () => { + it('writes the slot counts and projection selectors of the new template', async () => { + const type = await component('', { members: `a = 'A';` }) + const module = await swap(type, '{{ a }}') + + expect(templateFields(type)).toEqual({ + decls: 4, + vars: 1, + consts: null, + ngContentSelectors: ['[x]'], + }) + // Every constant the module declares is one it uses. + for (const [, name] of module.matchAll(/const (_c\d+) =/g)) { + expect(module.split(name).length - 1, `${name} is declared and used`).toBeGreaterThan(1) + } + }) + + describe('slot counts', () => { + const members = { members: `a = 'A'; b = 'B';` } + + it('follows a template that gains an element', async () => { + await expectSwapMatchesFreshCompile('

one

', '

one

two

three', members) + }) + + it('follows a template that gains a binding', async () => { + await expectSwapMatchesFreshCompile( + '

one

', + '

{{ a }} {{ b }}

', + members, + ) + }) + + it('follows a template that loses elements and bindings', async () => { + await expectSwapMatchesFreshCompile( + '

{{ a }}

{{ b }}

', + '

one

', + members, + ) + }) + + it('follows a template that gains a control-flow block', async () => { + await expectSwapMatchesFreshCompile( + '

one

', + '@if (a) {

{{ a }}

} @else { {{ b }} }', + members, + ) + }) + }) + + describe('projection selectors', () => { + it('follows a template that gains an ', async () => { + const type = await expectSwapMatchesFreshCompile( + '

one

', + '', + ) + expect(type.ɵcmp.ngContentSelectors).toEqual(['[x]', '*']) + }) + + it('follows a template that loses its ', async () => { + const type = await expectSwapMatchesFreshCompile('', '

one

') + expect(type.ɵcmp.ngContentSelectors).toBeUndefined() + }) + + it('follows a template that changes its ', async () => { + const type = await expectSwapMatchesFreshCompile( + '', + '', + ) + expect(type.ɵcmp.ngContentSelectors).toEqual(['[y]', 'z-el']) + }) + }) + + describe('consts', () => { + it('follows a template that gains an attribute', async () => { + const type = await expectSwapMatchesFreshCompile( + '

one

', + '

one

', + ) + expect(type.ɵcmp.consts).not.toBeNull() + }) + + it('clears the consts of a template that no longer needs any', async () => { + const type = await expectSwapMatchesFreshCompile('

one

', '

one

') + expect(type.ɵcmp.consts).toBeNull() + }) + + it('follows a template that gains an i18n message', async () => { + const type = await expectSwapMatchesFreshCompile( + '

one

', + '

Hello

', + ) + expect(JSON.stringify(templateFields(type).consts)).toContain('Hello') + }) + }) + + describe('fields the template does not decide', () => { + const io: ComponentSource = { + imports: 'input, model, output', + members: ` + clicked = output(); + renamed = output({ alias: 'renamedAlias' }); + checked = model(false); + label = input('', { alias: 'labelAlias', transform: (v) => String(v).toUpperCase() }); + plain = input(0); + `, + } + + it('leaves outputs as they were, for output(), model() and an alias', async () => { + const type = await component('

one

', io) + const outputs = { ...type.ɵcmp.outputs } + expect(outputs).toEqual({ + clicked: 'clicked', + renamedAlias: 'renamed', + checkedChange: 'checked', + }) + + await swap(type, '

one

') + + expect(type.ɵcmp.outputs).toEqual(outputs) + }) + + it('leaves inputs as they were, for an alias and a transform', async () => { + const type = await component('

one

', io) + const inputs = { ...type.ɵcmp.inputs } + const declaredInputs = { ...type.ɵcmp.declaredInputs } + expect(Object.keys(inputs).sort()).toEqual(['checked', 'labelAlias', 'plain']) + expect(inputs.labelAlias[0]).toBe('label') + + await swap(type, '

{{ label() }} {{ plain() }}

') + + expect(type.ɵcmp.inputs).toEqual(inputs) + expect(type.ɵcmp.declaredInputs).toEqual(declaredInputs) + }) + + it('leaves the change detection strategy as it was', async () => { + for (const strategy of ['Eager', 'OnPush']) { + const type = await component('

one

', { + imports: 'ChangeDetectionStrategy', + metadata: `changeDetection: ChangeDetectionStrategy.${strategy},`, + }) + const onPush = type.ɵcmp.onPush + expect(onPush).toBe(strategy === 'OnPush') + + await swap(type, '

one

two

') + + expect(type.ɵcmp.onPush, strategy).toBe(onPush) + } + }) + + it('leaves everything else as it was', async () => { + await expectSwapMatchesFreshCompile('

one

', '

{{ label() }}

', { + ...io, + metadata: `host: { '(click)': 'clicked.emit()', '[title]': 'label()', role: 'button' }, exportAs: 'xCmp',`, + }) + }) + }) + + it('does not build the definition again from the live one', async () => { + const type = await component('

one

') + const result = compileForHmrSync('

two

', 'X', '/x/a.ts', null, {}) + const applyMetadata = await loadUpdateModule(result.hmrModule) + let defined = 0 + const namespace = { ...core, ɵɵdefineComponent: () => (defined++, type.ɵcmp) } + + applyMetadata(type, [namespace]) + + expect(defined).toBe(0) + }) + + it('applies two swaps in a row', async () => { + const source: ComponentSource = { + imports: 'model, output', + members: `a = 'A'; clicked = output(); checked = model(false);`, + } + const type = await component('

one

', source) + const outputs = { ...type.ɵcmp.outputs } + const other = otherFields(type) + + await swap(type, '

{{ a }}

') + await swap(type, '{{ a }}') + + const fresh = await component( + '{{ a }}', + source, + ) + expect(templateFields(type)).toEqual(templateFields(fresh)) + expect(type.ɵcmp.outputs).toEqual(outputs) + expect(otherFields(type)).toEqual(other) + }) + + it('handles a component id whose path contains `@`', async () => { + const id = '/repo/node_modules/@scope/pkg/@x/a.ts' + const result = compileForHmrSync('

one

', 'X', id, null, {}) + expect(result.componentId).toBe(`${id}@X`) + expect(result.hmrModule).toContain('export default function X_UpdateMetadata(X, ɵɵnamespaces)') + + const type = await component('') + await swap(type, '

one

two

', id) + expect(type.ɵcmp.decls).toBe(4) + }) +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 24ee40e69..4fb27316e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -75,6 +75,9 @@ importers: '@angular/compiler-cli': specifier: ^22.0.0-rc.2 version: 22.2.1(@angular/compiler@22.2.1)(typescript@6.0.3) + '@angular/core': + specifier: ^22.0.0-rc.2 + version: 22.2.1(@angular/compiler@22.2.1)(rxjs@7.8.2)(zone.js@0.16.3) '@napi-rs/cli': specifier: 'catalog:' version: 3.10.7(@emnapi/core@1.11.2)(@emnapi/runtime@1.11.2)(@types/node@24.1.0)(supports-color@8.1.1) @@ -90,6 +93,9 @@ importers: oxfmt: specifier: 'catalog:' version: 0.72.0 + rxjs: + specifier: 'catalog:' + version: 7.8.2 sass: specifier: ^1.93.2 version: 1.105.1 From 29e6d0ce33e4d1c03cccf37ce04f9411a64a1314 Mon Sep 17 00:00:00 2001 From: Ashley Hunter Date: Fri, 9 Oct 2026 08:08:06 +0100 Subject: [PATCH 2/5] test(e2e): space repeat edits past the watcher's change throttle On Linux a hot swap is on screen about 30ms after the write, so a test that edited the same template again straight away wrote inside chokidar's 50ms change throttle and the dev server never saw the second edit. Wait 200ms before editing a file a second time. --- .../e2e/tests/hmr-update-definition.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts b/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts index a6b715439..9ec6f8dec 100644 --- a/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts +++ b/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts @@ -14,6 +14,13 @@ import { test, expect } from '../fixtures/test-fixture.js' * it converted `outputs` a second time, turning them round. */ +/** + * The dev server's watcher drops a change that follows another change to the + * same file within 50ms, and a hot swap can be on screen sooner than that. Wait + * this long before editing a file a second time. + */ +const WATCHER_THROTTLE_MS = 200 + /** The parts of a rendered component's definition these tests care about. */ async function definition(page: Page, selector: string) { return await page.evaluate((selector) => { @@ -70,6 +77,7 @@ test.describe('HMR update definition', () => { expect(await definition(page, 'app-lab')).toMatchObject({ decls: 6, vars: 2 }) // A second swap on the same component, down to less than it started with. + await page.waitForTimeout(WATCHER_THROTTLE_MS) await fileModifier.modifyFile('lab.html', () => 'min\n') await expect(page.locator('app-lab .lab-min')).toHaveText('min') @@ -99,6 +107,7 @@ test.describe('HMR update definition', () => { await expect(page.locator('app-lab .lab-py')).toHaveCount(0) expect(await definition(page, 'app-lab')).toMatchObject({ ngContentSelectors: ['[x]'] }) + await page.waitForTimeout(WATCHER_THROTTLE_MS) await fileModifier.modifyFile( 'lab.html', () => '

one

\n', @@ -110,6 +119,7 @@ test.describe('HMR update definition', () => { await expect(page.locator('app-lab')).toHaveText('PYonePXREST') expect(await definition(page, 'app-lab')).toMatchObject({ ngContentSelectors: ['[y]', '*'] }) + await page.waitForTimeout(WATCHER_THROTTLE_MS) await fileModifier.modifyFile('lab.html', () => '

one

\n') await expect(page.locator('app-lab')).toHaveText('one') @@ -136,6 +146,7 @@ test.describe('HMR update definition', () => { expect(await definition(page, 'app-lab')).toMatchObject({ consts: 4 }) // No attributes at all: the old consts must not linger. + await page.waitForTimeout(WATCHER_THROTTLE_MS) await fileModifier.modifyFile('lab.html', () => '

plain

\n') await expect(page.locator('app-lab p')).toHaveText('plain') @@ -181,6 +192,7 @@ test.describe('HMR update definition', () => { await expect(page.locator('.io-checked')).toHaveText('true') // A second swap. + await page.waitForTimeout(WATCHER_THROTTLE_MS) await fileModifier.modifyFile( 'io.html', () => '\n', From 6bf8042fda94838516b39f52f8b1e89d55be7ec1 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sat, 10 Oct 2026 00:08:25 +0800 Subject: [PATCH 3/5] fix(hmr): close update-module gaps found in review Follow-up to the definition-merge fix, addressing cases where a hot swap still carried stale or invalid state: - update_module: emit `tView: null` only when a template is being replaced. Styles-only updates no longer force a needless view rebuild through getOrCreateComponentTView. - update_module: `generate_hmr_update_module_from_js` emitted invalid JS (`template: ,`) for a styles-only call with empty template; empty template_js now produces a styles-only module with no template/consts/tView keys. - update_module: a template swap with no consts now emits `consts: null` instead of omitting the key, so the new template no longer indexes into the old template's constant pool. - transform: compile_template_for_hmr propagates error-severity diagnostics instead of emitting decls/vars = 0 output, and always uses DeferBlockDepsEmitMode::PerComponent (PerBlock left @defer resolver calls with no deferred-dependencies block to resolve, matching upstream handler.ts:1281 for local compilation). - lib.rs / vite-plugin: honor the component's declared encapsulation when encapsulating styles for a hot update instead of assuming Emulated, so ViewEncapsulation.None/ShadowDom components keep unshimmed CSS; error-severity diagnostics surface as errors and the plugin falls back to a full reload. - Document that the JS-facing generateHmrModule only produces correct modules when decls/vars/ngContentSelectors are unchanged; the vite plugin's live path (compileForHmrSync) writes all of them. Verified: cargo test -p oxc_angular_compiler 3065 passed, vitest hmr-update-module + ssr-hmr 30/30 against real @angular/core 22.2.1. --- .../src/component/transform.rs | 28 +++++-- crates/oxc_angular_compiler/src/hmr/mod.rs | 16 +++- .../src/hmr/update_module.rs | 83 +++++++++++++++---- .../e2e/app/src/app/lab.component.ts | 3 + .../e2e/tests/hmr-update-definition.spec.ts | 25 +++++- napi/angular-compiler/index.d.ts | 13 ++- napi/angular-compiler/src/lib.rs | 33 ++++++-- .../test/hmr-update-module.test.ts | 30 +++++-- napi/angular-compiler/test/ssr-hmr.test.ts | 5 +- napi/angular-compiler/vite-plugin/index.ts | 10 +++ 10 files changed, 206 insertions(+), 40 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 1a9157b57..61f97fc3d 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -428,6 +428,11 @@ pub struct HmrTemplateCompileOutput { /// The `ngContentSelectors` array as JavaScript code, when the template projects /// content. It refers to a constant declared in `declarations_js`. pub ng_content_selectors_js: Option, + + /// Non-fatal diagnostics the pipeline reported while compiling (warnings, + /// advice). Error-severity diagnostics are returned as `Err` instead, since + /// the emitted code cannot be trusted. + pub diagnostics: Vec, } /// Compiled component information. @@ -5500,11 +5505,14 @@ pub fn compile_template_for_hmr<'a>( // OXC is a single-file compiler (local compilation mode): always use Full mode. let mode = TemplateCompilationMode::Full; - let defer_block_deps_emit_mode = if options.jit { - DeferBlockDepsEmitMode::PerComponent - } else { - DeferBlockDepsEmitMode::PerBlock - }; + // Defer-block dependency resolution needs component metadata this compile + // does not have, and PerBlock mode reports "unable to find a dependency + // function for this deferred block" for every `@defer` when its `blocks` + // map is not populated — nothing ever populates it here. Local compilation + // uses PerComponent (upstream handler.ts:1281); with no resolver available + // it still emits `ɵɵdefer` with a null dependencies argument, which is what + // PerBlock's empty map would have produced anyway. + let defer_block_deps_emit_mode = DeferBlockDepsEmitMode::PerComponent; let enable_debug_locations = !options.advanced_optimizations; @@ -5536,6 +5544,15 @@ pub fn compile_template_for_hmr<'a>( // Collect any diagnostics from the compilation job diagnostics.extend(job.diagnostics.into_iter()); + // A pipeline error means the emitted code is already wrong — an update + // module must not be generated from it, and `decls`/`vars` may not be + // reliable (they default to 0 when slot allocation never ran, which would + // size the new TView incorrectly). Non-error diagnostics go out on the + // output so the caller can surface them. + if diagnostics.iter().any(|d| d.severity == oxc_diagnostics::Severity::Error) { + return Err(diagnostics); + } + let emitter = JsEmitter::new(); // Emit the template function @@ -5625,6 +5642,7 @@ pub fn compile_template_for_hmr<'a>( decls: job.root.decl_count.unwrap_or(0), vars: job.root.vars.unwrap_or(0), ng_content_selectors_js, + diagnostics, }) } diff --git a/crates/oxc_angular_compiler/src/hmr/mod.rs b/crates/oxc_angular_compiler/src/hmr/mod.rs index 621e0c2cd..85d709e5b 100644 --- a/crates/oxc_angular_compiler/src/hmr/mod.rs +++ b/crates/oxc_angular_compiler/src/hmr/mod.rs @@ -27,10 +27,18 @@ //! }); //! //! // HMR Update Module (served by Vite middleware) -//! export default { -//! ɵcmp: /* compiled component definition */, -//! template: /* template function */, -//! }; +//! export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces) { +//! const i0 = ɵɵnamespaces[0]; +//! Cmp.ɵcmp = { +//! ...Cmp.ɵcmp, +//! decls: /* new template's slot count */, +//! vars: /* new template's binding count */, +//! consts: /* new template's consts, or null */, +//! ngContentSelectors: /* new template's selectors, or undefined */, +//! template: /* new template function */, +//! tView: null, +//! }; +//! } //! ``` mod dependencies; diff --git a/crates/oxc_angular_compiler/src/hmr/update_module.rs b/crates/oxc_angular_compiler/src/hmr/update_module.rs index a393b9d71..d19d9098c 100644 --- a/crates/oxc_angular_compiler/src/hmr/update_module.rs +++ b/crates/oxc_angular_compiler/src/hmr/update_module.rs @@ -111,6 +111,10 @@ pub fn generate_hmr_update_module(options: &HmrUpdateModuleOptions<'_>) -> Strin &fields, options.styles, options.declarations_js, + // A missing template is a styles-only update: the live tView still + // matches the template it was built for, so keep it and let + // recreateMatchingLViews re-render rather than rebuilding it. + options.template.is_some(), ) } @@ -118,18 +122,24 @@ pub fn generate_hmr_update_module(options: &HmrUpdateModuleOptions<'_>) -> Strin /// /// This is a convenience function when you already have the template /// compiled to JavaScript. It knows only what it is given: the module replaces -/// the template function (and `consts`, when passed) and leaves `decls`, `vars` -/// and `ngContentSelectors` as the live definition has them, so it is only -/// correct for a template that needs the same ones. Use -/// [`generate_hmr_update_module`] with the output of a template compile to -/// write them all. +/// the template function and `consts`, but `decls`, `vars` and +/// `ngContentSelectors` stay as the live definition has them — keys the module +/// does not write survive the `...ɵcmp` spread — so it is only correct for a +/// template that needs the same slot counts and projection selectors as the +/// old one. For a template that changes them, use +/// [`generate_hmr_update_module`] with the output of a template compile, which +/// writes them all. /// /// # Arguments /// /// * `component_id` - Component ID (path@ClassName) -/// * `template_js` - Compiled template function as JavaScript +/// * `template_js` - Compiled template function as JavaScript, or an empty +/// string for a styles-only update /// * `styles` - Optional array of CSS styles /// * `declarations_js` - Optional constant declarations (child views, pooled constants) +/// * `consts_js` - The new template's consts array as JavaScript. When the +/// module emits a template, `None` writes `consts: null`, clearing whatever +/// the old template's consts held. /// /// # Returns /// @@ -145,21 +155,34 @@ pub fn generate_hmr_update_module_from_js( // can contain `@` (e.g. `node_modules/@scope/...`) but a class name cannot. let class_name = component_id.rsplit_once('@').map_or("Component", |(_, name)| name); - let mut fields = vec![("template", template_js.to_string())]; - if let Some(consts_js) = consts_js { - fields.push(("consts", consts_js.to_string())); + let mut fields = std::vec::Vec::new(); + if !template_js.is_empty() { + fields.push(("template", template_js.to_string())); + // A swapped template decides the consts it indexes into, so an absent + // `consts_js` clears them rather than keeping the old template's. + fields.push(("consts", consts_js.unwrap_or("null").to_string())); } - emit_update_module(component_id, class_name, &fields, styles, declarations_js) + emit_update_module( + component_id, + class_name, + &fields, + styles, + declarations_js, + !template_js.is_empty(), + ) } /// Emits an update module that copies the live definition and overrides `fields` -/// (name and JavaScript value) and, when known, `styles`. +/// (name and JavaScript value) and, when known, `styles`. `clear_tview` writes +/// `tView: null`, making the runtime build the view for the new template; it +/// must only be set when `fields` replaces the template. fn emit_update_module( component_id: &str, class_name: &str, fields: &[(&str, String)], styles: Option<&[String]>, declarations_js: Option<&str>, + clear_tview: bool, ) -> String { let mut output = String::new(); @@ -214,7 +237,9 @@ fn emit_update_module( } } - output.push_str(" tView: null,\n"); + if clear_tview { + output.push_str(" tView: null,\n"); + } output.push_str(" };\n"); output.push_str("}\n"); @@ -319,7 +344,9 @@ mod tests { None, ); - for field in ["decls:", "vars:", "consts:", "ngContentSelectors:", "template:"] { + // `tView` included: clearing it would make getOrCreateComponentTView + // rebuild an identical view before recreateMatchingLViews re-renders. + for field in ["decls:", "vars:", "consts:", "ngContentSelectors:", "template:", "tView:"] { assert!(!result.contains(field), "unexpected `{field}`:\n{result}"); } assert!(result.contains("color: red")); @@ -360,7 +387,9 @@ mod tests { } } - /// From template JavaScript alone the counts are unknown, so they are left as they are. + /// From template JavaScript alone the slot counts are unknown, so they are + /// left as they are — `decls`, `vars` and `ngContentSelectors` keep the old + /// template's values, which is only correct when they do not change. #[test] fn test_generate_hmr_update_module_from_js() { let result = generate_hmr_update_module_from_js( @@ -376,11 +405,35 @@ mod tests { assert!(result.contains("template: function MyComponent_Template(rf, ctx) { },")); assert!(result.contains("consts: [\"value1\"],")); assert!(result.contains("color: red")); + assert!(result.contains(" tView: null,\n };")); assert!(!result.contains("ɵɵdefineComponent")); assert!(!result.contains("decls:")); + assert!(!result.contains("ngContentSelectors:")); + + // The swapped template decides `consts`: no consts argument clears the + // old template's rather than keeping it through the spread. + let result = generate_hmr_update_module_from_js( + "a.ts@MyComponent", + "function T(rf, ctx) {}", + None, + None, + None, + ); + assert!(result.contains("consts: null,"), "{result}"); - let result = generate_hmr_update_module_from_js("MyComponent", "", None, None, None); + // No template is a styles-only update: no template key (which would be + // `template: ,` — invalid JS) and no `tView` clear. + let result = generate_hmr_update_module_from_js( + "MyComponent", + "", + Some(&["h1 { color: red; }".to_string()]), + None, + None, + ); assert!(result.contains("function Component_UpdateMetadata(Component")); + assert!(!result.contains("template:"), "{result}"); + assert!(!result.contains("tView"), "{result}"); + assert!(result.contains("color: red")); } #[test] diff --git a/napi/angular-compiler/e2e/app/src/app/lab.component.ts b/napi/angular-compiler/e2e/app/src/app/lab.component.ts index 6aebae044..4271d6e7e 100644 --- a/napi/angular-compiler/e2e/app/src/app/lab.component.ts +++ b/napi/angular-compiler/e2e/app/src/app/lab.component.ts @@ -7,6 +7,9 @@ import { ChangeDetectionStrategy, Component } from '@angular/core' @Component({ selector: 'app-lab', templateUrl: './lab.html', + // A style block so every template update module also merges `styles`, the + // key the swap otherwise leaves to the `...ɵcmp` spread. + styles: ['.lab-one { font-weight: bold }'], changeDetection: ChangeDetectionStrategy.Eager, }) export class Lab { diff --git a/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts b/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts index 9ec6f8dec..8611da6a1 100644 --- a/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts +++ b/napi/angular-compiler/e2e/tests/hmr-update-definition.spec.ts @@ -26,11 +26,15 @@ async function definition(page: Page, selector: string) { return await page.evaluate((selector) => { const element = document.querySelector(selector)! const def = (window as any).ng.getComponent(element).constructor.ɵcmp + // `consts` may be a thunk when the template has i18n initializers — its + // `.length` would be the function's arity, not the entry count. + const consts = typeof def.consts === 'function' ? def.consts() : def.consts return { decls: def.decls as number, vars: def.vars as number, - consts: (def.consts?.length ?? null) as number | null, + consts: (consts?.length ?? null) as number | null, ngContentSelectors: (def.ngContentSelectors ?? null) as string[] | null, + styles: (def.styles ?? null) as string[] | null, outputs: { ...def.outputs } as Record, inputs: Object.fromEntries( Object.entries(def.inputs as Record).map(([name, value]) => [ @@ -46,16 +50,25 @@ async function definition(page: Page, selector: string) { test.describe('HMR update definition', () => { let errors: string[] - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page, hmrDetector }) => { errors = [] page.on('pageerror', (error) => errors.push(error.message)) page.on('console', (message) => { if (message.type() === 'error') errors.push(message.text()) }) + // The DOM sentinel alone cannot prove no reload was requested: Vite's + // client drops a `full-reload` payload whose `.html` path does not match + // `location.pathname`, so the sentinel survives it. Record the wire. + await hmrDetector.captureWirePayloads() await page.goto('/') await page.waitForLoadState('networkidle') }) + test.afterEach(async ({ hmrDetector }) => { + const payloads = await hmrDetector.getWirePayloads() + expect(payloads.map((p) => p.type)).not.toContain('full-reload') + }) + test('a template that gains an element and a binding, then loses both', async ({ page, fileModifier, @@ -74,7 +87,13 @@ test.describe('HMR update definition', () => { await expect(page.locator('app-lab .lab-two')).toHaveText('LAB_A') await expect(page.locator('app-lab .lab-three')).toHaveAttribute('title', 'LAB_B') await expect(page.locator('app-lab .lab-one')).toHaveText('one') - expect(await definition(page, 'app-lab')).toMatchObject({ decls: 6, vars: 2 }) + const grown = await definition(page, 'app-lab') + expect(grown).toMatchObject({ decls: 6, vars: 2 }) + // The update module carries the component's styles into the merged + // definition — the spread alone would keep them, but an update module + // that regressed to emitting none would drop them. + expect(grown.styles?.join('')).toContain('.lab-one') + await expect(page.locator('app-lab .lab-one')).toHaveCSS('font-weight', '700') // A second swap on the same component, down to less than it started with. await page.waitForTimeout(WATCHER_THROTTLE_MS) diff --git a/napi/angular-compiler/index.d.ts b/napi/angular-compiler/index.d.ts index 52666dc58..7a37a173a 100644 --- a/napi/angular-compiler/index.d.ts +++ b/napi/angular-compiler/index.d.ts @@ -498,12 +498,23 @@ export interface FactoryNapiCompileResult { * This generates a JavaScript module that can be dynamically imported * during HMR to update a component's template and styles. * + * This knows only what it is given: the generated module replaces `template` + * and `consts`, but `decls`, `vars` and `ngContentSelectors` stay as the live + * definition has them, so it is only correct when the new template needs the + * same ones. For a template that changes them, use `compileForHmrSync`, which + * compiles the template and writes every field it decides. + * * # Arguments * * * `component_id` - The component ID (path@ClassName) - * * `template_js` - The compiled template function as JavaScript + * * `template_js` - The compiled template function as JavaScript, or an empty + * string for a styles-only update * * `styles` - The component's CSS styles, or `None` when unknown. An empty * array is definitive and emits `styles: []`, clearing the old styles. + * * `declarations_js` - Optional constant declarations the template references + * * `consts_js` - The new template's consts array as JavaScript. When + * `template_js` is given, `None` emits `consts: null`, clearing the old + * template's consts. * * # Returns * diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index dcae33a1c..a7779bfa6 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -533,12 +533,23 @@ pub fn compile_template( /// This generates a JavaScript module that can be dynamically imported /// during HMR to update a component's template and styles. /// +/// This knows only what it is given: the generated module replaces `template` +/// and `consts`, but `decls`, `vars` and `ngContentSelectors` stay as the live +/// definition has them, so it is only correct when the new template needs the +/// same ones. For a template that changes them, use `compileForHmrSync`, which +/// compiles the template and writes every field it decides. +/// /// # Arguments /// /// * `component_id` - The component ID (path@ClassName) -/// * `template_js` - The compiled template function as JavaScript +/// * `template_js` - The compiled template function as JavaScript, or an empty +/// string for a styles-only update /// * `styles` - The component's CSS styles, or `None` when unknown. An empty /// array is definitive and emits `styles: []`, clearing the old styles. +/// * `declarations_js` - Optional constant declarations the template references +/// * `consts_js` - The new template's consts array as JavaScript. When +/// `template_js` is given, `None` emits `consts: null`, clearing the old +/// template's consts. /// /// # Returns /// @@ -634,14 +645,18 @@ pub fn compile_for_hmr_sync( let mut all_styles: Vec = styles.unwrap_or_default(); all_styles.extend(output.styles); - // Apply style encapsulation for ViewEncapsulation.Emulated - // Angular uses %COMP% as a placeholder that the runtime replaces with the component ID + // Apply style encapsulation. Angular uses %COMP% as a placeholder + // that the runtime replaces with the component ID. Only Emulated + // encapsulation shims selectors; None and ShadowDom styles are + // served as written (the caller passes the component's + // encapsulation through `options.encapsulation`). + let emulate = opts.encapsulation.unwrap_or_default() == RustViewEncapsulation::Emulated; let encapsulated: Vec = all_styles .iter() .map(|style| { oxc_angular_compiler::styles::finalize_component_style( style, - true, + emulate, "_ngcontent-%COMP%", "_nghost-%COMP%", opts.minify_component_styles, @@ -674,7 +689,15 @@ pub fn compile_for_hmr_sync( declarations_js, }); - HmrCompileResult { hmr_module, component_id, template_js, errors: vec![] } + HmrCompileResult { + hmr_module, + component_id, + template_js, + // Non-fatal pipeline diagnostics (warnings, advice) — the + // update module was still generated. Errors come back through + // the `Err` arm instead. + errors: OxcError::from_diagnostics(&file_path, &template, output.diagnostics), + } } Err(diagnostics) => HmrCompileResult { hmr_module: String::new(), diff --git a/napi/angular-compiler/test/hmr-update-module.test.ts b/napi/angular-compiler/test/hmr-update-module.test.ts index 3732ef2b1..5f6147e97 100644 --- a/napi/angular-compiler/test/hmr-update-module.test.ts +++ b/napi/angular-compiler/test/hmr-update-module.test.ts @@ -74,13 +74,31 @@ async function loadUpdateModule(code: string): Promise<(...args: unknown[]) => v return (await import(/* @vite-ignore */ pathToFileURL(file).href)).default } +/** + * A template function's identity for comparison: its source text. A function + * compiled as a nested function expression carries a trailing `;` that a + * declaration does not, and that says nothing about the template. + */ +function templateSource(fn: unknown): string { + return String(fn).replace(/};(\s|$)/g, '}$1') +} + /** Hot-swaps `template` into `type` the way the runtime does. */ -async function swap(type: any, template: string, id = '/x/a.ts'): Promise { +async function swap(type: any, template: string, id = '/x/a.ts') { const result = compileForHmrSync(template, type.name, id, null, {}) expect(result.errors).toEqual([]) + // Seed a stale view so `def.tView` can only end up null if the update + // module wrote `tView: null` — an unrendered component's tView is already + // null, which would let a missing clear slip through. The sentinel is + // never a real view's, so ɵɵreplaceMetadata finds nothing to recreate. + type.ɵcmp.tView ??= {} const applyMetadata = await loadUpdateModule(result.hmrModule) ;(core as any).ɵɵreplaceMetadata(type, applyMetadata, [core], []) - return result.hmrModule + // A compiled function's source is what Function#toString returns, so this + // asserts the definition's template is the freshly compiled one, not a + // stale function the spread kept. + expect(templateSource(type.ɵcmp.template)).toBe(templateSource(result.templateJs)) + return result } /** The fields of a definition that its template decides. */ @@ -125,17 +143,17 @@ async function expectSwapMatchesFreshCompile( describe('compileForHmrSync update module', () => { it('writes the slot counts and projection selectors of the new template', async () => { const type = await component('', { members: `a = 'A';` }) - const module = await swap(type, '{{ a }}') + const { hmrModule } = await swap(type, '{{ a }}') - expect(templateFields(type)).toEqual({ + expect(templateFields(type)).toMatchObject({ decls: 4, vars: 1, consts: null, ngContentSelectors: ['[x]'], }) // Every constant the module declares is one it uses. - for (const [, name] of module.matchAll(/const (_c\d+) =/g)) { - expect(module.split(name).length - 1, `${name} is declared and used`).toBeGreaterThan(1) + for (const [, name] of hmrModule.matchAll(/const (_c\d+) =/g)) { + expect(hmrModule.split(name).length - 1, `${name} is declared and used`).toBeGreaterThan(1) } }) diff --git a/napi/angular-compiler/test/ssr-hmr.test.ts b/napi/angular-compiler/test/ssr-hmr.test.ts index a873f71aa..bdbfd8197 100644 --- a/napi/angular-compiler/test/ssr-hmr.test.ts +++ b/napi/angular-compiler/test/ssr-hmr.test.ts @@ -135,7 +135,10 @@ describe('compileForHmrSync with @ in the file path', () => { }) it('generateHmrModule names the update function after the class', () => { - const hmrModule = generateHmrModule('node_modules/@scope/pkg/src/a.ts@Foo', 'null') + const hmrModule = generateHmrModule( + 'node_modules/@scope/pkg/src/a.ts@Foo', + 'function Foo_Template(rf, ctx) {}', + ) expect(hmrModule).toContain('export default function Foo_UpdateMetadata(Foo, ɵɵnamespaces) {') expect(parseSync('hmr.js', hmrModule).errors).toEqual([]) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index 10972e2b4..d2787a257 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -828,9 +828,19 @@ export function angular(options: PluginOptions = {}): Plugin[] { const result = compileForHmrSync(templateContent, className, resolvedId, styles, { angularVersion: pluginOptions.angularVersion, + encapsulation: classMetadata.encapsulation, minifyComponentStyles: getMinifyComponentStyles(), }) + // A failed compile must not be served as an (empty) update + // module — that would call `ɵɵreplaceMetadata` with no + // callback and throw inside the listener. Fall back to the + // `angular:invalidate` full-reload path in the catch below. + const hmrErrors = result.errors.filter((e) => e.severity === 'Error') + if (hmrErrors.length > 0) { + throw new Error(hmrErrors.map((e) => e.message).join('\n')) + } + // Only consume the pending slot once we have real content to // serve. If we deleted unconditionally and the file was // transiently empty (truncate phase of an atomic write on From ec1ae2328a061dc5048b8d5dea29fed1adf77c43 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sat, 10 Oct 2026 00:21:50 +0800 Subject: [PATCH 4/5] style: format hmr-update-module.test.ts with oxfmt --- napi/angular-compiler/test/hmr-update-module.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/napi/angular-compiler/test/hmr-update-module.test.ts b/napi/angular-compiler/test/hmr-update-module.test.ts index 5f6147e97..cf54fdc0f 100644 --- a/napi/angular-compiler/test/hmr-update-module.test.ts +++ b/napi/angular-compiler/test/hmr-update-module.test.ts @@ -143,7 +143,10 @@ async function expectSwapMatchesFreshCompile( describe('compileForHmrSync update module', () => { it('writes the slot counts and projection selectors of the new template', async () => { const type = await component('', { members: `a = 'A';` }) - const { hmrModule } = await swap(type, '{{ a }}') + const { hmrModule } = await swap( + type, + '{{ a }}', + ) expect(templateFields(type)).toMatchObject({ decls: 4, From 1083689c09224c78eb591ddfa4799075f892b9c3 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sat, 10 Oct 2026 00:51:59 +0800 Subject: [PATCH 5/5] fix(hmr): resolve listener diagnostics that surfaced as full reloads The fatal-diagnostics change exposed two latent false-positive diagnostics in the template-only HMR path, which CI's e2e suite turned into page reloads for any component using `$event` or a two-way binding: - resolve_names verifier: `$event` inside listener handler ops legitimately remains a LexicalRead until reify emits it as the handler parameter (upstream's resolveDollarEvent rewrites it to a ReadVarExpr before its verifier runs; this port defers to reify). Exempt `$event` reads visited with IN_CHILD_OPERATION so plain bindings like `{{ $event }}` still error as upstream does. - transform_two_way_binding_set: validate resolved target forms (ResolvedPropertyRead/ResolvedKeyedRead/ResolvedSafePropertyRead) alongside the raw-Ast ones, matching upstream's acceptance of o.ReadPropExpr / o.ReadKeyExpr / ir.ReadVariableExpr. A resolved `[(checked)]="ioChecked"` target no longer reports "Unsupported expression in two-way action binding". Verified: cargo test 3065/3065, vitest hmr suites 30/30, and the previously failing playwright specs pass locally (hmr-css, hmr-html, hmr-html-write-strategies 12/12, hmr-update-definition + hmr-input-component 10/10). --- .../src/pipeline/phases/resolve_names.rs | 30 +++++++++++++++---- .../phases/transform_two_way_binding_set.rs | 8 +++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs b/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs index 89d4534a5..1f9256812 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs @@ -17,10 +17,11 @@ use rustc_hash::FxHashMap; use crate::ast::expression::AngularExpression; use crate::ir::enums::SemanticVariableKind; use crate::ir::expression::{ - ContextExpr, IrExpression, ReadVariableExpr, ResolvedCallExpr, ResolvedKeyedReadExpr, - ResolvedPropertyReadExpr, ResolvedSafePropertyReadExpr, RestoreViewTarget, VisitorContextFlag, - transform_expressions_in_create_op, transform_expressions_in_update_op, - visit_expressions_in_create_op, visit_expressions_in_update_op, + ContextExpr, IrExpression, LexicalReadExpr, ReadVariableExpr, ResolvedCallExpr, + ResolvedKeyedReadExpr, ResolvedPropertyReadExpr, ResolvedSafePropertyReadExpr, + RestoreViewTarget, VisitorContextFlag, transform_expressions_in_create_op, + transform_expressions_in_update_op, visit_expressions_in_create_op, + visit_expressions_in_update_op, }; use crate::ir::ops::{CreateOp, UpdateOp, XrefId}; use crate::pipeline::compilation::{ComponentCompilationJob, HostBindingCompilationJob}; @@ -1551,13 +1552,27 @@ fn verify_no_lexical_reads_remain(job: &mut ComponentCompilationJob<'_>) { // Collect errors in a RefCell to allow mutation from within the Fn closure let errors: RefCell> = RefCell::new(Vec::new()); + // `$event` inside listener handlers (IN_CHILD_OPERATION) legitimately stays + // a LexicalRead until reify turns it into the handler parameter. Upstream's + // resolveDollarEvent rewrites it to a ReadVarExpr before this check, so its + // verifier never sees it; this port leaves the LexicalRead in place instead, + // so it is exempt here. + let is_exempt_event_param = |lexical: &LexicalReadExpr<'_>, + flags: VisitorContextFlag| + -> bool { + flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) && lexical.name.as_str() == "$event" + }; + for view in job.all_views() { // Check create ops for op in view.create.iter() { visit_expressions_in_create_op( op, - &|expr, _flags| { + &|expr, flags| { if let IrExpression::LexicalRead(lexical) = expr { + if is_exempt_event_param(lexical, flags) { + return; + } errors.borrow_mut().push( OxcDiagnostic::error(format!( "AssertionError: no lexical reads should remain, but found read of {}", @@ -1575,8 +1590,11 @@ fn verify_no_lexical_reads_remain(job: &mut ComponentCompilationJob<'_>) { for op in view.update.iter() { visit_expressions_in_update_op( op, - &|expr, _flags| { + &|expr, flags| { if let IrExpression::LexicalRead(lexical) = expr { + if is_exempt_event_param(lexical, flags) { + return; + } errors.borrow_mut().push( OxcDiagnostic::error(format!( "AssertionError: no lexical reads should remain, but found read of {}", diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/transform_two_way_binding_set.rs b/crates/oxc_angular_compiler/src/pipeline/phases/transform_two_way_binding_set.rs index 73e495d75..5fe690a66 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/transform_two_way_binding_set.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/transform_two_way_binding_set.rs @@ -114,6 +114,14 @@ fn validate_two_way_binding_target(expr: &IrExpression<'_>) -> Option None, // Other IR expression types are not valid for two-way binding _ => Some( OxcDiagnostic::error("Unsupported expression in two-way action binding")