From ff097ffff5bce5c742f185c29abd19812b401e12 Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Thu, 7 Dec 2023 11:20:34 -0800 Subject: [PATCH] refactor(compiler): Add failing tests about structural attr bindings While running a g3 presubmit, I discovered two related novel failure modes: 1. Simple case: this new test uses an `ngFor` structural directive, which binds a context variable. That variable is immediately used in an attribute binding. It looks like we generate an extra attribute instruction, which might result in an invalid property read at runtime. 2. Complex case: this is another attribute binding, this time on a structural element, inside of an `ng-template`. Not sure what's going on here. --- .../GOLDEN_PARTIAL.js | 86 +++++++++++++++++++ .../r3_view_compiler_template/TEST_CASES.json | 24 ++++++ ...inding_on_structural_inside_ng_template.js | 24 ++++++ ...inding_on_structural_inside_ng_template.ts | 15 ++++ .../ng_for_context_in_attr_binding.js | 18 ++++ .../ng_for_context_in_attr_binding.ts | 17 ++++ 6 files changed, 184 insertions(+) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js index 2b0a89960d61ad..fb5827221ff9de 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js @@ -1112,3 +1112,89 @@ export declare class MyComponent { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: ng_for_context_in_attr_binding.js + ****************************************************************************************************/ +import { Component, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { +} +MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: ` +
+
+`, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ + type: Component, + args: [{ + selector: 'my-component', + template: ` +
+
+`, + }] + }] }); +export class MyModule { +} +MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule }); +MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [MyComponent] }); +MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{ + type: NgModule, + args: [{ declarations: [MyComponent] }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: ng_for_context_in_attr_binding.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + someField: any; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class MyModule { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} + +/**************************************************************************************************** + * PARTIAL FILE: attr_binding_on_structural_inside_ng_template.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { +} +MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-component", ngImport: i0, template: ` + + + +`, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ + type: Component, + args: [{ + selector: 'my-component', + standalone: true, + template: ` + + + +`, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: attr_binding_on_structural_inside_ng_template.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + someField: any; + someBooleanField: boolean; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json index 3ed7b79e3e8574..deb9c3cc36c76e 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json @@ -356,6 +356,30 @@ "failureMessage": "Incorrect template" } ] + }, + { + "description": "should handle ngFor context variables when used in bindings", + "inputFiles": [ + "ng_for_context_in_attr_binding.ts" + ], + "expectations": [ + { + "failureMessage": "Incorrect template" + } + ], + "skipForTemplatePipeline": true + }, + { + "description": "should handle attribute bindings inside an ng-template", + "inputFiles": [ + "attr_binding_on_structural_inside_ng_template.ts" + ], + "expectations": [ + { + "failureMessage": "Incorrect template" + } + ], + "skipForTemplatePipeline": true } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.js new file mode 100644 index 00000000000000..4c23be0f680110 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.js @@ -0,0 +1,24 @@ +function MyComponent_ng_template_0_span_0_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵelement(0, "span"); + } if (rf & 2) { + const $ctx_r2$ = i0.ɵɵnextContext(2); + i0.ɵɵattribute("someAttr", $ctx_r2$.someField); + } +} + +function MyComponent_ng_template_0_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵtemplate(0, MyComponent_ng_template_0_span_0_Template, 1, 1, "span", 1); + } if (rf & 2) { + const $ctx_r0$ = i0.ɵɵnextContext(); + i0.ɵɵproperty("ngIf", $ctx_r0$.someBooleanField); + } +} + +… +consts: [["someLocalRef", ""], [4, "ngIf"]], template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 1, 1, "ng-template", null, 0, i0.ɵɵtemplateRefExtractor); + } +} \ No newline at end of file diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.ts new file mode 100644 index 00000000000000..f56ab2c4685a7a --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/attr_binding_on_structural_inside_ng_template.ts @@ -0,0 +1,15 @@ +import {Component, NgModule} from '@angular/core'; + +@Component({ + selector: 'my-component', + standalone: true, + template: ` + + + +`, +}) +export class MyComponent { + someField!: any; + someBooleanField!: boolean; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.js new file mode 100644 index 00000000000000..062810902cdd3a --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.js @@ -0,0 +1,18 @@ +MyComponent_div_0_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵelement(0, "div"); + } if (rf & 2) { + const $someElem_r1$ = ctx.$implicit; + i0.ɵɵattribute("someInputAttr", $someElem_r1$.someAttr()); + } +} + +… +consts: [[__AttributeMarker.Template__, "ngFor", "ngForOf"]], +template:function MyComponent_Template(rf, ctx){ + if (rf & 1) { + i0.ɵɵtemplate(0, MyComponent_div_0_Template, 1, 1, "div", 0); + } if (rf & 2) { + i0.ɵɵproperty("ngForOf", ctx.someField.someMethod()); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.ts new file mode 100644 index 00000000000000..225f9cee781f65 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_for_context_in_attr_binding.ts @@ -0,0 +1,17 @@ +import {Component, NgModule} from '@angular/core'; + +@Component({ + selector: 'my-component', + template: ` +
+
+`, +}) +export class MyComponent { + someField!: any; +} + +@NgModule({declarations: [MyComponent]}) +export class MyModule { +}