From 2e237abb095188d2983e4281fc16a175d71579df Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 12 Oct 2020 16:57:07 -0700 Subject: [PATCH] refactor(core): Change `TName.tagName` to a more generic `value` name. (#39233) This is a pre-requisite for making the `TNode.value` a generic storage mechanism for attaching data to `TNode`. PR Close #39233 --- packages/core/src/debug/debug_node.ts | 2 +- packages/core/src/render3/errors.ts | 2 +- packages/core/src/render3/i18n/i18n_util.ts | 8 +++---- .../core/src/render3/instructions/element.ts | 2 +- .../i18n_icu_container_visitor.ts | 2 +- .../src/render3/instructions/lview_debug.ts | 6 +++--- .../core/src/render3/instructions/shared.ts | 21 +++++++++---------- packages/core/src/render3/interfaces/node.ts | 12 +++++++---- .../core/src/render3/node_selector_matcher.ts | 7 +++---- packages/core/test/acceptance/debug_spec.ts | 4 ++-- packages/core/test/render3/is_shape_of.ts | 2 +- packages/core/test/render3/matchers_spec.ts | 2 +- 12 files changed, 35 insertions(+), 35 deletions(-) diff --git a/packages/core/src/debug/debug_node.ts b/packages/core/src/debug/debug_node.ts index 2cdbd75585..8f0cc9a706 100644 --- a/packages/core/src/debug/debug_node.ts +++ b/packages/core/src/debug/debug_node.ts @@ -266,7 +266,7 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme const lView = context.lView; const tData = lView[TVIEW].data; const tNode = tData[context.nodeIndex] as TNode; - return tNode.tagName!; + return tNode.value!; } catch (e) { return this.nativeNode.nodeName; } diff --git a/packages/core/src/render3/errors.ts b/packages/core/src/render3/errors.ts index 6373c70dad..ad163b5447 100644 --- a/packages/core/src/render3/errors.ts +++ b/packages/core/src/render3/errors.ts @@ -22,7 +22,7 @@ export function throwCyclicDependencyError(token: any): never { /** Called when there are multiple component selectors that match a given node */ export function throwMultipleComponentError(tNode: TNode): never { - throw new Error(`Multiple components match node with tagname ${tNode.tagName}`); + throw new Error(`Multiple components match node with tagname ${tNode.value}`); } export function throwMixedMultiProviderError() { diff --git a/packages/core/src/render3/i18n/i18n_util.ts b/packages/core/src/render3/i18n/i18n_util.ts index e0e7d46ddd..8993c6093d 100644 --- a/packages/core/src/render3/i18n/i18n_util.ts +++ b/packages/core/src/render3/i18n/i18n_util.ts @@ -40,9 +40,9 @@ export function getTIcu(tView: TView, index: number): TIcu|null { // either TIcu or TIcuContainerNode. This is not ideal, but we still think it is OK because it // will be just two cases which fits into the browser inline cache (inline cache can take up to // 4) - const tIcu = value.hasOwnProperty('currentCaseLViewIndex') ? + const tIcu: TIcu = value.hasOwnProperty('currentCaseLViewIndex') ? value : - (value as TIcuContainerNode).tagName as any; + (value as TIcuContainerNode).value as any; ngDevMode && assertTIcu(tIcu); return tIcu; } @@ -71,9 +71,7 @@ export function setTIcu(tView: TView, index: number, tIcu: TIcu): void { tView.data[index] = tIcu; } else { ngDevMode && assertNodeType(tNode, TNodeType.IcuContainer); - // FIXME(misko): This is a hack which allows us to associate `TI18n` with `TNode`. - // This should be refactored so that one can attach arbitrary data with `TNode` - tNode.tagName = tIcu as any; + tNode.value = tIcu as any; } } diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 6fc23174f2..01aaa4c4e5 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -188,7 +188,7 @@ function logUnknownElementError( // execute the check below. if (schemas === null) return; - const tagName = tNode.tagName; + const tagName = tNode.value; // If the element matches any directive, it's considered as valid. if (!hasDirectives && tagName !== null) { diff --git a/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts b/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts index 89b4217929..78c239d7cd 100644 --- a/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts +++ b/packages/core/src/render3/instructions/i18n_icu_container_visitor.ts @@ -43,7 +43,7 @@ export function loadIcuContainerVisitor() { // FIXME(misko): This is a hack which allows us to associate `TI18n` with `TNode`. // This should be refactored so that one can attach arbitrary data with `TNode` ngDevMode && assertTNodeForLView(tIcuContainerNode, lView); - const tIcu: TIcu = tIcuContainerNode.tagName as any; + const tIcu: TIcu = tIcuContainerNode.value as any; enterIcu(tIcu, lView); return icuContainerIteratorNext; } diff --git a/packages/core/src/render3/instructions/lview_debug.ts b/packages/core/src/render3/instructions/lview_debug.ts index de52469d60..4d929b0642 100644 --- a/packages/core/src/render3/instructions/lview_debug.ts +++ b/packages/core/src/render3/instructions/lview_debug.ts @@ -184,7 +184,7 @@ class TNode implements ITNode { public propertyBindings: number[]|null, // public flags: TNodeFlags, // public providerIndexes: TNodeProviderIndexes, // - public tagName: string|null, // + public value: string|null, // public attrs: (string|AttributeMarker|(string|SelectorFlags)[])[]|null, // public mergedAttrs: (string|AttributeMarker|(string|SelectorFlags)[])[]|null, // public localNames: (string|number)[]|null, // @@ -256,9 +256,9 @@ class TNode implements ITNode { } get template_(): string { - if (this.tagName === null && this.type === TNodeType.Element) return '#text'; + if (this.value === null && this.type === TNodeType.Element) return '#text'; const buf: string[] = []; - const tagName = typeof this.tagName === 'string' && this.tagName || this.type_; + const tagName = typeof this.value === 'string' && this.value || this.type_; buf.push('<', tagName); if (this.flags) { buf.push(' ', this.flags_); diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 54c2ceb813..3ed54440a8 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -227,7 +227,7 @@ export function getOrCreateTNode( } } else if (tNode.type == TNodeType.Placeholder) { tNode.type = type; - tNode.tagName = name; + tNode.value = name; tNode.attrs = attrs; const parent = getCurrentParentTNode(); tNode.injectorIndex = parent === null ? -1 : parent.injectorIndex; @@ -834,7 +834,7 @@ export function createTNode( tagName: string|null, attrs: TAttributes|null): TNode; export function createTNode( tView: TView, tParent: TElementNode|TContainerNode|null, type: TNodeType, adjustedIndex: number, - tagName: string|null, attrs: TAttributes|null): TNode { + value: string|null, attrs: TAttributes|null): TNode { ngDevMode && assertNotSame(attrs, undefined, '\'undefined\' is not valid value for \'attrs\''); ngDevMode && ngDevMode.tNode++; ngDevMode && tParent && assertTNodeForTView(tParent, tView); @@ -852,7 +852,7 @@ export function createTNode( null, // propertyBindings: number[]|null 0, // flags: TNodeFlags 0, // providerIndexes: TNodeProviderIndexes - tagName, // tagName: string|null + value, // value: string|null attrs, // attrs: (string|AttributeMarker|(string|SelectorFlags)[])[]|null null, // mergedAttrs null, // localNames: (string|number)[]|null @@ -885,7 +885,7 @@ export function createTNode( propertyBindings: null, flags: 0, providerIndexes: 0, - tagName: tagName, + value: value, attrs: attrs, mergedAttrs: null, localNames: null, @@ -1027,7 +1027,7 @@ export function elementPropertyInternal( // It is assumed that the sanitizer is only added when the compiler determines that the // property is risky, so sanitization can be done without further checks. - value = sanitizer != null ? (sanitizer(value, tNode.tagName || '', propName) as any) : value; + value = sanitizer != null ? (sanitizer(value, tNode.value || '', propName) as any) : value; if (isProceduralRenderer(renderer)) { renderer.setProperty(element as RElement, propName, value); } else if (!isAnimationProp(propName)) { @@ -1037,7 +1037,7 @@ export function elementPropertyInternal( } else if (tNode.type === TNodeType.Container || tNode.type === TNodeType.ElementContainer) { // If the node is a container and the property didn't // match any of the inputs or schemas we should throw. - if (ngDevMode && !matchingSchemas(tView, tNode.tagName)) { + if (ngDevMode && !matchingSchemas(tView, tNode.value)) { logUnknownPropertyError(propName, tNode); } } @@ -1104,7 +1104,7 @@ function validateProperty( // The property is considered valid if the element matches the schema, it exists on the element // or it is synthetic, and we are in a browser context (web worker nodes should be skipped). - if (matchingSchemas(tView, tNode.tagName) || propName in element || isAnimationProp(propName)) { + if (matchingSchemas(tView, tNode.value) || propName in element || isAnimationProp(propName)) { return true; } @@ -1135,8 +1135,7 @@ export function matchingSchemas(tView: TView, tagName: string|null): boolean { * @param tNode Node on which we encountered the property. */ function logUnknownPropertyError(propName: string, tNode: TNode): void { - console.error( - `Can't bind to '${propName}' since it isn't a known property of '${tNode.tagName}'.`); + console.error(`Can't bind to '${propName}' since it isn't a known property of '${tNode.value}'.`); } /** @@ -1404,7 +1403,7 @@ function findDirectiveDefMatches( if (ngDevMode) { assertNodeOfPossibleTypes( tNode, [TNodeType.Element], - `"${tNode.tagName}" tags cannot be used as component hosts. ` + + `"${tNode.value}" 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); @@ -1525,7 +1524,7 @@ export function elementAttributeInternal( `Host bindings are not valid on ng-container or ng-template.`); } const element = getNativeByTNode(tNode, lView) as RElement; - setElementAttribute(lView[RENDERER], element, namespace, tNode.tagName, name, value, sanitizer); + setElementAttribute(lView[RENDERER], element, namespace, tNode.value, name, value, sanitizer); } export function setElementAttribute( diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index a8b71f46d7..e682122031 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -427,10 +427,14 @@ export interface TNode { // TODO(misko): break this into actual vars. providerIndexes: TNodeProviderIndexes; - /** The tag name associated with this node. */ - // FIXME(misko): rename to `value` and change the type to `any` so that - // subclasses of `TNode` can use it to link additional payload - tagName: string|null; + /** + * The value name associated with this node. + * if type: + * `TNodeType.Text`: text value + * `TNodeType.Element`: tag name + * `TNodeType.ICUContainer`: `TIcu` + */ + value: string|null; /** * Attributes associated with an element. We need to store attributes to support various use-cases diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts index 8e1bebef8a..9730a741d0 100644 --- a/packages/core/src/render3/node_selector_matcher.ts +++ b/packages/core/src/render3/node_selector_matcher.ts @@ -62,7 +62,7 @@ function isCssClassMatching( * @param tNode current TNode */ export function isInlineTemplate(tNode: TNode): boolean { - return tNode.type === TNodeType.Container && tNode.tagName !== NG_TEMPLATE_SELECTOR; + return tNode.type === TNodeType.Container && tNode.value !== NG_TEMPLATE_SELECTOR; } /** @@ -78,9 +78,8 @@ export function isInlineTemplate(tNode: TNode): boolean { */ function hasTagAndTypeMatch( tNode: TNode, currentSelector: string, isProjectionMode: boolean): boolean { - const tagNameToCompare = tNode.type === TNodeType.Container && !isProjectionMode ? - NG_TEMPLATE_SELECTOR : - tNode.tagName; + const tagNameToCompare = + tNode.type === TNodeType.Container && !isProjectionMode ? NG_TEMPLATE_SELECTOR : tNode.value; return currentSelector === tagNameToCompare; } diff --git a/packages/core/test/acceptance/debug_spec.ts b/packages/core/test/acceptance/debug_spec.ts index 5047501d71..8172c98f90 100644 --- a/packages/core/test/acceptance/debug_spec.ts +++ b/packages/core/test/acceptance/debug_spec.ts @@ -54,7 +54,7 @@ onlyInIvy('Ivy specific').describe('Debug Representation', () => { end: HEADER_OFFSET + 2, length: 2, content: [ - {index: HEADER_OFFSET + 0, t: matchTNode({tagName: 'div'}), l: matchDomElement('div')}, + {index: HEADER_OFFSET + 0, t: matchTNode({value: 'div'}), l: matchDomElement('div')}, {index: HEADER_OFFSET + 1, t: matchTI18n(), l: null}, ] }); @@ -70,7 +70,7 @@ onlyInIvy('Ivy specific').describe('Debug Representation', () => { length: 1, content: [{ index: HEADER_OFFSET + 3, - t: matchTNode({type: TNodeType.Element, tagName: null}), + t: matchTNode({type: TNodeType.Element, value: null}), l: matchDomText('Hello World') }] }); diff --git a/packages/core/test/render3/is_shape_of.ts b/packages/core/test/render3/is_shape_of.ts index 972c6fa487..ac72703c7e 100644 --- a/packages/core/test/render3/is_shape_of.ts +++ b/packages/core/test/render3/is_shape_of.ts @@ -157,7 +157,7 @@ const ShapeOfTNode: ShapeOf = { propertyBindings: true, flags: true, providerIndexes: true, - tagName: true, + value: true, attrs: true, mergedAttrs: true, localNames: true, diff --git a/packages/core/test/render3/matchers_spec.ts b/packages/core/test/render3/matchers_spec.ts index 62fd5c76c4..a9f0e1fc71 100644 --- a/packages/core/test/render3/matchers_spec.ts +++ b/packages/core/test/render3/matchers_spec.ts @@ -60,7 +60,7 @@ describe('render3 matchers', () => { it('should match', () => { expect(tNode).toEqual(matchTNode()); - expect(tNode).toEqual(matchTNode({type: TNodeType.Element, tagName: 'tagName'})); + expect(tNode).toEqual(matchTNode({type: TNodeType.Element, value: 'tagName'})); expect({node: tNode}).toEqual({node: matchTNode({type: TNodeType.Element})}); }); });