Skip to content

Commit 0e5f9ba

Browse files
alan-agius4thePunderWoman
alan-agius4
authored and
thePunderWoman
committed
fix(core): generate consistent component IDs (#48253)
Prior to this change the component IDs where generated based on a counter. This is problematic as there is no guarantee that a component will get the same ID that was assigned on the server when generated on the client side. This is paramount to be able to re-use the component styles generated on the server. PR Close #48253
1 parent 0b71b0a commit 0e5f9ba

File tree

12 files changed

+105
-37
lines changed

12 files changed

+105
-37
lines changed

‎goldens/size-tracking/integration-payloads.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"cli-hello-world": {
33
"uncompressed": {
44
"runtime": 908,
5-
"main": 126280,
5+
"main": 126191,
66
"polyfills": 33792
77
}
88
},

‎packages/core/src/render3/definition.ts‎

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {NG_COMP_DEF, NG_DIR_DEF, NG_MOD_DEF, NG_PIPE_DEF} from './fields';
2020
import {ComponentDef, ComponentDefFeature, ComponentTemplate, ComponentType, ContentQueriesFunction, DependencyTypeList, DirectiveDef, DirectiveDefFeature, DirectiveDefListOrFactory, HostBindingsFunction, PipeDef, PipeDefListOrFactory, TypeOrFactory, ViewQueriesFunction} from './interfaces/definition';
2121
import {TAttributes, TConstantsOrFactory} from './interfaces/node';
2222
import {CssSelectorList} from './interfaces/projection';
23+
import {stringifyCSSSelectorList} from './node_selector_matcher';
2324

2425
interface DirectiveDefinition<T> {
2526
/**
@@ -266,9 +267,6 @@ interface ComponentDefinition<T> extends Omit<DirectiveDefinition<T>, 'features'
266267
schemas?: SchemaMetadata[]|null;
267268
}
268269

269-
/** Counter used to generate unique IDs for component definitions. */
270-
let componentDefCount = 0;
271-
272270
/**
273271
* Create a component definition object.
274272
*
@@ -307,17 +305,19 @@ export function ɵɵdefineComponent<T>(componentDefinition: ComponentDefinition<
307305
getStandaloneInjector: null,
308306
data: componentDefinition.data || {},
309307
encapsulation: componentDefinition.encapsulation || ViewEncapsulation.Emulated,
310-
id: `c${componentDefCount++}`,
311308
styles: componentDefinition.styles || EMPTY_ARRAY,
312309
_: null,
313310
schemas: componentDefinition.schemas || null,
314311
tView: null,
312+
id: getComponentId(def),
315313
};
316314

317315
initFeatures(def);
318316
const dependencies = componentDefinition.dependencies;
319317
def.directiveDefs = extractDefListOrFactory(dependencies, /* pipeDef */ false);
320318
def.pipeDefs = extractDefListOrFactory(dependencies, /* pipeDef */ true);
319+
def.id = getComponentId(def);
320+
321321
return def;
322322
});
323323
}
@@ -655,3 +655,72 @@ function extractDefListOrFactory(
655655
.map(dep => defExtractor(dep))
656656
.filter(nonNull);
657657
}
658+
659+
/**
660+
* A map that contains the generated component IDs and type.
661+
*/
662+
export const GENERATED_COMP_IDS = new Map<string, Type<unknown>>();
663+
664+
/**
665+
* A method can returns a component ID from the component definition using a variant of DJB2 hash
666+
* algorithm.
667+
*/
668+
function getComponentId(componentDef: ComponentDef<unknown>): string {
669+
let hash = 0;
670+
671+
// We cannot rely solely on the component selector as the same selector can be used in different
672+
// modules.
673+
//
674+
// `componentDef.style` is not used, due to it causing inconsistencies. Ex: when server
675+
// component styles has no sourcemaps and browsers do.
676+
//
677+
// Example:
678+
// https://github.com/angular/components/blob/d9f82c8f95309e77a6d82fd574c65871e91354c2/src/material/core/option/option.ts#L248
679+
// https://github.com/angular/components/blob/285f46dc2b4c5b127d356cb7c4714b221f03ce50/src/material/legacy-core/option/option.ts#L32
680+
681+
const hashSelectors = [
682+
componentDef.selectors,
683+
componentDef.ngContentSelectors,
684+
componentDef.hostVars,
685+
componentDef.hostAttrs,
686+
componentDef.consts,
687+
componentDef.vars,
688+
componentDef.decls,
689+
componentDef.encapsulation,
690+
componentDef.standalone,
691+
// We cannot use 'componentDef.type.name' as the name of the symbol will change and will not
692+
// match in the server and browser bundles.
693+
Object.getOwnPropertyNames(componentDef.type.prototype),
694+
!!componentDef.contentQueries,
695+
!!componentDef.viewQuery,
696+
].join('|');
697+
698+
for (const char of hashSelectors) {
699+
hash = Math.imul(31, hash) + char.charCodeAt(0) << 0;
700+
}
701+
702+
// Force positive number hash.
703+
// 2147483647 = equivalent of Integer.MAX_VALUE.
704+
hash += 2147483647 + 1;
705+
706+
const compId = 'c' + hash;
707+
708+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
709+
if (GENERATED_COMP_IDS.has(compId)) {
710+
const previousCompDefType = GENERATED_COMP_IDS.get(compId)!;
711+
if (previousCompDefType !== componentDef.type) {
712+
// TODO: use `formatRuntimeError` to have an error code and we can later on create an error
713+
// guide to explain this further.
714+
console.warn(`Component ID generation collision detected. Components '${
715+
previousCompDefType.name}' and '${componentDef.type.name}' with selector '${
716+
stringifyCSSSelectorList(
717+
componentDef
718+
.selectors)}' generated the same component ID. To fix this, you can change the selector of one of those components or add an extra host attribute to force a different ID.`);
719+
}
720+
} else {
721+
GENERATED_COMP_IDS.set(compId, componentDef.type);
722+
}
723+
}
724+
725+
return compId;
726+
}

