fix(ivy): throw if @Input and @ContentChild share a property (#28415)
In View Engine, we supported @Input and @ContentChild annotations on the same property. This feature was somewhat brittle because it would only work for static queries, so it would break if a content child was passed in wrapped in an *ngIf. Due to the inconsistent behavior and low usage both internally and externally, we will likely be deprecating it in the next version, and it does not make sense to perpetuate it in Ivy. This commit ensures that we now throw in Ivy if we encounter the two annotations on the same property. PR Close #28415
This commit is contained in:
parent
7d9aa67d8c
commit
fdc6e159b4
|
@ -394,6 +394,10 @@ export function queriesFromFields(
|
||||||
fields: {member: ClassMember, decorators: Decorator[]}[], reflector: ReflectionHost,
|
fields: {member: ClassMember, decorators: Decorator[]}[], reflector: ReflectionHost,
|
||||||
evaluator: PartialEvaluator): R3QueryMetadata[] {
|
evaluator: PartialEvaluator): R3QueryMetadata[] {
|
||||||
return fields.map(({member, decorators}) => {
|
return fields.map(({member, decorators}) => {
|
||||||
|
// 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`);
|
||||||
|
}
|
||||||
if (decorators.length !== 1) {
|
if (decorators.length !== 1) {
|
||||||
throw new Error(`Cannot have multiple query decorators on the same class member`);
|
throw new Error(`Cannot have multiple query decorators on the same class member`);
|
||||||
} else if (!isPropertyTypeMember(member)) {
|
} else if (!isPropertyTypeMember(member)) {
|
||||||
|
|
|
@ -1688,6 +1688,43 @@ describe('compiler compliance', () => {
|
||||||
expectEmit(source, ContentQueryComponentDefinition, 'Invalid ContentQuery declaration');
|
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') 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', () => {
|
describe('pipes', () => {
|
||||||
|
|
|
@ -13,7 +13,7 @@ import {resolveForwardRef} from '../../di/forward_ref';
|
||||||
import {getReflect, reflectDependencies} from '../../di/jit/util';
|
import {getReflect, reflectDependencies} from '../../di/jit/util';
|
||||||
import {Type} from '../../interface/type';
|
import {Type} from '../../interface/type';
|
||||||
import {Query} from '../../metadata/di';
|
import {Query} from '../../metadata/di';
|
||||||
import {Component, Directive} from '../../metadata/directives';
|
import {Component, Directive, Input} from '../../metadata/directives';
|
||||||
import {componentNeedsResolution, maybeQueueResolutionOfComponentResources} from '../../metadata/resource_loading';
|
import {componentNeedsResolution, maybeQueueResolutionOfComponentResources} from '../../metadata/resource_loading';
|
||||||
import {ViewEncapsulation} from '../../metadata/view';
|
import {ViewEncapsulation} from '../../metadata/view';
|
||||||
import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
|
import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
|
||||||
|
@ -180,13 +180,17 @@ function extractQueriesMetadata(
|
||||||
const queriesMeta: R3QueryMetadataFacade[] = [];
|
const queriesMeta: R3QueryMetadataFacade[] = [];
|
||||||
for (const field in propMetadata) {
|
for (const field in propMetadata) {
|
||||||
if (propMetadata.hasOwnProperty(field)) {
|
if (propMetadata.hasOwnProperty(field)) {
|
||||||
propMetadata[field].forEach(ann => {
|
const annotations = propMetadata[field];
|
||||||
|
annotations.forEach(ann => {
|
||||||
if (isQueryAnn(ann)) {
|
if (isQueryAnn(ann)) {
|
||||||
if (!ann.selector) {
|
if (!ann.selector) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Can't construct a query for the property "${field}" of ` +
|
`Can't construct a query for the property "${field}" of ` +
|
||||||
`"${renderStringify(type)}" since the query selector wasn't defined.`);
|
`"${renderStringify(type)}" since the query selector wasn't defined.`);
|
||||||
}
|
}
|
||||||
|
if (annotations.some(isInputAnn)) {
|
||||||
|
throw new Error(`Cannot combine @Input decorators with query decorators`);
|
||||||
|
}
|
||||||
queriesMeta.push(convertToR3QueryMetadata(field, ann));
|
queriesMeta.push(convertToR3QueryMetadata(field, ann));
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
@ -213,6 +217,10 @@ function isViewQuery(value: any): value is Query {
|
||||||
return name === 'ViewChild' || name === 'ViewChildren';
|
return name === 'ViewChild' || name === 'ViewChildren';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isInputAnn(value: any): value is Input {
|
||||||
|
return value.ngMetadataName === 'Input';
|
||||||
|
}
|
||||||
|
|
||||||
function splitByComma(value: string): string[] {
|
function splitByComma(value: string): string[] {
|
||||||
return value.split(',').map(piece => piece.trim());
|
return value.split(',').map(piece => piece.trim());
|
||||||
}
|
}
|
||||||
|
|
|
@ -6,13 +6,13 @@
|
||||||
* found in the LICENSE file at https://angular.io/license
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import {ANALYZE_FOR_ENTRY_COMPONENTS, ApplicationRef, Component, ComponentRef, ContentChild, Directive, ErrorHandler, EventEmitter, HostListener, InjectionToken, Injector, Input, NgModule, NgModuleRef, NgZone, Output, Pipe, PipeTransform, Provider, QueryList, Renderer2, SimpleChanges, TemplateRef, ViewChildren, ViewContainerRef, destroyPlatform, ɵivyEnabled as ivyEnabled} from '@angular/core';
|
import {ANALYZE_FOR_ENTRY_COMPONENTS, ApplicationRef, Component, ComponentRef, ContentChild, Directive, ErrorHandler, EventEmitter, HostListener, InjectionToken, Injector, Input, NgModule, NgModuleRef, NgZone, Output, Pipe, PipeTransform, Provider, QueryList, Renderer2, SimpleChanges, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, destroyPlatform, ɵivyEnabled as ivyEnabled} from '@angular/core';
|
||||||
import {TestBed, fakeAsync, inject, tick} from '@angular/core/testing';
|
import {TestBed, fakeAsync, inject, tick} from '@angular/core/testing';
|
||||||
import {BrowserModule, By, DOCUMENT} from '@angular/platform-browser';
|
import {BrowserModule, By, DOCUMENT} from '@angular/platform-browser';
|
||||||
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
|
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
|
||||||
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
|
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
|
||||||
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
||||||
import {fixmeIvy, modifiedInIvy} from '@angular/private/testing';
|
import {fixmeIvy, modifiedInIvy, onlyInIvy} from '@angular/private/testing';
|
||||||
|
|
||||||
if (ivyEnabled) {
|
if (ivyEnabled) {
|
||||||
describe('ivy', () => { declareTests(); });
|
describe('ivy', () => { declareTests(); });
|
||||||
|
@ -355,7 +355,7 @@ function declareTests(config?: {useJit: boolean}) {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
fixmeIvy('FW-797: @ContentChildren results are assigned after @Input bindings')
|
modifiedInIvy('Static ViewChild and ContentChild queries are resolved in update mode')
|
||||||
.it('should support @ContentChild and @Input on the same property for static queries',
|
.it('should support @ContentChild and @Input on the same property for static queries',
|
||||||
() => {
|
() => {
|
||||||
@Directive({selector: 'test'})
|
@Directive({selector: 'test'})
|
||||||
|
@ -387,6 +387,22 @@ function declareTests(config?: {useJit: boolean}) {
|
||||||
expect(testDirs[2].tpl).toBeDefined();
|
expect(testDirs[2].tpl).toBeDefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
onlyInIvy('Ivy does not support @ContentChild and @Input on the same property')
|
||||||
|
.it('should throw if @ContentChild and @Input are on the same property', () => {
|
||||||
|
@Directive({selector: 'test'})
|
||||||
|
class Test {
|
||||||
|
@Input() @ContentChild(TemplateRef) tpl !: TemplateRef<any>;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Component({selector: 'my-app', template: `<test></test>`})
|
||||||
|
class App {
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(() => {
|
||||||
|
TestBed.configureTestingModule({declarations: [App, Test]}).createComponent(App);
|
||||||
|
}).toThrowError(/Cannot combine @Input decorators with query decorators/);
|
||||||
|
});
|
||||||
|
|
||||||
it('should not add ng-version for dynamically created components', () => {
|
it('should not add ng-version for dynamically created components', () => {
|
||||||
@Component({template: ''})
|
@Component({template: ''})
|
||||||
class App {
|
class App {
|
||||||
|
|
Loading…
Reference in New Issue