From f52df99fe369854c4ee49e3c01afa29cf2ae7126 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 19 Jun 2021 10:59:00 +0200 Subject: [PATCH] fix(compiler): generate view restoration for keyed write inside template listener (#42603) If an implcit receiver is accessed in a listener inside of an `ng-template`, we generate some extra code in order to ensure that we're assigning to the correct object. The problem is that the logic wasn't covering keyed writes which caused it to write to the wrong object and throw an assertion error at runtime. These changes expand the logic to cover keyed writes. Fixes #41267. PR Close #42603 --- .../GOLDEN_PARTIAL.js | 52 +++++++++++++++++++ .../r3_view_compiler_listener/TEST_CASES.json | 17 ++++++ ...it_receiver_keyed_write_inside_template.ts | 17 ++++++ ...er_keyed_write_inside_template_template.js | 13 +++++ .../src/compiler_util/expression_converter.ts | 7 +++ .../compiler/src/render3/view/template.ts | 5 ++ .../src/view_compiler/type_check_compiler.ts | 3 ++ .../src/view_compiler/view_compiler.ts | 6 ++- .../core/test/acceptance/listener_spec.ts | 24 +++++++++ 9 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template_template.js diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js index 3125914021..b2ce2b31a0 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js @@ -584,3 +584,55 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE ****************************************************************************************************/ export {}; +/**************************************************************************************************** + * PARTIAL FILE: implicit_receiver_keyed_write_inside_template.js + ****************************************************************************************************/ +import { Component, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { + constructor() { + this.message = ''; + } +} +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: "12.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: "12.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: implicit_receiver_keyed_write_inside_template.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + message: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class MyModule { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json index 7a46d6f143..992de375c8 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json @@ -264,6 +264,23 @@ "failureMessage": "Incorrect event listener" } ] + }, + { + "description": "should generate the view restoration statements if a keyed write is used in an event listener from within an ng-template", + "inputFiles": [ + "implicit_receiver_keyed_write_inside_template.ts" + ], + "expectations": [ + { + "files": [ + { + "expected": "implicit_receiver_keyed_write_inside_template_template.js", + "generated": "implicit_receiver_keyed_write_inside_template.js" + } + ], + "failureMessage": "Incorrect template" + } + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template.ts new file mode 100644 index 0000000000..63ff6a62cd --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template.ts @@ -0,0 +1,17 @@ +import {Component, NgModule} from '@angular/core'; + +@Component({ + selector: 'my-component', + template: ` + + + + ` +}) +export class MyComponent { + message = ''; +} + +@NgModule({declarations: [MyComponent]}) +export class MyModule { +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template_template.js new file mode 100644 index 0000000000..bac6f51471 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/implicit_receiver_keyed_write_inside_template_template.js @@ -0,0 +1,13 @@ +function MyComponent_ng_template_0_Template(rf, $ctx$) { + if (rf & 1) { + const _r3 = $i0$.ɵɵgetCurrentView(); + $i0$.ɵɵelementStart(0, "button", 1); + $i0$.ɵɵlistener("click", function MyComponent_ng_template_0_Template_button_click_0_listener() { + $i0$.ɵɵrestoreView(_r3); + const $ctx_2$ = $i0$.ɵɵnextContext(); + return ($ctx_2$["mes" + "sage"] = "hello"); + }); + $i0$.ɵɵtext(1, "Click me"); + $i0$.ɵɵelementEnd(); + } +} diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index 476651660f..cd0b50f4b4 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -19,6 +19,7 @@ export interface LocalResolver { getLocal(name: string): o.Expression|null; notifyImplicitReceiverUse(): void; globals?: Set; + maybeRestoreView(retrievalLevel: number, localRefLookup: boolean): void; } export class ConvertActionBindingResult { @@ -487,6 +488,11 @@ class _AstToIrVisitor implements cdAst.AstVisitor { const obj: o.Expression = this._visit(ast.receiver, _Mode.Expression); const key: o.Expression = this._visit(ast.key, _Mode.Expression); const value: o.Expression = this._visit(ast.value, _Mode.Expression); + + if (obj === this._implicitReceiver) { + this._localResolver.maybeRestoreView(0, false); + } + return convertToStatementIfNeeded(mode, obj.key(key).set(value)); } @@ -982,6 +988,7 @@ function flattenStatements(arg: any, output: o.Statement[]) { class DefaultLocalResolver implements LocalResolver { constructor(public globals?: Set) {} notifyImplicitReceiverUse(): void {} + maybeRestoreView(): void {} getLocal(name: string): o.Expression|null { if (name === EventHandlerVars.event.name) { return EventHandlerVars.event; diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 686b93ceeb..e4ed819a7f 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -339,6 +339,11 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this._bindingScope.notifyImplicitReceiverUse(); } + // LocalResolver + maybeRestoreView(retrievalLevel: number, localRefLookup: boolean): void { + this._bindingScope.maybeRestoreView(retrievalLevel, localRefLookup); + } + private i18nTranslate( message: i18n.Message, params: {[name: string]: o.Expression} = {}, ref?: o.ReadVarExpr, transformFn?: (raw: o.ReadVarExpr) => o.Expression): o.ReadVarExpr { diff --git a/packages/compiler/src/view_compiler/type_check_compiler.ts b/packages/compiler/src/view_compiler/type_check_compiler.ts index 6eaa3f6552..9518b0a77d 100644 --- a/packages/compiler/src/view_compiler/type_check_compiler.ts +++ b/packages/compiler/src/view_compiler/type_check_compiler.ts @@ -78,6 +78,7 @@ const DYNAMIC_VAR_NAME = '_any'; class TypeCheckLocalResolver implements LocalResolver { notifyImplicitReceiverUse(): void {} + maybeRestoreView(): void {} getLocal(name: string): o.Expression|null { if (name === EventHandlerVars.event.name) { // References to the event should not be type-checked. @@ -290,6 +291,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { } notifyImplicitReceiverUse(): void {} + maybeRestoreView(): void {} + getLocal(name: string): o.Expression|null { if (name == EventHandlerVars.event.name) { return o.variable(this.getOutputVar(o.BuiltinTypeName.Dynamic)); diff --git a/packages/compiler/src/view_compiler/view_compiler.ts b/packages/compiler/src/view_compiler/view_compiler.ts index b8c9f76c80..e8c4ef6ba9 100644 --- a/packages/compiler/src/view_compiler/view_compiler.ts +++ b/packages/compiler/src/view_compiler/view_compiler.ts @@ -680,11 +680,15 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { } notifyImplicitReceiverUse(): void { - // Not needed in View Engine as View Engine walks through the generated + // Not needed in ViewEngine as ViewEngine walks through the generated // expressions to figure out if the implicit receiver is used and needs // to be generated as part of the pre-update statements. } + maybeRestoreView(): void { + // Not necessary in ViewEngine, because view restoration is an Ivy concept. + } + private _createLiteralArrayConverter(sourceSpan: ParseSourceSpan, argCount: number): BuiltinConverter { if (argCount === 0) { diff --git a/packages/core/test/acceptance/listener_spec.ts b/packages/core/test/acceptance/listener_spec.ts index 51bf82f4ff..118cbb84ce 100644 --- a/packages/core/test/acceptance/listener_spec.ts +++ b/packages/core/test/acceptance/listener_spec.ts @@ -528,4 +528,28 @@ describe('event listeners', () => { expect(eventVariable).toBe(10); expect(eventObject?.type).toBe('click'); }); + + it('should be able to use a keyed write on `this` from a listener inside an ng-template', () => { + @Component({ + template: ` + + + + + + ` + }) + class MyComp { + message = ''; + } + + TestBed.configureTestingModule({declarations: [MyComp], imports: [CommonModule]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + const button = fixture.nativeElement.querySelector('button'); + button.click(); + fixture.detectChanges(); + + expect(fixture.componentInstance.message).toBe('hello'); + }); });