fix(compiler): preserve this.$event and this.$any accesses in expressions (#39323)
Currently expressions `$event.foo()` and `this.$event.foo()`, as well as `$any(foo)` and `this.$any(foo)`, are treated as the same expression by the compiler, because `this` is considered the same implicit receiver as when the receiver is omitted. This introduces the following issues: 1. Any time something called `$any` is used, it'll be stripped away, leaving only the first parameter. 2. If something called `$event` is used anywhere in a template, it'll be preserved as `$event`, rather than being rewritten to `ctx.$event`, causing the value to undefined at runtime. This applies to listener, property and text bindings. These changes resolve the first issue and part of the second one by preserving anything that is accessed through `this`, even if it's one of the "special" ones like `$any` or `$event`. Furthermore, these changes only expose the `$event` global variable inside event listeners, whereas previously it was available everywhere. Fixes #30278. PR Close #39323
This commit is contained in:
committed by
Joey Perrott
parent
4e68254514
commit
cbc0907bfd
@@ -6,7 +6,7 @@
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, Unary} from '@angular/compiler';
|
||||
import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '@angular/compiler';
|
||||
import * as ts from 'typescript';
|
||||
import {TypeCheckingConfig} from '../api';
|
||||
|
||||
@@ -137,6 +137,10 @@ class AstTranslator implements AstVisitor {
|
||||
throw new Error('Method not implemented.');
|
||||
}
|
||||
|
||||
visitThisReceiver(ast: ThisReceiver): never {
|
||||
throw new Error('Method not implemented.');
|
||||
}
|
||||
|
||||
visitInterpolation(ast: Interpolation): ts.Expression {
|
||||
// Build up a chain of binary + operations to simulate the string concatenation of the
|
||||
// interpolation's expressions. The chain is started using an actual string literal to ensure
|
||||
@@ -363,6 +367,9 @@ class VeSafeLhsInferenceBugDetector implements AstVisitor {
|
||||
visitImplicitReceiver(ast: ImplicitReceiver): boolean {
|
||||
return false;
|
||||
}
|
||||
visitThisReceiver(ast: ThisReceiver): boolean {
|
||||
return false;
|
||||
}
|
||||
visitInterpolation(ast: Interpolation): boolean {
|
||||
return ast.expressions.some(exp => exp.visit(this));
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
|
||||
import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
|
||||
import * as ts from 'typescript';
|
||||
|
||||
import {Reference} from '../../imports';
|
||||
@@ -1586,7 +1586,9 @@ class TcbExpressionTranslator {
|
||||
const result = tsCallMethod(pipe, 'transform', [expr, ...args]);
|
||||
addParseSpanInfo(result, ast.sourceSpan);
|
||||
return result;
|
||||
} else if (ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver) {
|
||||
} else if (
|
||||
ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver &&
|
||||
!(ast.receiver instanceof ThisReceiver)) {
|
||||
// Resolve the special `$any(expr)` syntax to insert a cast of the argument to type `any`.
|
||||
// `$any(expr)` -> `expr as any`
|
||||
if (ast.name === '$any' && ast.args.length === 1) {
|
||||
@@ -1843,7 +1845,7 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator {
|
||||
// function that the converted expression becomes a child of, just create a reference to the
|
||||
// parameter by its name.
|
||||
if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver &&
|
||||
ast.name === EVENT_PARAMETER) {
|
||||
!(ast.receiver instanceof ThisReceiver) && ast.name === EVENT_PARAMETER) {
|
||||
const event = ts.createIdentifier(EVENT_PARAMETER);
|
||||
addParseSpanInfo(event, ast.nameSpan);
|
||||
return event;
|
||||
|
||||
@@ -584,6 +584,12 @@ describe('type check blocks', () => {
|
||||
expect(block).toContain('(((ctx).a) as any)');
|
||||
});
|
||||
|
||||
it('should handle $any accessed through `this`', () => {
|
||||
const TEMPLATE = `{{this.$any(a)}}`;
|
||||
const block = tcb(TEMPLATE);
|
||||
expect(block).toContain('((ctx).$any(((ctx).a)))');
|
||||
});
|
||||
|
||||
describe('experimental DOM checking via lib.dom.d.ts', () => {
|
||||
it('should translate unclaimed bindings to their property equivalent', () => {
|
||||
const TEMPLATE = `<label [for]="'test'"></label>`;
|
||||
@@ -684,6 +690,14 @@ describe('type check blocks', () => {
|
||||
expect(block).toContain(
|
||||
'_t3.addEventListener("event", function ($event): any { (_t2 = 3); });');
|
||||
});
|
||||
|
||||
it('should ignore accesses to $event through `this`', () => {
|
||||
const TEMPLATE = `<div (event)="foo(this.$event)"></div>`;
|
||||
const block = tcb(TEMPLATE);
|
||||
|
||||
expect(block).toContain(
|
||||
'_t1.addEventListener("event", function ($event): any { (ctx).foo(((ctx).$event)); });');
|
||||
});
|
||||
});
|
||||
|
||||
describe('config', () => {
|
||||
|
||||
@@ -447,4 +447,137 @@ describe('compiler compliance: listen()', () => {
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect host bindings');
|
||||
});
|
||||
|
||||
it('should assume $event is referring to the event variable in a listener by default', () => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: '<div (click)="c($event)"></div>'
|
||||
})
|
||||
class Comp {
|
||||
c(event: MouseEvent) {}
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵlistener("click", function Comp_Template_div_click_0_listener($event) { return ctx.c($event); });
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect event listener');
|
||||
});
|
||||
|
||||
it('should preserve accesses to $event if it is done through `this` in a listener', () => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: '<div (click)="c(this.$event)"></div>'
|
||||
})
|
||||
class Comp {
|
||||
$event = {};
|
||||
c(value: {}) {}
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵlistener("click", function Comp_Template_div_click_0_listener() { return ctx.c(ctx.$event); });
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect event listener');
|
||||
});
|
||||
|
||||
it('should not assume that $event is referring to an event object inside a property', () => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: '<div [event]="$event"></div>'
|
||||
})
|
||||
class Comp {
|
||||
$event = 1;
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵproperty("event", ctx.$event);
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect property binding');
|
||||
});
|
||||
|
||||
it('should assume $event is referring to the event variable in a listener by default inside a host binding',
|
||||
() => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Directive} from '@angular/core';
|
||||
|
||||
@Directive({
|
||||
host: {
|
||||
'(click)': 'c($event)'
|
||||
}
|
||||
})
|
||||
class Dir {
|
||||
c(event: MouseEvent) {}
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵlistener("click", function Dir_click_HostBindingHandler($event) { return ctx.c($event); });
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect event listener');
|
||||
});
|
||||
|
||||
it('should preserve accesses to $event if it is done through `this` in a listener inside a host binding',
|
||||
() => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Directive} from '@angular/core';
|
||||
|
||||
@Directive({
|
||||
host: {
|
||||
'(click)': 'c(this.$event)'
|
||||
}
|
||||
})
|
||||
class Dir {
|
||||
$event = {};
|
||||
c(value: {}) {}
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵlistener("click", function Dir_click_HostBindingHandler() { return ctx.c(ctx.$event); });
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect event listener');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -191,4 +191,57 @@ describe('r3_view_compiler', () => {
|
||||
expectEmit(result.source, template, 'Incorrect initialization attributes');
|
||||
});
|
||||
});
|
||||
|
||||
describe('$any', () => {
|
||||
it('should strip out $any wrappers', () => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: '<div [tabIndex]="$any(10)"></div>'
|
||||
})
|
||||
class Comp {
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵproperty("tabIndex", 10);
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect template');
|
||||
});
|
||||
|
||||
it('should preserve $any if it is accessed through `this`', () => {
|
||||
const files = {
|
||||
app: {
|
||||
'spec.ts': `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: '<div [tabIndex]="this.$any(null)"></div>'
|
||||
})
|
||||
class Comp {
|
||||
$any(value: null): any {
|
||||
return value as any;
|
||||
}
|
||||
}
|
||||
`
|
||||
}
|
||||
};
|
||||
|
||||
const template = `
|
||||
…
|
||||
i0.ɵɵproperty("tabIndex", ctx.$any(null));
|
||||
`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
expectEmit(result.source, template, 'Incorrect template');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user