refactor(ivy): use FatalDiagnosticError to throw more descriptive errors while extracting queries information (#31123)

Prior to this commit, the logic to extract query information from class fields used an instance of regular Error class to throw an error. As a result, some useful information (like reference to a specific field) was missing. Replacing Error class with FatalDiagnosticError one makes the error more verbose that should simplify debugging.

PR Close #31123
This commit is contained in:
Andrew Kushnir 2019-06-18 17:23:51 -07:00 committed by Kara Erickson
parent b11a2057c6
commit 2aba485118
4 changed files with 80 additions and 43 deletions

View File

@ -435,16 +435,24 @@ export function queriesFromFields(
fields: {member: ClassMember, decorators: Decorator[]}[], reflector: ReflectionHost,
evaluator: PartialEvaluator): R3QueryMetadata[] {
return fields.map(({member, decorators}) => {
const decorator = decorators[0];
const node = member.node || decorator.node;
// Throw in case of `@Input() @ContentChild('foo') foo: any`, which is not supported in Ivy
if (member.decorators !.some(v => v.name === 'Input')) {
throw new Error(`Cannot combine @Input decorators with query decorators`);
throw new FatalDiagnosticError(
ErrorCode.DECORATOR_COLLISION, node,
'Cannot combine @Input decorators with query decorators');
}
if (decorators.length !== 1) {
throw new Error(`Cannot have multiple query decorators on the same class member`);
throw new FatalDiagnosticError(
ErrorCode.DECORATOR_COLLISION, node,
'Cannot have multiple query decorators on the same class member');
} else if (!isPropertyTypeMember(member)) {
throw new Error(`Query decorator must go on a property-type member`);
throw new FatalDiagnosticError(
ErrorCode.DECORATOR_UNEXPECTED, node,
'Query decorator must go on a property-type member');
}
const decorator = decorators[0];
return extractQueryMetadata(
decorator.node, decorator.name, decorator.args || [], member.name, reflector, evaluator);
});

View File

@ -14,7 +14,7 @@ export enum ErrorCode {
DECORATOR_UNEXPECTED = 1005,
/**
* This error code indicates that there are incompatible decorators on a type.
* This error code indicates that there are incompatible decorators on a type or a class field.
*/
DECORATOR_COLLISION = 1006,

View File

@ -1985,44 +1985,6 @@ describe('compiler compliance', () => {
expectEmit(source, ContentQueryComponentDefinition, 'Invalid ContentQuery declaration');
});
it('should throw error if content queries share a property with inputs', () => {
const files = {
app: {
...directive,
'content_query.ts': `
import {Component, ContentChild, Input, NgModule} from '@angular/core';
@Component({
selector: 'content-query-component',
template: \`
<div><ng-content></ng-content></div>
\`
})
export class ContentQueryComponent {
@Input() @ContentChild('foo', {static: false}) foo: any;
}
@Component({
selector: 'my-app',
template: \`
<content-query-component>
<div #foo></div>
</content-query-component>
\`
})
export class MyApp { }
@NgModule({declarations: [ContentQueryComponent, MyApp]})
export class MyModule { }
`
}
};
expect(() => compile(files, angularFiles))
.toThrowError(/Cannot combine @Input decorators with query decorators/);
});
});
describe('pipes', () => {

View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics';
import {LazyRoute} from '@angular/compiler-cli/src/ngtsc/routing';
import * as path from 'path';
import * as ts from 'typescript';
@ -1018,6 +1019,72 @@ describe('ngtsc behavioral tests', () => {
expect(trim(errors[0].messageText as string))
.toContain('Directive TestDir has no selector, please add it!');
});
it('should throw error if content queries share a property with inputs', () => {
env.tsconfig({});
env.write('test.ts', `
import {Component, ContentChild, Input} from '@angular/core';
@Component({
selector: 'test-cmp',
template: '<ng-content></ng-content>'
})
export class TestCmp {
@Input() @ContentChild('foo', {static: false}) foo: any;
}
`);
const errors = env.driveDiagnostics();
const {code, messageText} = errors[0];
expect(code).toBe(ngErrorCode(ErrorCode.DECORATOR_COLLISION));
expect(trim(messageText as string))
.toContain('Cannot combine @Input decorators with query decorators');
});
it('should throw error if multiple query decorators are used on the same field', () => {
env.tsconfig({});
env.write('test.ts', `
import {Component, ContentChild} from '@angular/core';
@Component({
selector: 'test-cmp',
template: '...'
})
export class TestCmp {
@ContentChild('bar', {static: true})
@ContentChild('foo', {static: false})
foo: any;
}
`);
const errors = env.driveDiagnostics();
const {code, messageText} = errors[0];
expect(code).toBe(ngErrorCode(ErrorCode.DECORATOR_COLLISION));
expect(trim(messageText as string))
.toContain('Cannot have multiple query decorators on the same class member');
});
it('should throw error if query decorators are used on non property-type member', () => {
env.tsconfig({});
env.write('test.ts', `
import {Component, ContentChild} from '@angular/core';
@Component({
selector: 'test-cmp',
template: '...'
})
export class TestCmp {
@ContentChild('foo', {static: false})
private someFn() {}
}
`);
const errors = env.driveDiagnostics();
const {code, messageText} = errors[0];
expect(code).toBe(ngErrorCode(ErrorCode.DECORATOR_UNEXPECTED));
expect(trim(messageText as string))
.toContain('Query decorator must go on a property-type member');
});
});
describe('multiple decorators on classes', () => {