Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
feat(core): de-duplicate host directives
With host directives we can end up in a situation where the same directive applies multiple times to the same element, potentially with conflicting configurations. The runtime isn't set up for a directive to apply more than once so historically we were throwing an error when we detect duplicates.

This ended up limiting the usefulness of host directives to library authors, because it meant that host directives couldn't be reused as much as authors wanted. To address the issue, these changes introduce logic in the compiler and runtime that will de-duplicate host directives with the following logic:

1. If a directive matches once in the template and more than once as a host directive, the host directive matches will be discarded and only the template match will apply. The mental model is that a host directive match represents `Partial<YourDirective>` while a template match represents the full `YourDirective`.
2. If a directive matches multiple times as a host directive, we merge the input/output mappings from all the instances into a single one. If we detect a case where an input/output is exposed under multiple names during the merging process, both the compiler and the runtime will produce an error.

Fixes #57846.
  • Loading branch information
crisbeto committed Apr 2, 2026
commit a76b3d1e1d9a68e262720e79c00b5b101d6dd344
20 changes: 0 additions & 20 deletions packages/core/src/render3/assert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,26 +174,6 @@ export function assertParentView(lView: LView | null, errMessage?: string) {
);
}

export function assertNoDuplicateDirectives(directives: DirectiveDef<unknown>[]): void {
// The array needs at least two elements in order to have duplicates.
if (directives.length < 2) {
return;
}

const seenDirectives = new Set<DirectiveDef<unknown>>();

for (const current of directives) {
if (seenDirectives.has(current)) {
throw new RuntimeError(
RuntimeErrorCode.DUPLICATE_DIRECTIVE,
`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
* NodeInjector data structure.
Expand Down
76 changes: 57 additions & 19 deletions packages/core/src/render3/features/host_directives_feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import {resolveForwardRef} from '../../di';
import {RuntimeError, RuntimeErrorCode} from '../../errors';
import {assertEqual} from '../../util/assert';
import {EMPTY_OBJ} from '../../util/empty';
import {getComponentDef, getDirectiveDef} from '../def_getters';
import {isComponentDef} from '../interfaces/type_checks';
import type {
Expand Down Expand Up @@ -94,7 +93,7 @@ function resolveHostDirectives(matches: DirectiveDef<unknown>[]): HostDirectiveR
hostDirectiveRanges ??= new Map();

// TODO(pk): probably could return matches instead of taking in an array to fill in?
findHostDirectiveDefs(def, allDirectiveDefs, hostDirectiveDefs);
findHostDirectiveDefs(def, allDirectiveDefs, hostDirectiveDefs, matches);

// Note that these indexes are within the offset by `directiveStart`. We can't do the
// offsetting here, because `directiveStart` hasn't been initialized on the TNode yet.
Expand All @@ -113,23 +112,37 @@ function resolveHostDirectives(matches: DirectiveDef<unknown>[]): HostDirectiveR
allDirectiveDefs.push(matches[i]);
}

// We need to patch the `declaredInputs` so that `ngOnChanges` can map the properties correctly.
// Note that we do this at the end so that all host directive inputs have been merged.
if (hostDirectiveDefs !== null) {
hostDirectiveDefs.forEach((def, hostDirectiveDef) => {
patchDeclaredInputs(hostDirectiveDef.declaredInputs, def.inputs);
});
}

return [allDirectiveDefs, hostDirectiveDefs, hostDirectiveRanges];
}

function findHostDirectiveDefs(
currentDef: DirectiveDef<unknown>,
matchedDefs: DirectiveDef<unknown>[],
hostDirectiveDefs: HostDirectiveDefs,
templateMatches: readonly DirectiveDef<unknown>[],
): void {
if (currentDef.hostDirectives !== null) {
for (const configOrFn of currentDef.hostDirectives) {
if (typeof configOrFn === 'function') {
const resolved = configOrFn();
for (const config of resolved) {
trackHostDirectiveDef(createHostDirectiveDef(config), matchedDefs, hostDirectiveDefs);
trackHostDirectiveDef(
createHostDirectiveDef(config),
matchedDefs,
hostDirectiveDefs,
templateMatches,
);
}
} else {
trackHostDirectiveDef(configOrFn, matchedDefs, hostDirectiveDefs);
trackHostDirectiveDef(configOrFn, matchedDefs, hostDirectiveDefs, templateMatches);
}
}
}
Expand All @@ -138,29 +151,56 @@ function findHostDirectiveDefs(
/** Tracks a single host directive during directive matching. */
function trackHostDirectiveDef(
def: HostDirectiveDef,
matchedDefs: DirectiveDef<unknown>[],
finalMatches: DirectiveDef<unknown>[],
hostDirectiveDefs: HostDirectiveDefs,
templateMatches: readonly DirectiveDef<unknown>[],
) {
const hostDirectiveDef = getDirectiveDef(def.directive)!;

if (typeof ngDevMode === 'undefined' || ngDevMode) {
validateHostDirective(def, hostDirectiveDef);
}

// We need to patch the `declaredInputs` so that
// `ngOnChanges` can map the properties correctly.
patchDeclaredInputs(hostDirectiveDef.declaredInputs, def.inputs);

// Host directives execute before the host so that its host bindings can be overwritten.
findHostDirectiveDefs(hostDirectiveDef, matchedDefs, hostDirectiveDefs);
hostDirectiveDefs.set(hostDirectiveDef, def);
matchedDefs.push(hostDirectiveDef);
findHostDirectiveDefs(hostDirectiveDef, finalMatches, hostDirectiveDefs, templateMatches);

if (hostDirectiveDefs.has(hostDirectiveDef)) {
const existing = hostDirectiveDefs.get(hostDirectiveDef)!;
mergeBindingMaps(existing, def.inputs, 'input');
mergeBindingMaps(existing, def.outputs, 'output');
} else if (!templateMatches.includes(hostDirectiveDef)) {
hostDirectiveDefs.set(hostDirectiveDef, def);
finalMatches.push(hostDirectiveDef);
}
}

function mergeBindingMaps(
existingDef: HostDirectiveDef,
newMap: HostDirectiveBindingMap,
kind: 'input' | 'output',
) {
// Note: we don't do something like `existingDef[kind]` to avoid property renaming issues.
const targetMap = kind === 'input' ? existingDef.inputs : existingDef.outputs;

Object.keys(newMap).forEach((publicName) => {
const alias = newMap[publicName];

if (!targetMap.hasOwnProperty(publicName) || targetMap[publicName] === alias) {
targetMap[publicName] = alias;
} else if (typeof ngDevMode === 'undefined' || ngDevMode) {
const message =
`${kind === 'input' ? 'Input' : 'Output'} "${publicName}" from ${existingDef.directive.name} ` +
`is exposed under the following conflicting names: "${targetMap[publicName]}" and "${alias}". ` +
`An ${kind} can only be exposed under a single name.`;
throw new RuntimeError(RuntimeErrorCode.HOST_DIRECTIVE_CONFLICTING_ALIAS, message);
}
});
}

/** Creates a `HostDirectiveDef` from a used-defined host directive configuration. */
function createHostDirectiveDef(config: HostDirectiveConfig): HostDirectiveDef {
return typeof config === 'function'
? {directive: resolveForwardRef(config), inputs: EMPTY_OBJ, outputs: EMPTY_OBJ}
? {directive: resolveForwardRef(config), inputs: {}, outputs: {}}
: {
directive: resolveForwardRef(config.directive),
inputs: bindingArrayToMap(config.inputs),
Expand All @@ -173,14 +213,12 @@ function createHostDirectiveDef(config: HostDirectiveConfig): HostDirectiveDef {
* a map in the form of `{publicName: 'alias', otherPublicName: 'otherAlias'}`.
*/
function bindingArrayToMap(bindings: string[] | undefined): HostDirectiveBindingMap {
if (bindings === undefined || bindings.length === 0) {
return EMPTY_OBJ;
}

const result: HostDirectiveBindingMap = {};

for (let i = 0; i < bindings.length; i += 2) {
result[bindings[i]] = bindings[i + 1];
if (bindings !== undefined && bindings.length > 0) {
for (let i = 0; i < bindings.length; i += 2) {
result[bindings[i]] = bindings[i + 1];
}
}

return result;
Expand Down
Loading
Loading