fix(core): host directive validation not picking up duplicate directi…

…ves on component node

Fixes that, depending on the matching and import order, in some cases we weren't throwing the error saying that a directive matched multiple times on the same element.

Fixes #52072.

packages/core/src/render3/assert.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {RuntimeError, RuntimeErrorCode} from '../errors';
 import {assertDefined, assertEqual, assertNumber, throwError} from '../util/assert';
 
 import {getComponentDef, getNgModuleDef} from './definition';
@@ -140,6 +141,19 @@ export function assertParentView(lView: LView|null, errMessage?: string) {
       errMessage || 'Component views should always have a parent view (component\'s host view)');
 }
 
+export function assertNoDuplicateDirectives(directives: DirectiveDef<unknown>[]): void {
+  const seenDirectives = new Set<DirectiveDef<unknown>>();
+
+  for (const current of directives) {
+    if (seenDirectives.has(current)) {
+      throw new RuntimeError(
+          RuntimeErrorCode.DUPLICATE_DIRECTITVE,
+          `Directive ${current.type.name} matches multiple times on the same element. ` +
+              `Directives can only match an element once.`);
+    }
+    seenDirectives.add(current);
+  }
+}
 
 /**
  * This is a basic sanity check that the `injectorIndex` seems to point to what looks like a

packages/core/src/render3/component_ref.ts

@@ -27,7 +27,7 @@ import {VERSION} from '../version';
 import {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from '../view/provider_flags';
 
 import {AfterRenderEventManager} from './after_render_hooks';
-import {assertComponentType} from './assert';
+import {assertComponentType, assertNoDuplicateDirectives} from './assert';
 import {attachPatchData} from './context_discovery';
 import {getComponentDef} from './definition';
 import {getNodeInjectable, NodeInjector} from './di';
@@ -246,6 +246,7 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
         hostDirectiveDefs = new Map();
         rootComponentDef.findHostDirectiveDefs(rootComponentDef, rootDirectives, hostDirectiveDefs);
         rootDirectives.push(rootComponentDef);
+        ngDevMode && assertNoDuplicateDirectives(rootDirectives);
       } else {
         rootDirectives = [rootComponentDef];
       }

packages/core/src/render3/features/host_directives_feature.ts

@@ -65,7 +65,7 @@ function findHostDirectiveDefs(
       const hostDirectiveDef = getDirectiveDef(hostDirectiveConfig.directive)!;
 
       if (typeof ngDevMode === 'undefined' || ngDevMode) {
-        validateHostDirective(hostDirectiveConfig, hostDirectiveDef, matchedDefs);
+        validateHostDirective(hostDirectiveConfig, hostDirectiveDef);
       }
 
       // We need to patch the `declaredInputs` so that
@@ -144,11 +144,10 @@ function patchDeclaredInputs(
  * Verifies that the host directive has been configured correctly.
  * @param hostDirectiveConfig Host directive configuration object.
  * @param directiveDef Directive definition of the host directive.
- * @param matchedDefs Directives that have been matched so far.
  */
 function validateHostDirective(
-    hostDirectiveConfig: HostDirectiveDef<unknown>, directiveDef: DirectiveDef<any>|null,
-    matchedDefs: DirectiveDef<unknown>[]): asserts directiveDef is DirectiveDef<unknown> {
+    hostDirectiveConfig: HostDirectiveDef<unknown>,
+    directiveDef: DirectiveDef<any>|null): asserts directiveDef is DirectiveDef<unknown> {
   const type = hostDirectiveConfig.directive;
 
   if (directiveDef === null) {
@@ -170,13 +169,6 @@ function validateHostDirective(
         `Host directive ${directiveDef.type.name} must be standalone.`);
   }
 
-  if (matchedDefs.indexOf(directiveDef) > -1) {
-    throw new RuntimeError(
-        RuntimeErrorCode.DUPLICATE_DIRECTITVE,
-        `Directive ${directiveDef.type.name} matches multiple times on the same element. ` +
-            `Directives can only match an element once.`);
-  }
-
   validateMappings('input', directiveDef, hostDirectiveConfig.inputs);
   validateMappings('output', directiveDef, hostDirectiveConfig.outputs);
 }

packages/core/src/render3/instructions/shared.ts

@@ -23,7 +23,7 @@ import {assertDefined, assertEqual, assertGreaterThan, assertGreaterThanOrEqual,
 import {escapeCommentText} from '../../util/dom';
 import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
 import {stringify} from '../../util/stringify';
-import {assertFirstCreatePass, assertFirstUpdatePass, assertLView, assertTNodeForLView, assertTNodeForTView} from '../assert';
+import {assertFirstCreatePass, assertFirstUpdatePass, assertLView, assertNoDuplicateDirectives, assertTNodeForLView, assertTNodeForTView} from '../assert';
 import {attachPatchData} from '../context_discovery';
 import {getFactoryDef} from '../definition_factory';
 import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di';
@@ -1130,6 +1130,7 @@ function findDirectiveDefMatches(
       }
     }
   }
+  ngDevMode && matches !== null && assertNoDuplicateDirectives(matches);
   return matches === null ? null : [matches, hostDirectiveDefs];
 }
 

packages/core/test/acceptance/host_directives_spec.ts

@@ -2921,6 +2921,45 @@ describe('host directives', () => {
               'NG0309: Directive HostDir matches multiple times on the same element. Directives can only match an element once.');
     });
 
+    it('should throw an error if a host directive matches multiple times on a component', () => {
+      @Directive({standalone: true, selector: '[dir]'})
+      class HostDir {
+      }
+
+      @Component({
+        selector: 'comp',
+        hostDirectives: [HostDir],
+        standalone: true,
+        template: '',
+      })
+      class Comp {
+      }
+
+      const baseAppMetadata = {
+        template: '<comp dir></comp>',
+        standalone: true,
+      };
+
+      const expectedError =
+          'NG0309: Directive HostDir matches multiple times on the same element. Directives can only match an element once.';
+
+      // Note: the definition order in `imports` seems to affect the
+      // directive matching order so we test both scenarios.
+      expect(() => {
+        @Component({...baseAppMetadata, imports: [Comp, HostDir]})
+        class App {
+        }
+        TestBed.createComponent(App);
+      }).toThrowError(expectedError);
+
+      expect(() => {
+        @Component({...baseAppMetadata, imports: [HostDir, Comp]})
+        class App {
+        }
+        TestBed.createComponent(App);
+      }).toThrowError(expectedError);
+    });
+
     it('should throw an error if a host directive appears multiple times in a chain', () => {
       @Directive({standalone: true})
       class DuplicateHostDir {