feat(compiler): add support for shorthand property declarations in templates (#42421)

Adds support for shorthand property declarations inside Angular templates. E.g. doing `{foo, bar}` instead of `{foo: foo, bar: bar}`.

Fixes #10277.

PR Close #42421
This commit is contained in:
Kristiyan Kostadinov 2021-06-19 08:07:20 +02:00 committed by Dylan Hunn
parent 699a8b43cb
commit cc672f05bf
17 changed files with 390 additions and 10 deletions

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AST, ASTWithSource, BindingPipe, MethodCall, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
import {AST, ASTWithSource, BindingPipe, MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
import * as ts from 'typescript';
import {AbsoluteFsPath} from '../../file_system';
@ -482,8 +482,20 @@ export class SymbolBuilder {
expression.nameSpan :
expression.sourceSpan;
let node = findFirstMatchingNode(
this.typeCheckBlock, {withSpan, filter: (n: ts.Node): n is ts.Node => true});
let node: ts.Node|null = null;
// Property reads in templates usually map to a `PropertyAccessExpression`
// (e.g. `ctx.foo`) so try looking for one first.
if (expression instanceof PropertyRead) {
node = findFirstMatchingNode(
this.typeCheckBlock, {withSpan, filter: ts.isPropertyAccessExpression});
}
// Otherwise fall back to searching for any AST node.
if (node === null) {
node = findFirstMatchingNode(this.typeCheckBlock, {withSpan, filter: anyNodeFilter});
}
if (node === null) {
return null;
}
@ -560,3 +572,8 @@ export class SymbolBuilder {
}
}
}
/** Filter predicate function that matches any AST node. */
function anyNodeFilter(n: ts.Node): n is ts.Node {
return true;
}

View File

@ -462,6 +462,50 @@ class TestComponent {
`TestComponent.html(4, 18): Property 'heihgt' does not exist on type 'TestComponent'. Did you mean 'height'?`,
]);
});
it('works for shorthand property declarations', () => {
const messages = diagnose(
`<div dir [input]="{a, b: 2}"></div>`, `
class Dir {
input: {a: string, b: number};
}
class TestComponent {
a: number;
}`,
[{
type: 'directive',
name: 'Dir',
selector: '[dir]',
exportAs: ['dir'],
inputs: {input: 'input'},
}]);
expect(messages).toEqual(
[`TestComponent.html(1, 20): Type 'number' is not assignable to type 'string'.`]);
});
it('works for shorthand property declarations referring to template variables', () => {
const messages = diagnose(
`
<span #span></span>
<div dir [input]="{span, b: 2}"></div>
`,
`
class Dir {
input: {span: string, b: number};
}
class TestComponent {}`,
[{
type: 'directive',
name: 'Dir',
selector: '[dir]',
exportAs: ['dir'],
inputs: {input: 'input'},
}]);
expect(messages).toEqual(
[`TestComponent.html(3, 30): Type 'HTMLElement' is not assignable to type 'string'.`]);
});
});
describe('method call spans', () => {

View File

@ -42,6 +42,15 @@ describe('type check blocks diagnostics', () => {
'(ctx).m /*3,4*/({ "foo": ((ctx).a /*11,12*/) /*11,12*/, "bar": ((ctx).b /*19,20*/) /*19,20*/ } /*5,21*/) /*3,22*/');
});
it('should annotate literal map expressions with shorthand declarations', () => {
// The additional method call is present to avoid that the object literal is emitted as
// statement, which would wrap it into parenthesis that clutter the expected output.
const TEMPLATE = '{{ m({a, b}) }}';
expect(tcbWithSpans(TEMPLATE))
.toContain(
'((ctx).m /*3,4*/({ "a": ((ctx).a /*6,7*/) /*6,7*/, "b": ((ctx).b /*9,10*/) /*9,10*/ } /*5,11*/) /*3,12*/)');
});
it('should annotate literal array expressions', () => {
const TEMPLATE = '{{ [a, b] }}';
expect(tcbWithSpans(TEMPLATE))

View File

@ -668,12 +668,16 @@ runInEachFileSystem(() => {
const fileName = absoluteFrom('/main.ts');
const templateString = `
{{ [1, 2, 3] }}
{{ { hello: "world" } }}`;
{{ { hello: "world" } }}
{{ { foo } }}`;
const testValues = setup([
{
fileName,
templates: {'Cmp': templateString},
source: `export class Cmp {}`,
source: `
type Foo {name: string;}
export class Cmp {foo: Foo;}
`,
},
]);
templateTypeChecker = testValues.templateTypeChecker;
@ -701,6 +705,15 @@ runInEachFileSystem(() => {
expect(program.getTypeChecker().typeToString(symbol.tsType))
.toEqual('{ hello: string; }');
});
it('literal map shorthand property', () => {
const shorthandProp =
(interpolation.expressions[2] as LiteralMap).values[0] as PropertyRead;
const symbol = templateTypeChecker.getSymbolOfNode(shorthandProp, cmp)!;
assertExpressionSymbol(symbol);
expect(program.getTypeChecker().symbolToString(symbol.tsSymbol!)).toEqual('foo');
expect(program.getTypeChecker().typeToString(symbol.tsType)).toEqual('Foo');
});
});
describe('pipes', () => {

View File

@ -870,3 +870,54 @@ export declare class MyModule {
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}
/****************************************************************************************************
* PARTIAL FILE: shorthand_property_declaration.js
****************************************************************************************************/
import { Component, NgModule } from '@angular/core';
import * as i0 from "@angular/core";
export class MyComponent {
constructor() {
this.a = 1;
this.c = 3;
}
_handleClick(_value) { }
}
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: "ng-component", ngImport: i0, template: `
<div (click)="_handleClick({a, b: 2, c})"></div>
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
type: Component,
args: [{
template: `
<div (click)="_handleClick({a, b: 2, c})"></div>
`
}]
}] });
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: shorthand_property_declaration.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class MyComponent {
a: number;
c: number;
_handleClick(_value: any): void;
static ɵfac: i0.ɵɵFactoryDeclaration<MyComponent, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "ng-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>;
}

View File

@ -269,6 +269,23 @@
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should handle shorthand property declarations in templates",
"inputFiles": [
"shorthand_property_declaration.ts"
],
"expectations": [
{
"files": [
{
"expected": "shorthand_property_declaration_template.js",
"generated": "shorthand_property_declaration.js"
}
],
"failureMessage": "Incorrect template"
}
]
}
]
}

