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 b2ce2b31a0..5e698d0a5f 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 @@ -636,3 +636,51 @@ export declare class MyModule { static ɵinj: i0.ɵɵInjectorDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: embedded_view_listener_context.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: "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: embedded_view_listener_context.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + 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 992de375c8..52cce67833 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 @@ -281,6 +281,23 @@ "failureMessage": "Incorrect template" } ] + }, + { + "description": "should reference correct context in listener inside embedded view", + "inputFiles": [ + "embedded_view_listener_context.ts" + ], + "expectations": [ + { + "files": [ + { + "expected": "embedded_view_listener_context_template.js", + "generated": "embedded_view_listener_context.js" + } + ], + "failureMessage": "Incorrect template" + } + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context.ts new file mode 100644 index 0000000000..72876362f8 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context.ts @@ -0,0 +1,16 @@ +import {Component, NgModule} from '@angular/core'; + +@Component({ + selector: 'my-component', + template: ` + + + + ` +}) +export class MyComponent { +} + +@NgModule({declarations: [MyComponent]}) +export class MyModule { +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context_template.js new file mode 100644 index 0000000000..f903938b9c --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context_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", 0); + $i0$.ɵɵlistener("click", function MyComponent_ng_template_0_Template_button_click_0_listener() { + const restoredCtx = $i0$.ɵɵrestoreView(_r3); + const $obj_r1$ = restoredCtx.$implicit; + return $obj_r1$.value = 1; + }); + $i0$.ɵɵtext(1, "Change"); + $i0$.ɵɵelementEnd(); + } +} diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index cd0b50f4b4..6911c6d981 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -19,7 +19,7 @@ export interface LocalResolver { getLocal(name: string): o.Expression|null; notifyImplicitReceiverUse(): void; globals?: Set; - maybeRestoreView(retrievalLevel: number, localRefLookup: boolean): void; + maybeRestoreView(): void; } export class ConvertActionBindingResult { @@ -490,7 +490,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor { const value: o.Expression = this._visit(ast.value, _Mode.Expression); if (obj === this._implicitReceiver) { - this._localResolver.maybeRestoreView(0, false); + this._localResolver.maybeRestoreView(); } return convertToStatementIfNeeded(mode, obj.key(key).set(value)); diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index e4ed819a7f..9f5d1b6191 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -340,8 +340,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } // LocalResolver - maybeRestoreView(retrievalLevel: number, localRefLookup: boolean): void { - this._bindingScope.maybeRestoreView(retrievalLevel, localRefLookup); + maybeRestoreView(): void { + this._bindingScope.maybeRestoreView(); } private i18nTranslate( @@ -1656,7 +1656,6 @@ const SHARED_CONTEXT_KEY = '$$shared_ctx$$'; type BindingData = { retrievalLevel: number; lhs: o.Expression; declareLocalCallback?: DeclareLocalVarCallback; declare: boolean; priority: number; - localRef: boolean; }; /** @@ -1701,15 +1700,14 @@ export class BindingScope implements LocalResolver { lhs: value.lhs, declareLocalCallback: value.declareLocalCallback, declare: false, - priority: value.priority, - localRef: value.localRef + priority: value.priority }; // Cache the value locally. this.map.set(name, value); // Possibly generate a shared context var this.maybeGenerateSharedContextVar(value); - this.maybeRestoreView(value.retrievalLevel, value.localRef); + this.maybeRestoreView(); } if (value.declareLocalCallback && !value.declare) { @@ -1754,7 +1752,6 @@ export class BindingScope implements LocalResolver { declare: false, declareLocalCallback: declareLocalCallback, priority: priority, - localRef: localRef || false }); return this; } @@ -1823,24 +1820,22 @@ export class BindingScope implements LocalResolver { }, declare: false, priority: DeclarationPriority.SHARED_CONTEXT, - localRef: false }); } getComponentProperty(name: string): o.Expression { const componentValue = this.map.get(SHARED_CONTEXT_KEY + 0)!; componentValue.declare = true; - this.maybeRestoreView(0, false); + this.maybeRestoreView(); return componentValue.lhs.prop(name); } - maybeRestoreView(retrievalLevel: number, localRefLookup: boolean) { - // We want to restore the current view in listener fns if: - // 1 - we are accessing a value in a parent view, which requires walking the view tree rather - // than using the ctx arg. In this case, the retrieval and binding level will be different. - // 2 - we are looking up a local ref, which requires restoring the view where the local - // ref is stored - if (this.isListenerScope() && (retrievalLevel < this.bindingLevel || localRefLookup)) { + maybeRestoreView() { + // View restoration is required for listener instructions inside embedded views, because + // they only run in creation mode and they can have references to the context object. + // If the context object changes in update mode, the reference will be incorrect, because + // it was established during creation. + if (this.isListenerScope()) { if (!this.parent!.restoreViewVariable) { // parent saves variable to generate a shared `const $s$ = getCurrentView();` instruction this.parent!.restoreViewVariable = o.variable(this.parent!.freshReferenceName()); diff --git a/packages/core/test/acceptance/listener_spec.ts b/packages/core/test/acceptance/listener_spec.ts index 118cbb84ce..597d830d03 100644 --- a/packages/core/test/acceptance/listener_spec.ts +++ b/packages/core/test/acceptance/listener_spec.ts @@ -552,4 +552,45 @@ describe('event listeners', () => { expect(fixture.componentInstance.message).toBe('hello'); }); + + it('should reference the correct context object if it is swapped out', () => { + @Component({ + template: ` + + + + + + ` + }) + class MyComp { + one = {value: 'one'}; + two = {value: 'two'}; + current = this.one; + } + + TestBed.configureTestingModule({declarations: [MyComp], imports: [CommonModule]}); + const fixture = TestBed.createComponent(MyComp); + const instance = fixture.componentInstance; + fixture.detectChanges(); + const button = fixture.nativeElement.querySelector('button'); + + expect(instance.one.value).toBe('one'); + expect(instance.two.value).toBe('two'); + + button.click(); + fixture.detectChanges(); + + expect(instance.one.value).toBe('one!'); + expect(instance.two.value).toBe('two'); + + instance.current = instance.two; + fixture.detectChanges(); + + button.click(); + fixture.detectChanges(); + + expect(instance.one.value).toBe('one!'); + expect(instance.two.value).toBe('two!'); + }); });