Skip to content

Commit 35e5bbb

Browse files
refactor(core): reuse setupStaticAttributes in ComponentRef (#59572)
This change refactor how the dynamically created component deals with attributes in order to reuse the existing setupStaticAttributes logic (instead of having specific and similar code). PR Close #59572
1 parent 4e6017a commit 35e5bbb

File tree

14 files changed

+43
-84
lines changed

14 files changed

+43
-84
lines changed

packages/core/src/render3/component_ref.ts

Lines changed: 41 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
import {ComponentFactoryResolver as AbstractComponentFactoryResolver} from '../linker/component_factory_resolver';
2727
import {createElementRef, ElementRef} from '../linker/element_ref';
2828
import {NgModuleRef} from '../linker/ng_module_factory';
29-
import {Renderer2, RendererFactory2} from '../render/api';
29+
import {RendererFactory2} from '../render/api';
3030
import {Sanitizer} from '../sanitization/sanitizer';
3131
import {assertDefined, assertGreaterThan, assertIndexInRange} from '../util/assert';
3232

@@ -56,13 +56,13 @@ import {ComponentDef, DirectiveDef, HostDirectiveDefs} from './interfaces/defini
5656
import {InputFlags} from './interfaces/input_flags';
5757
import {
5858
NodeInputBindings,
59+
TAttributes,
5960
TContainerNode,
6061
TElementContainerNode,
6162
TElementNode,
6263
TNode,
6364
TNodeType,
6465
} from './interfaces/node';
65-
import {Renderer} from './interfaces/renderer';
6666
import {RElement, RNode} from './interfaces/renderer_dom';
6767
import {
6868
CONTEXT,
@@ -75,14 +75,16 @@ import {
7575
TViewType,
7676
} from './interfaces/view';
7777
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
78-
import {setupStaticAttributes, writeDirectClass} from './node_manipulation';
78+
79+
import {createElementNode} from './dom_node_manipulation';
80+
import {setupStaticAttributes} from './node_manipulation';
7981
import {
8082
extractAttrsAndClassesFromSelector,
8183
stringifyCSSSelectorList,
8284
} from './node_selector_matcher';
8385
import {enterView, getCurrentTNode, getLView, leaveView} from './state';
8486
import {computeStaticStyling} from './styling/static_styling';
85-
import {mergeHostAttrs, setUpAttributes} from './util/attrs_utils';
87+
import {mergeHostAttrs} from './util/attrs_utils';
8688
import {debugStringifyTypeForError, stringifyForError} from './util/stringify_utils';
8789
import {getComponentLViewByIndex, getNativeByTNode, getTNode} from './util/view_utils';
8890
import {ViewRef} from './view_ref';
@@ -91,7 +93,8 @@ import {unregisterLView} from './interfaces/lview_tracking';
9193
import {profiler} from './profiler';
9294
import {ProfilerEvent} from './profiler_types';
9395
import {executeContentQueries} from './queries/query_execution';
94-
import {createElementNode} from './dom_node_manipulation';
96+
import {AttributeMarker} from './interfaces/attribute_marker';
97+
import {CssSelector} from './interfaces/projection';
9598

9699
export class ComponentFactoryResolver extends AbstractComponentFactoryResolver {
97100
/**
@@ -160,6 +163,18 @@ function getNamespace(elementName: string): string | null {
160163
return name === 'svg' ? SVG_NAMESPACE : name === 'math' ? MATH_ML_NAMESPACE : null;
161164
}
162165

166+
// TODO(pk): change the extractAttrsAndClassesFromSelector so it returns TAttributes already?
167+
function getRootTAttributesFromSelector(selector: CssSelector) {
168+
const {attrs, classes} = extractAttrsAndClassesFromSelector(selector);
169+
170+
const tAtts: TAttributes = attrs;
171+
if (classes.length) {
172+
tAtts.push(AttributeMarker.Classes, ...classes);
173+
}
174+
175+
return tAtts;
176+
}
177+
163178
/**
164179
* ComponentFactory interface implementation.
165180
*/
@@ -359,27 +374,39 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
359374
}
360375

361376
const hostTNode = createRootComponentTNode(rootLView, hostRNode);
377+
// If host dom element is created (instead of being provided as part of the dynamic component creation), also apply attributes and classes extracted from component selector.
378+
const tAttributes = rootSelectorOrNode
379+
? ['ng-version', '0.0.0-PLACEHOLDER']
380+
: // Extract attributes and classes from the first selector only to match VE behavior.
381+
getRootTAttributesFromSelector(this.componentDef.selectors[0]);
382+
383+
for (const def of rootDirectives) {
384+
hostTNode.mergedAttrs = mergeHostAttrs(hostTNode.mergedAttrs, def.hostAttrs);
385+
}
386+
hostTNode.mergedAttrs = mergeHostAttrs(hostTNode.mergedAttrs, tAttributes);
387+
388+
computeStaticStyling(hostTNode, hostTNode.mergedAttrs, true);
389+
390+
// TODO(crisbeto): in practice `hostRNode` should always be defined, but there are some
391+
// tests where the renderer is mocked out and `undefined` is returned. We should update the
392+
// tests so that this check can be removed.
393+
if (hostRNode) {
394+
setupStaticAttributes(hostRenderer, hostRNode, hostTNode);
395+
}
396+
362397
componentView = createRootComponentView(
363398
hostTNode,
364399
hostRNode,
365400
rootComponentDef,
366401
rootDirectives,
367402
rootLView,
368403
environment,
369-
hostRenderer,
370404
);
371405

372406
tElementNode = getTNode(rootTView, HEADER_OFFSET) as TElementNode;
373407

374-
// TODO(crisbeto): in practice `hostRNode` should always be defined, but there are some
375-
// tests where the renderer is mocked out and `undefined` is returned. We should update the
376-
// tests so that this check can be removed.
377-
if (hostRNode) {
378-
setRootNodeAttributes(hostRenderer, rootComponentDef, hostRNode, rootSelectorOrNode);
379-
}
380-
381408
if (projectableNodes !== undefined) {
382-
projectNodes(tElementNode, this.ngContentSelectors, projectableNodes);
409+
projectNodes(hostTNode, this.ngContentSelectors, projectableNodes);
383410
}
384411

385412
// TODO: should LifecycleHooksFeature and other host features be generated by the compiler
@@ -528,10 +555,8 @@ function createRootComponentView(
528555
rootDirectives: DirectiveDef<any>[],
529556
rootView: LView,
530557
environment: LViewEnvironment,
531-
hostRenderer: Renderer,
532558
): LView {
533559
const tView = rootView[TVIEW];
534-
applyRootComponentStyling(rootDirectives, tNode, hostRNode, hostRenderer);
535560

536561
// Hydration info is on the host element and needs to be retrieved
537562
// and passed to the component LView.
@@ -564,26 +589,6 @@ function createRootComponentView(
564589
return (rootView[tNode.index] = componentView);
565590
}
566591

567-
/** Sets up the styling information on a root component. */
568-
function applyRootComponentStyling(
569-
rootDirectives: DirectiveDef<any>[],
570-
tNode: TElementNode,
571-
rNode: RElement | null,
572-
hostRenderer: Renderer,
573-
): void {
574-
for (const def of rootDirectives) {
575-
tNode.mergedAttrs = mergeHostAttrs(tNode.mergedAttrs, def.hostAttrs);
576-
}
577-
578-
if (tNode.mergedAttrs !== null) {
579-
computeStaticStyling(tNode, tNode.mergedAttrs, true);
580-
581-
if (rNode !== null) {
582-
setupStaticAttributes(hostRenderer, rNode, tNode);
583-
}
584-
}
585-
}
586-
587592
/**
588593
* Creates a root component and sets it up with features and host bindings.Shared by
589594
* renderComponent() and ViewContainerRef.createComponent().
@@ -640,30 +645,6 @@ function createRootComponent<T>(
640645
return component;
641646
}
642647

643-
/** Sets the static attributes on a root component. */
644-
function setRootNodeAttributes(
645-
hostRenderer: Renderer2,
646-
componentDef: ComponentDef<unknown>,
647-
hostRNode: RElement,
648-
rootSelectorOrNode: any,
649-
) {
650-
if (rootSelectorOrNode) {
651-
// The placeholder will be replaced with the actual version at build time.
652-
setUpAttributes(hostRenderer, hostRNode, ['ng-version', '0.0.0-PLACEHOLDER']);
653-
} else {
654-
// If host element is created as a part of this function call (i.e. `rootSelectorOrNode`
655-
// is not defined), also apply attributes and classes extracted from component selector.
656-
// Extract attributes and classes from the first selector only to match VE behavior.
657-
const {attrs, classes} = extractAttrsAndClassesFromSelector(componentDef.selectors[0]);
658-
if (attrs) {
659-
setUpAttributes(hostRenderer, hostRNode, attrs);
660-
}
661-
if (classes && classes.length > 0) {
662-
writeDirectClass(hostRenderer, hostRNode, classes.join(' '));
663-
}
664-
}
665-
}
666-
667648
/** Projects the `projectableNodes` that were specified when creating a root component. */
668649
function projectNodes(
669650
tNode: TElementNode,

packages/core/src/render3/node_manipulation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ function writeDirectStyle(renderer: Renderer, element: RElement, newValue: strin
11691169
* @param element The element which needs to be updated.
11701170
* @param newValue The new class list to write.
11711171
*/
1172-
export function writeDirectClass(renderer: Renderer, element: RElement, newValue: string) {
1172+
function writeDirectClass(renderer: Renderer, element: RElement, newValue: string) {
11731173
ngDevMode && assertString(newValue, "'newValue' should be a string");
11741174
if (newValue === '') {
11751175
// There are tests in `google3` which expect `element.getAttribute('class')` to be `null`.

packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,6 @@
468468
"setIsRefreshingViews",
469469
"setSelectedIndex",
470470
"setStyles",
471-
"setUpAttributes",
472471
"setupStaticAttributes",
473472
"shimStylesContent",
474473
"shouldSearchParent",
@@ -489,7 +488,6 @@
489488
"viewShouldHaveReactiveConsumer",
490489
"visitDslNode",
491490
"walkProviderTree",
492-
"writeDirectClass",
493491
"writeToDirectiveInput",
494492
"ɵɵdefineComponent",
495493
"ɵɵdefineInjectable",

packages/core/test/bundling/animations/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,6 @@
494494
"setIsRefreshingViews",
495495
"setSelectedIndex",
496496
"setStyles",
497-
"setUpAttributes",
498497
"setupStaticAttributes",
499498
"shimStylesContent",
500499
"shouldSearchParent",
@@ -515,7 +514,6 @@
515514
"viewShouldHaveReactiveConsumer",
516515
"visitDslNode",
517516
"walkProviderTree",
518-
"writeDirectClass",
519517
"writeToDirectiveInput",
520518
"ɵɵdefineComponent",
521519
"ɵɵdefineInjectable",

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,6 @@
400400
"setInputsFromAttrs",
401401
"setIsRefreshingViews",
402402
"setSelectedIndex",
403-
"setUpAttributes",
404403
"setupStaticAttributes",
405404
"shimStylesContent",
406405
"shouldSearchParent",
@@ -419,7 +418,6 @@
419418
"viewShouldHaveReactiveConsumer",
420419
"walkProviderTree",
421420
"wasLastNodeCreated",
422-
"writeDirectClass",
423421
"writeToDirectiveInput",
424422
"ɵɵdefineComponent",
425423
"ɵɵdefineInjectable",

packages/core/test/bundling/defer/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,6 @@
858858
"setInputsFromAttrs",
859859
"setIsRefreshingViews",
860860
"setSelectedIndex",
861-
"setUpAttributes",
862861
"setupStaticAttributes",
863862
"shimStylesContent",
864863
"shouldAttachRegularTrigger",
@@ -880,7 +879,6 @@
880879
"viewShouldHaveReactiveConsumer",
881880
"walkProviderTree",
882881
"wasLastNodeCreated",
883-
"writeDirectClass",
884882
"writeToDirectiveInput",
885883
"ɵɵdefer",
886884
"ɵɵdefineComponent",

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,6 @@
596596
"setTStylingRangeNext",
597597
"setTStylingRangeNextDuplicate",
598598
"setTStylingRangePrevDuplicate",
599-
"setUpAttributes",
600599
"setUpControl",
601600
"setUpValidators",
602601
"setupStaticAttributes",
@@ -630,7 +629,6 @@
630629
"walkProviderTree",
631630
"wasLastNodeCreated",
632631
"wrapListener",
633-
"writeDirectClass",
634632
"writeToDirectiveInput",
635633
"{getPrototypeOf:getPrototypeOf,prototype:objectProto,keys:getKeys}",
636634
"{isArray:isArray2}",

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,6 @@
589589
"setTStylingRangeNext",
590590
"setTStylingRangeNextDuplicate",
591591
"setTStylingRangePrevDuplicate",
592-
"setUpAttributes",
593592
"setUpControl",
594593
"setUpValidators",
595594
"setupStaticAttributes",
@@ -623,7 +622,6 @@
623622
"walkProviderTree",
624623
"wasLastNodeCreated",
625624
"wrapListener",
626-
"writeDirectClass",
627625
"writeToDirectiveInput",
628626
"{getPrototypeOf:getPrototypeOf,prototype:objectProto,keys:getKeys}",
629627
"{isArray:isArray2}",

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,6 @@
314314
"setInjectImplementation",
315315
"setIsRefreshingViews",
316316
"setSelectedIndex",
317-
"setUpAttributes",
318317
"shouldSearchParent",
319318
"storeLViewOnDestroy",
320319
"stringify",
@@ -330,7 +329,6 @@
330329
"viewAttachedToChangeDetector",
331330
"viewShouldHaveReactiveConsumer",
332331
"walkProviderTree",
333-
"writeDirectClass",
334332
"writeToDirectiveInput",
335333
"ɵɵdefineInjectable",
336334
"ɵɵdefineInjector",

packages/core/test/bundling/hydration/bundle.golden_symbols.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,6 @@
423423
"setIsRefreshingViews",
424424
"setSegmentHead",
425425
"setSelectedIndex",
426-
"setUpAttributes",
427426
"shimStylesContent",
428427
"shouldSearchParent",
429428
"siblingAfter",
@@ -445,7 +444,6 @@
445444
"viewShouldHaveReactiveConsumer",
446445
"walkProviderTree",
447446
"withDomHydration",
448-
"writeDirectClass",
449447
"writeToDirectiveInput",
450448
"ɵɵdefineComponent",
451449
"ɵɵdefineInjectable",

0 commit comments

Comments
 (0)