View File

@ -0,0 +1,16 @@
import {Component, NgModule} from '@angular/core';
@Component({
template: `
<div (click)="_handleClick({a, b: 2, c})"></div>
`
})
export class MyComponent {
a = 1;
c = 3;
_handleClick(_value: any) {}
}
@NgModule({declarations: [MyComponent]})
export class MyModule {
}

View File

@ -0,0 +1,13 @@
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
i0.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener() {
return ctx._handleClick({
a: ctx.a,
b: 2,
c: ctx.c
});
});
}
}

View File

@ -929,11 +929,23 @@ export class _ParseAST {
if (!this.consumeOptionalCharacter(chars.$RBRACE)) {
this.rbracesExpected++;
do {
const keyStart = this.inputIndex;
const quoted = this.next.isString();
const key = this.expectIdentifierOrKeywordOrString();
keys.push({key, quoted});
this.expectCharacter(chars.$COLON);
values.push(this.parsePipe());
// Properties with quoted keys can't use the shorthand syntax.
if (quoted) {
this.expectCharacter(chars.$COLON);
values.push(this.parsePipe());
} else if (this.consumeOptionalCharacter(chars.$COLON)) {
values.push(this.parsePipe());
} else {
const span = this.span(keyStart);
const sourceSpan = this.sourceSpan(keyStart);
values.push(new PropertyRead(
span, sourceSpan, sourceSpan, new ImplicitReceiver(span, sourceSpan), key));
}
} while (this.consumeOptionalCharacter(chars.$COMMA));
this.rbracesExpected--;
this.expectCharacter(chars.$RBRACE);

View File

@ -122,6 +122,23 @@ describe('parser', () => {
expectActionError('{1234:0}', 'expected identifier, keyword, or string');
expectActionError('{#myField:0}', 'expected identifier, keyword or string');
});
it('should parse property shorthand declarations', () => {
checkAction('{a, b, c}', '{a: a, b: b, c: c}');
checkAction('{a: 1, b}', '{a: 1, b: b}');
checkAction('{a, b: 1}', '{a: a, b: 1}');
checkAction('{a: 1, b, c: 2}', '{a: 1, b: b, c: 2}');
});
it('should not allow property shorthand declaration on quoted properties', () => {
expectActionError('{"a-b"}', 'expected : at column 7');
});
it('should not infer invalid identifiers as shorthand property declarations', () => {
expectActionError('{a.b}', 'expected } at column 3');
expectActionError('{a["b"]}', 'expected } at column 3');
expectActionError('{1234}', ' expected identifier, keyword, or string at column 2');
});
});
describe('member access', () => {

View File

@ -360,4 +360,15 @@ describe('expression AST absolute source spans', () => {
expect(spans).toContain(['nestedPlaceholder', new AbsoluteSourceSpan(89, 106)]);
});
});
describe('object literal', () => {
it('is correct for object literals with shorthand property declarations', () => {
const spans =
humanizeExpressionSource(parse('<div (click)="test({a: 1, b, c: 3, foo})"></div>').nodes);
expect(spans).toContain(['{a: 1, b: b, c: 3, foo: foo}', new AbsoluteSourceSpan(19, 39)]);
expect(spans).toContain(['b', new AbsoluteSourceSpan(26, 27)]);
expect(spans).toContain(['foo', new AbsoluteSourceSpan(35, 38)]);
});
});
});

