refactor(core): throw more descriptive error message in case of invalid host element (#35916)

This commit replaces an assert with more descriptive error message that is thrown in case `<ng-template>` or `<ng-container>` is used as host element for a Component.

Resolves #35240.

PR Close #35916
This commit is contained in:
Andrew Kushnir 2020-03-06 11:57:29 -08:00
parent 83fe963a4b
commit 0879d2e85d
12 changed files with 95 additions and 28 deletions

View File

@ -62,7 +62,7 @@
"bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.",
"bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338",
"bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1198917, see PR#37221 and PR#37317 for more info",
"bundle": 1209688
"bundle": 1210239
}
}
}

View File

@ -225,7 +225,7 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
createElementRef(viewEngine_ElementRef, tElementNode, rootLView), rootLView, tElementNode);
// The host element of the internal root view is attached to the component's host view node.
ngDevMode && assertNodeOfPossibleTypes(rootTView.node, TNodeType.View);
ngDevMode && assertNodeOfPossibleTypes(rootTView.node, [TNodeType.View]);
rootTView.node!.child = tElementNode;
return componentRef;

View File

@ -267,7 +267,7 @@ export function diPublicInInjector(
export function injectAttributeImpl(tNode: TNode, attrNameToInject: string): string|null {
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer);
tNode, [TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer]);
ngDevMode && assertDefined(tNode, 'expecting tNode');
if (attrNameToInject === 'class') {
return tNode.classes;

View File

@ -9,8 +9,7 @@ import {InjectFlags, InjectionToken, resolveForwardRef} from '../../di';
import {ɵɵinject} from '../../di/injector_compatibility';
import {Type} from '../../interface/type';
import {getOrCreateInjectable, injectAttributeImpl} from '../di';
import {TDirectiveHostNode, TNodeType} from '../interfaces/node';
import {assertNodeOfPossibleTypes} from '../node_assert';
import {TDirectiveHostNode} from '../interfaces/node';
import {getLView, getPreviousOrParentTNode} from '../state';
/**

View File

@ -128,7 +128,7 @@ function listenerInternal(
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer);
tNode, [TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer]);
let processOutputs = true;

View File

@ -15,6 +15,7 @@ import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGrea
import {createNamedArrayType} from '../../util/named_array_type';
import {initNgDevMode} from '../../util/ng_dev_mode';
import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
import {stringify} from '../../util/stringify';
import {assertFirstCreatePass, assertLContainer, assertLView} from '../assert';
import {attachPatchData} from '../context_discovery';
import {getFactoryDef} from '../definition';
@ -272,7 +273,7 @@ export function assignTViewNodeToLView(
let tNode = tView.node;
if (tNode == null) {
ngDevMode && tParentNode &&
assertNodeOfPossibleTypes(tParentNode, TNodeType.Element, TNodeType.Container);
assertNodeOfPossibleTypes(tParentNode, [TNodeType.Element, TNodeType.Container]);
tView.node = tNode = createTNode(
tView,
tParentNode as TElementNode | TContainerNode | null, //
@ -1278,7 +1279,7 @@ function instantiateAllDirectives(
const isComponent = isComponentDef(def);
if (isComponent) {
ngDevMode && assertNodeOfPossibleTypes(tNode, TNodeType.Element);
ngDevMode && assertNodeOfPossibleTypes(tNode, [TNodeType.Element]);
addComponentLogic(lView, tNode as TElementNode, def as ComponentDef<any>);
}
@ -1366,7 +1367,7 @@ function findDirectiveDefMatches(
ngDevMode && assertFirstCreatePass(tView);
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Element, TNodeType.ElementContainer, TNodeType.Container);
tNode, [TNodeType.Element, TNodeType.ElementContainer, TNodeType.Container]);
const registry = tView.directiveRegistry;
let matches: any[]|null = null;
if (registry) {
@ -1377,6 +1378,12 @@ function findDirectiveDefMatches(
diPublicInInjector(getOrCreateNodeInjectorForNode(tNode, viewData), tView, def.type);
if (isComponentDef(def)) {
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, [TNodeType.Element],
`"${tNode.tagName}" tags cannot be used as component hosts. ` +
`Please use a different tag to activate the ${
stringify(def.type)} component.`);
if (tNode.flags & TNodeFlags.isComponentHost) throwMultipleComponentError(tNode);
markAsComponentHost(tView, tNode);
// The component is always stored first with directives after.

View File

@ -26,11 +26,13 @@ export function assertNodeType(tNode: TNode, type: TNodeType): asserts tNode is
assertEqual(tNode.type, type, `should be a ${typeName(type)}`);
}
export function assertNodeOfPossibleTypes(tNode: TNode|null, ...types: TNodeType[]): void {
export function assertNodeOfPossibleTypes(
tNode: TNode|null, types: TNodeType[], message?: string): void {
assertDefined(tNode, 'should be called with a TNode');
const found = types.some(type => tNode.type === type);
assertEqual(
found, true,
message ??
`Should be one of ${types.map(typeName).join(', ')} but got ${typeName(tNode.type)}`);
}

View File

@ -552,7 +552,7 @@ function getRenderParent(tView: TView, tNode: TNode, currentView: LView): REleme
} else {
// We are inserting a root element of the component view into the component host element and
// it should always be eager.
ngDevMode && assertNodeOfPossibleTypes(hostTNode, TNodeType.Element);
ngDevMode && assertNodeOfPossibleTypes(hostTNode, [TNodeType.Element]);
return currentView[HOST];
}
} else {
@ -698,10 +698,10 @@ export function appendChild(
*/
function getFirstNativeNode(lView: LView, tNode: TNode|null): RNode|null {
if (tNode !== null) {
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer,
TNodeType.IcuContainer, TNodeType.Projection);
ngDevMode && assertNodeOfPossibleTypes(tNode, [
TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer, TNodeType.IcuContainer,
TNodeType.Projection
]);
const tNodeType = tNode.type;
if (tNodeType === TNodeType.Element) {
@ -778,10 +778,10 @@ function applyNodes(
renderParent: RElement|null, beforeNode: RNode|null, isProjection: boolean) {
while (tNode != null) {
ngDevMode && assertTNodeForLView(tNode, lView);
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer,
TNodeType.Projection, TNodeType.Projection, TNodeType.IcuContainer);
ngDevMode && assertNodeOfPossibleTypes(tNode, [
TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer, TNodeType.Projection,
TNodeType.IcuContainer
]);
const rawSlotValue = lView[tNode.index];
const tNodeType = tNode.type;
if (isProjection) {
@ -798,7 +798,7 @@ function applyNodes(
applyProjectionRecursive(
renderer, action, lView, tNode as TProjectionNode, renderParent, beforeNode);
} else {
ngDevMode && assertNodeOfPossibleTypes(tNode, TNodeType.Element, TNodeType.Container);
ngDevMode && assertNodeOfPossibleTypes(tNode, [TNodeType.Element, TNodeType.Container]);
applyToElementOrContainer(action, renderer, renderParent, rawSlotValue, beforeNode);
}
}

View File

@ -326,7 +326,7 @@ function createSpecialToken(lView: LView, tNode: TNode, read: any): any {
} else if (read === ViewContainerRef) {
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer);
tNode, [TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer]);
return createContainerRef(
ViewContainerRef, ViewEngine_ElementRef,
tNode as TElementNode | TContainerNode | TElementContainerNode, lView);

View File

@ -340,7 +340,7 @@ export function createContainerRef(
ngDevMode &&
assertNodeOfPossibleTypes(
hostTNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer);
hostTNode, [TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer]);
let lContainer: LContainer;
const slotValue = hostView[hostTNode.index];

View File

@ -324,10 +324,10 @@ function collectNativeNodes(
tView: TView, lView: LView, tNode: TNode|null, result: any[],
isProjection: boolean = false): any[] {
while (tNode !== null) {
ngDevMode &&
assertNodeOfPossibleTypes(
tNode, TNodeType.Element, TNodeType.Container, TNodeType.Projection,
TNodeType.ElementContainer, TNodeType.IcuContainer);
ngDevMode && assertNodeOfPossibleTypes(tNode, [
TNodeType.Element, TNodeType.Container, TNodeType.Projection, TNodeType.ElementContainer,
TNodeType.IcuContainer
]);
const lNode = lView[tNode.index];
if (lNode !== null) {

View File

@ -11,7 +11,7 @@ import {ApplicationRef, Component, ComponentFactoryResolver, ComponentRef, Eleme
import {TestBed} from '@angular/core/testing';
import {ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {onlyInIvy} from '@angular/private/testing';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
import {domRendererFactory3} from '../../src/render3/interfaces/renderer';
@ -259,6 +259,65 @@ describe('component', () => {
expect(wrapperEls.length).toBe(2); // other elements are preserved
});
describe('invalid host element', () => {
it('should throw when <ng-container> is used as a host element for a Component', () => {
@Component({
selector: 'ng-container',
template: '...',
})
class Comp {
}
@Component({
selector: 'root',
template: '<ng-container></ng-container>',
})
class App {
}
TestBed.configureTestingModule({declarations: [App, Comp]});
if (ivyEnabled) {
expect(() => TestBed.createComponent(App))
.toThrowError(
/"ng-container" tags cannot be used as component hosts. Please use a different tag to activate the Comp component/);
} else {
// In VE there is no special check for the case when `<ng-container>` is used as a host
// element for a Component. VE tries to attach Component's content to a Comment node that
// represents the `<ng-container>` location and this call fails with a
// browser/environment-specific error message, so we just verify that this scenario is
// triggering an error in VE.
expect(() => TestBed.createComponent(App)).toThrow();
}
});
it('should throw when <ng-template> is used as a host element for a Component', () => {
@Component({
selector: 'ng-template',
template: '...',
})
class Comp {
}
@Component({
selector: 'root',
template: '<ng-template></ng-template>',
})
class App {
}
TestBed.configureTestingModule({declarations: [App, Comp]});
if (ivyEnabled) {
expect(() => TestBed.createComponent(App))
.toThrowError(
/"ng-template" tags cannot be used as component hosts. Please use a different tag to activate the Comp component/);
} else {
expect(() => TestBed.createComponent(App))
.toThrowError(
/Components on an embedded template: Comp \("\[ERROR ->\]<ng-template><\/ng-template>"\)/);
}
});
});
it('should use a new ngcontent attribute for child elements created w/ Renderer2', () => {
@Component({
selector: 'app-root',