Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
Expand Up @@ -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
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/component_ref.ts
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/di.ts
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/render3/instructions/di.ts
Expand Up @@ -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';

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/instructions/listener.ts
Expand Up @@ -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;

Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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, //
Expand Down Expand Up @@ -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>);
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/render3/node_assert.ts
Expand Up @@ -26,12 +26,14 @@ 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,
`Should be one of ${types.map(typeName).join(', ')} but got ${typeName(tNode.type)}`);
message ??
AndrewKushnir marked this conversation as resolved.
Show resolved Hide resolved
`Should be one of ${types.map(typeName).join(', ')} but got ${typeName(tNode.type)}`);
}

export function assertNodeNotOfTypes(tNode: TNode, types: TNodeType[], message?: string): void {
Expand Down
20 changes: 10 additions & 10 deletions packages/core/src/render3/node_manipulation.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/query.ts
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/view_engine_compatibility.ts
Expand Up @@ -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];
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/render3/view_ref.ts
Expand Up @@ -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) {
Expand Down
61 changes: 60 additions & 1 deletion packages/core/test/acceptance/component_spec.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -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',
Expand Down