View File

@ -2041,6 +2041,26 @@ describe('acceptance integration tests', () => {
expect(fixture.nativeElement.innerHTML).toContain('<text>Hello</text>');
});
it('should handle shorthand property declarations in templates', () => {
@Directive({selector: '[my-dir]'})
class Dir {
@Input('my-dir') value: any;
}
@Component({template: `<div [my-dir]="{a, b: 2, someProp}"></div>`})
class App {
@ViewChild(Dir) directive!: Dir;
a = 1;
someProp = 3;
}
TestBed.configureTestingModule({declarations: [App, Dir]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.componentInstance.directive.value).toEqual({a: 1, b: 2, someProp: 3});
});
describe('tView.firstUpdatePass', () => {
function isFirstUpdatePass() {
const lView = getLView();

View File

@ -458,6 +458,32 @@ describe('definitions', () => {
const definitionAndBoundSpan = ngLS.getDefinitionAndBoundSpan(APP_COMPONENT, position);
expect(definitionAndBoundSpan).toBeUndefined();
});
it('should work for object literals with shorthand declarations in an action', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `<div (click)="setHero({na¦me, id: 1})"></div>`,
expectedSpanText: 'name',
});
expect(definitions!.length).toEqual(1);
const [def] = definitions;
expect(def.textSpan).toEqual('name');
expect(def.fileName).toContain('/app/app.component.ts');
expect(def.contextSpan).toContain(`name = 'Frodo';`);
});
it('should work for object literals with shorthand declarations in a data binding', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `{{ {na¦me} }}`,
expectedSpanText: 'name',
});
expect(definitions!.length).toEqual(1);
const [def] = definitions;
expect(def.textSpan).toEqual('name');
expect(def.fileName).toContain('/app/app.component.ts');
expect(def.contextSpan).toContain(`name = 'Frodo';`);
});
});
describe('external resources', () => {

View File

@ -622,6 +622,38 @@ describe('getTargetAtPosition for expression AST', () => {
expect(isExpressionNode(node!)).toBe(true);
expect(node).toBeInstanceOf(e.Conditional);
});
describe('object literal shorthand', () => {
it('should locate on literal with one shorthand property', () => {
const {errors, nodes, position} = parse(`{{ {va¦l1} }}`);
expect(errors).toBe(null);
const {context} = getTargetAtPosition(nodes, position)!;
expect(context.kind).toBe(TargetNodeKind.RawExpression);
const {node} = context as SingleNodeTarget;
expect(node).toBeInstanceOf(e.PropertyRead);
expect((node as e.PropertyRead).name).toBe('val1');
});
it('should locate on literal with multiple shorthand properties', () => {
const {errors, nodes, position} = parse(`{{ {val1, va¦l2} }}`);
expect(errors).toBe(null);
const {context} = getTargetAtPosition(nodes, position)!;
expect(context.kind).toBe(TargetNodeKind.RawExpression);
const {node} = context as SingleNodeTarget;
expect(node).toBeInstanceOf(e.PropertyRead);
expect((node as e.PropertyRead).name).toBe('val2');
});
it('should locale on property with mixed shorthand and regular properties', () => {
const {errors, nodes, position} = parse(`{{ {val1: 'val1', va¦l2} }}`);
expect(errors).toBe(null);
const {context} = getTargetAtPosition(nodes, position)!;
expect(context.kind).toBe(TargetNodeKind.RawExpression);
const {node} = context as SingleNodeTarget;
expect(node).toBeInstanceOf(e.PropertyRead);
expect((node as e.PropertyRead).name).toBe('val2');
});
});
});
describe('findNodeAtPosition for microsyntax expression', () => {

View File

@ -500,6 +500,48 @@ describe('quick info', () => {
expect(documentation).toBe('This is the title of the `AppCmp` Component.');
});
});
it('should work for object literal with shorthand property declarations', () => {
initMockFileSystem('Native');
env = LanguageServiceTestEnv.setup();
project = env.addProject(
'test', {
'app.ts': `
import {Component, NgModule} from '@angular/core';
import {CommonModule} from '@angular/common';
@Component({
selector: 'some-cmp',
templateUrl: './app.html',
})
export class SomeCmp {
val1 = 'one';
val2 = 2;
doSomething(obj: {val1: string, val2: number}) {}
}
@NgModule({
declarations: [SomeCmp],
imports: [CommonModule],
})
export class AppModule{
}
`,
'app.html': `{{doSomething({val1, val2})}}`,
},
{strictTemplates: true});
env.expectNoSourceDiagnostics();
project.expectNoSourceDiagnostics();
const template = project.openFile('app.html');
template.moveCursorToText('val¦1');
const quickInfo = template.getQuickInfoAtPosition();
expect(toText(quickInfo!.displayParts)).toEqual('(property) SomeCmp.val1: string');
template.moveCursorToText('val¦2');
const quickInfo2 = template.getQuickInfoAtPosition();
expect(toText(quickInfo2!.displayParts)).toEqual('(property) SomeCmp.val2: number');
});
});
describe('generics', () => {

View File

@ -235,7 +235,7 @@ describe('find references and rename locations', () => {
beforeEach(() => {
const files = {
'app.ts': `
'app.ts': `
import {Component} from '@angular/core';
@Component({template: '<div (click)="title = otherTitle"></div>' })
@ -617,7 +617,7 @@ describe('find references and rename locations', () => {
let file: OpenBuffer;
beforeEach(() => {
const files = {
'app.ts': `
'app.ts': `
import {Component} from '@angular/core';
@Component({template: '<div *ngFor="let hero of heroes; let iRef = index">{{iRef}}</div>'})
@ -739,6 +739,42 @@ describe('find references and rename locations', () => {
assertTextSpans(renameLocations, ['name']);
});
});
describe('when cursor is on property read of variable', () => {
let file: OpenBuffer;
beforeEach(() => {
const files = {
'app.ts': `
import {Component} from '@angular/core';
@Component({template: '<div (click)="setHero({name})"></div>'})
export class AppCmp {
name = 'Frodo';
setHero(hero: {name: string}) {}
}`
};
env = LanguageServiceTestEnv.setup();
const project = createModuleAndProjectWithDeclarations(env, 'test', files);
file = project.openFile('app.ts');
file.moveCursorToText('{na¦me}');
});
it('should find references', () => {
const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(2);
assertFileNames(refs, ['app.ts']);
assertTextSpans(refs, ['name']);
});
it('should find rename locations', () => {
const renameLocations = getRenameLocationsAtPosition(file)!;
expect(renameLocations.length).toBe(2);
assertFileNames(renameLocations, ['app.ts']);
assertTextSpans(renameLocations, ['name']);
});
});
});
describe('pipes', () => {
@ -1245,7 +1281,7 @@ describe('find references and rename locations', () => {
}`,
'app.ts': `
import {Component} from '@angular/core';
@Component({template: '<div string-model [(model)]="title"></div>'})
export class AppCmp {
title = 'title';

View File

@ -48,7 +48,11 @@ export class AppComponent {
constNames = [{name: 'name'}] as const;
private myField = 'My Field';
strOrNumber: string|number = '';
name = 'Frodo';
setTitle(newTitle: string) {
this.title = newTitle;
}
setHero(obj: Hero) {
this.hero = obj;
}
}