‎packages/core/src/render3/jit/module.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {NgModuleDef, NgModuleTransitiveScopes, NgModuleType} from '../../metadat
1919
import {deepForEach, flatten} from '../../util/array_utils';
2020
import {assertDefined} from '../../util/assert';
2121
import {EMPTY_ARRAY} from '../../util/empty';
22-
import {getComponentDef, getDirectiveDef, getNgModuleDef, getPipeDef, isStandalone} from '../definition';
22+
import {GENERATED_COMP_IDS, getComponentDef, getDirectiveDef, getNgModuleDef, getPipeDef, isStandalone} from '../definition';
2323
import {NG_COMP_DEF, NG_DIR_DEF, NG_FACTORY_DEF, NG_MOD_DEF, NG_PIPE_DEF} from '../fields';
2424
import {ComponentDef} from '../interfaces/definition';
2525
import {maybeUnwrapFn} from '../util/misc_utils';
@@ -421,6 +421,7 @@ export function resetCompiledComponents(): void {
421421
ownerNgModule = new WeakMap<Type<any>, NgModuleType<any>>();
422422
verifiedNgModule = new WeakMap<NgModuleType<any>, boolean>();
423423
moduleQueue.length = 0;
424+
GENERATED_COMP_IDS.clear();
424425
}
425426

