honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler): incorrect context object being referenced from listener instructions inside embedded views (#42755) 

Currently unless a listener inside of an embedded view tries to reference something from the parent view, or if the reference is a local ref, we don't generate the view restoration instructions and we allow for the value to be picked up from the context object in the function parameters. The problem is that the listener is only run during creation mode and the context object may have been swapped out afterwards.

These changes fix the issue by always generating the view restoration instructions for listeners inside templates.

Fixes #42698.

PR Close #42755
This commit is contained in:
Kristiyan Kostadinov 2021-07-03 11:59:58 +02:00 committed by Andrew Kushnir
parent 4c482bf3f1
commit 404c8d0d88
7 changed files with 148 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js
View File

@ -636,3 +636,51 @@ export declare class MyModule {
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}
/****************************************************************************************************
* 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: `
<ng-template let-obj>
<button (click)="obj.value = 1">Change</button>
</ng-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: `
<ng-template let-obj>
<button (click)="obj.value = 1">Change</button>
</ng-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<MyComponent, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never>;
}
export declare class MyModule {
static ɵfac: i0.ɵɵFactoryDeclaration<MyModule, never>;
static ɵmod: i0.ɵɵNgModuleDeclaration<MyModule, [typeof MyComponent], never, never>;
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}

17
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json
View File

@ -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"
}
]
}
]
}

16
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context.ts Normal file
View File

@ -0,0 +1,16 @@
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'my-component',
template: `
<ng-template let-obj>
<button (click)="obj.value = 1">Change</button>
</ng-template>
`
})
export class MyComponent {
}
@NgModule({declarations: [MyComponent]})
export class MyModule {
}

13
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/embedded_view_listener_context_template.js Normal file
View File

@ -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();
}
}

4
packages/compiler/src/compiler_util/expression_converter.ts
View File

@ -19,7 +19,7 @@ export interface LocalResolver {
getLocal(name: string): o.Expression|null;
notifyImplicitReceiverUse(): void;
globals?: Set<string>;
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));

27
packages/compiler/src/render3/view/template.ts
View File

@ -340,8 +340,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, 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());

41
packages/core/test/acceptance/listener_spec.ts
View File

@ -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: `
<ng-template let-obj #template>
<button (click)="obj.value = obj.value + '!'">Change</button>
</ng-template>
<ng-container *ngTemplateOutlet="template; context: {$implicit: current}"></ng-container>
`
})
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!');
});
});