cargdev/angular
1
0
Fork 0
Code Issues 17 Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(core): assert TNode is not a container when setting attribute on element (#37111)

Browse Source 
This PR provides a more helpful error than the one currently present:
`el.setAttribute is not a function`. It is not valid to have directives with host bindings
on `ng-template` or `ng-container` nodes. VE would silently ignore this, while Ivy
attempts to set the attribute and throws an error because these are comment nodes
and do not have `setAttribute` functionality.

It is better to throw a helpful error than to silently ignore this because
putting a directive with host binding on an `ng-template` or `ng-container` is most often a mistake.
Developers should be made aware that the host binding will have no effect in these cases.

Note that an error is already thrown in Ivy, as mentioned above, so this
is not a breaking change and can be merged to both master and patch.

Resolves #35994

PR Close #37111
This commit is contained in:
Andrew Scott
2020-05-14 09:34:51 -07:00
committed by atscott
parent 8bead6bfdd
commit fdbe9f5d9f
4 changed files with 68 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
packages/core/src/render3/instructions/shared.ts
View File

@ -29,7 +29,7 @@ import {isProceduralRenderer, RComment, RElement, Renderer3, RendererFactory3, R
import {SanitizerFn} from '../interfaces/sanitization';
import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks';
import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TRANSPLANTED_VIEWS_TO_REFRESH, TVIEW, TView, TViewType} from '../interfaces/view';
import {assertNodeOfPossibleTypes} from '../node_assert';
import {assertNodeNotOfTypes, assertNodeOfPossibleTypes} from '../node_assert';
import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher';
import {enterView, getBindingsEnabled, getCheckNoChangesMode, getCurrentDirectiveIndex, getIsParent, getPreviousOrParentTNode, getSelectedIndex, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentDirectiveIndex, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
import {NO_CHANGE} from '../tokens';
@ -1484,8 +1484,14 @@ function addComponentLogic<T>(lView: LView, hostTNode: TElementNode, def: Compon
export function elementAttributeInternal(
tNode: TNode, lView: LView, name: string, value: any, sanitizer: SanitizerFn|null|undefined,
namespace: string|null|undefined) {
ngDevMode && assertNotSame(value, NO_CHANGE as any, 'Incoming value should never be NO_CHANGE.');
ngDevMode && validateAgainstEventAttributes(name);
if (ngDevMode) {
assertNotSame(value, NO_CHANGE as any, 'Incoming value should never be NO_CHANGE.');
validateAgainstEventAttributes(name);
assertNodeNotOfTypes(
tNode, [TNodeType.Container, TNodeType.ElementContainer],
`Attempted to set attribute \`${name}\` on a container node. ` +
`Host bindings are not valid on ng-container or ng-template.`);
}
const element = getNativeByTNode(tNode, lView) as RElement;
const renderer = lView[RENDERER];
if (value == null) {

9
packages/core/src/render3/node_assert.ts
View File

@ -34,6 +34,15 @@ export function assertNodeOfPossibleTypes(tNode: TNode|null, ...types: TNodeType
`Should be one of ${types.map(typeName).join(', ')} but got ${typeName(tNode.type)}`);
}
export function assertNodeNotOfTypes(tNode: TNode, types: TNodeType[], message?: string): void {
assertDefined(tNode, 'should be called with a TNode');
const found = types.some(type => tNode.type === type);
assertEqual(
found, false,
message ??
`Should not be one of ${types.map(typeName).join(', ')} but got ${typeName(tNode.type)}`);
}
function typeName(type: TNodeType): string {
if (type == TNodeType.Projection) return 'Projection';
if (type == TNodeType.Container) return 'Container';