426427
/**

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -479,9 +479,6 @@
479479
{
480480
"name": "SELF_TOKEN_REGEX"
481481
},
482-
{
483-
"name": "SERVER_TRANSITION_PROVIDERS"
484-
},
485482
{
486483
"name": "SHARED_ANIMATION_PROVIDERS"
487484
},
@@ -551,9 +548,6 @@
551548
{
552549
"name": "TRACKED_LVIEWS"
553550
},
554-
{
555-
"name": "TRANSITION_ID"
556-
},
557551
{
558552
"name": "TRUE_BOOLEAN_VALUES"
559553
},
@@ -749,9 +743,6 @@
749743
{
750744
"name": "collectNativeNodes"
751745
},
752-
{
753-
"name": "componentDefCount"
754-
},
755746
{
756747
"name": "computeStaticStyling"
757748
},
@@ -911,6 +902,9 @@
911902
{
912903
"name": "getComponentDef"
913904
},
905+
{
906+
"name": "getComponentId"
907+
},
914908
{
915909
"name": "getComponentLViewByIndex"
916910
},

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -548,9 +548,6 @@
548548
{
549549
"name": "collectNativeNodes"
550550
},
551-
{
552-
"name": "componentDefCount"
553-
},
554551
{
555552
"name": "computeStaticStyling"
556553
},
@@ -674,6 +671,9 @@
674671
{
675672
"name": "getComponentDef"
676673
},
674+
{
675+
"name": "getComponentId"
676+
},
677677
{
678678
"name": "getComponentLViewByIndex"
679679
},

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -752,9 +752,6 @@
752752
{
753753
"name": "collectStylingFromTAttrs"
754754
},
755-
{
756-
"name": "componentDefCount"
757-
},
758755
{
759756
"name": "compose"
760757
},
@@ -944,6 +941,9 @@
944941
{
945942
"name": "getComponentDef"
946943
},
944+
{
945+
"name": "getComponentId"
946+
},
947947
{
948948
"name": "getComponentLViewByIndex"
949949
},

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,9 +731,6 @@
731731
{
732732
"name": "collectStylingFromTAttrs"
733733
},
734-
{
735-
"name": "componentDefCount"
736-
},
737734
{
738735
"name": "composeAsyncValidators"
739736
},
@@ -911,6 +908,9 @@
911908
{
912909
"name": "getComponentDef"
913910
},
911+
{
912+
"name": "getComponentId"
913+
},
914914
{
915915
"name": "getComponentLViewByIndex"
916916
},

‎packages/core/test/bundling/router/bundle.golden_symbols.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -941,9 +941,6 @@
941941
{
942942
"name": "compare"
943943
},
944-
{
945-
"name": "componentDefCount"
946-
},
947944
{
948945
"name": "computeStaticStyling"
949946
},
@@ -1214,6 +1211,9 @@
12141211
{
12151212
"name": "getComponentDef"
12161213
},
1214+
{
1215+
"name": "getComponentId"
1216+
},
12171217
{
12181218
"name": "getComponentLViewByIndex"
12191219
},

‎packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -494,9 +494,6 @@
494494
{
495495
"name": "collectNativeNodes"
496496
},
497-
{
498-
"name": "componentDefCount"
499-
},
500497
{
501498
"name": "concatStringsWithSpace"
502499
},
@@ -605,6 +602,9 @@
605602
{
606603
"name": "getComponentDef"
607604
},
605+
{
606+
"name": "getComponentId"
607+
},
608608
{
609609
"name": "getComponentLViewByIndex"
610610
},

‎packages/core/test/bundling/todo/bundle.golden_symbols.json‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -444,12 +444,15 @@
444444
"name": "TRACKED_LVIEWS"
445445
},
446446
{
447+
<<<<<<< HEAD
447448
"name": "TRANSITION_ID"
448449
},
449450
{
450451
"name": "TYPE"
451452
},
452453
{
454+
=======
455+
>>>>>>> 0bd0ef957b (fix(core): generate consistent component IDs)
453456
"name": "TemplateRef"
454457
},
455458
{
@@ -650,9 +653,6 @@
650653
{
651654
"name": "collectStylingFromTAttrs"
652655
},
653-
{
654-
"name": "componentDefCount"
655-
},
656656
{
657657
"name": "computeStaticStyling"
658658
},
@@ -800,6 +800,9 @@
800800
{
801801
"name": "getComponentDef"
802802
},
803+
{
804+
"name": "getComponentId"
805+
},
803806
{
804807
"name": "getComponentLViewByIndex"
805808
},
@@ -1454,4 +1457,4 @@
14541457
{
14551458
"name": "ɵɵtextInterpolate1"
14561459
}
1457-
]
1460+
]

0 commit comments

Comments
 (0)