Skip to content

feat(composition): add composeDirective #2703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
2xrJohann wants to merge 5 commits into from
Closed

feat(composition): add composeDirective #2703

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d70d6a7
feat(composition): add composeDirective
2xrJohann Mar 29, 2026
3ba65a5
feat(composition): add composeDirective
2xrJohann Mar 29, 2026
3d80ca9
feat(composition): add composeDirective
2xrJohann Mar 29, 2026
8616632
feat(composition): add composeDirective
2xrJohann Mar 29, 2026
0eefab2
feat(composition): add composeDirective
2xrJohann Mar 29, 2026
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
29 changes: 29 additions & 0 deletions composition/src/errors/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,35 @@ import {
type TypeName,
} from '../types/types';

// @composeDirective(name: "myDirective") — missing @ prefix
export function composeDirectiveNameMissingAtPrefixError(name: string): Error {
return new Error(
`The name argument of "@composeDirective" must start with "@", but received "${name}".`
);
}

// @composeDirective(name: "@unknownDirective") — directive not defined
export function undefinedComposeDirectiveNameError(name: string): Error {
return new Error(
`The directive "@${name}" declared in "@composeDirective" is not defined in this subgraph.`
);
}

// @composeDirective(name: "@key") — built-in federation directive
export function composeDirectiveBuiltInError(name: string): Error {
return new Error(
`The directive "@${name}" is a built-in federation directive and cannot be used with "@composeDirective".`
);
}

// @composeDirective with conflicting repeatable declarations across subgraphs
export function composeDirectiveRepeatableConflictError(name: string, subgraphNames: Set<string>): Error {
return new Error(
`The composed directive "@${name}" has conflicting "repeatable" declarations across subgraphs: [${[...subgraphNames].join(', ')}].` +
` All subgraphs that define a composed directive must agree on whether it is repeatable.`,
);
}

export const minimumSubgraphRequirementError = new Error('At least one subgraph is required for federation.');

export function multipleNamedTypeDefinitionError(
Expand Down
1 change: 1 addition & 0 deletions composition/src/normalization/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export type NormalizationFailure = {

export type NormalizationSuccess = {
authorizationDataByParentTypeName: Map<string, AuthorizationData>;
composedDirectiveDefinitionDataByDirectiveName: Map<DirectiveName, PersistedDirectiveDefinitionData>;
concreteTypeNamesByAbstractTypeName: Map<string, Set<string>>;
conditionalFieldDataByCoordinates: Map<string, ConditionalFieldData>;
configurationDataByTypeName: Map<TypeName, ConfigurationData>;
Expand Down
1 change: 1 addition & 0 deletions composition/src/schema-building/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ export type ObjectDefinitionData = {
export type PersistedDirectiveDefinitionData = {
argumentDataByName: Map<string, InputValueData>;
executableLocations: Set<string>;
locations?: Set<string>;
name: DirectiveName;
repeatable: boolean;
subgraphNames: Set<SubgraphName>;
Expand Down
51 changes: 39 additions & 12 deletions composition/src/schema-building/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ import {
type PersistedDirectivesData,
type SchemaData,
} from './types';
import { type MutableDefinitionNode, type MutableFieldNode, type MutableInputValueNode } from './ast';
import {
type MutableDefinitionNode,
type MutableDirectiveDefinitionNode,
type MutableFieldNode,
type MutableInputValueNode,
} from './ast';
import { type ObjectTypeNode, setToNameNodeArray, stringToNameNode } from '../ast/utils';
import {
incompatibleInputValueDefaultValuesError,
Expand Down Expand Up @@ -439,7 +444,10 @@ function getRouterPersistedDirectiveNodes<T extends NodeData>(
return persistedDirectiveNodes;
}

export function getClientPersistedDirectiveNodes<T extends NodeData>(nodeData: T): ConstDirectiveNode[] {
export function getClientPersistedDirectiveNodes<T extends NodeData>(
nodeData: T,
composedDirectiveNames?: ReadonlySet<string>,
): ConstDirectiveNode[] {
const persistedDirectiveNodes: Array<ConstDirectiveNode> = [];
if (nodeData.persistedDirectivesData.isDeprecated) {
persistedDirectiveNodes.push(generateDeprecatedDirective(nodeData.persistedDirectivesData.deprecatedReason));
Expand All @@ -451,6 +459,11 @@ export function getClientPersistedDirectiveNodes<T extends NodeData>(nodeData: T
);
continue;
}
if (composedDirectiveNames?.has(directiveName)) {
// Composed directives may be repeatable; all validated nodes are safe to emit.
persistedDirectiveNodes.push(...directiveNodes);
continue;
}
// Only include @deprecated, @oneOf, and @semanticNonNull in the client schema.
if (!PERSISTED_CLIENT_DIRECTIVES.has(directiveName)) {
continue;
Expand All @@ -463,16 +476,19 @@ export function getClientPersistedDirectiveNodes<T extends NodeData>(nodeData: T
return persistedDirectiveNodes;
}

export function getClientSchemaFieldNodeByFieldData(fieldData: FieldData): MutableFieldNode {
const directives = getClientPersistedDirectiveNodes(fieldData);
export function getClientSchemaFieldNodeByFieldData(
fieldData: FieldData,
composedDirectiveNames?: ReadonlySet<string>,
): MutableFieldNode {
const directives = getClientPersistedDirectiveNodes(fieldData, composedDirectiveNames);
const argumentNodes: MutableInputValueNode[] = [];
for (const inputValueData of fieldData.argumentDataByName.values()) {
if (isNodeDataInaccessible(inputValueData)) {
continue;
}
argumentNodes.push({
...inputValueData.node,
directives: getClientPersistedDirectiveNodes(inputValueData),
directives: getClientPersistedDirectiveNodes(inputValueData, composedDirectiveNames),
});
}
return {
Expand Down Expand Up @@ -540,24 +556,35 @@ function addValidatedArgumentNodes(
return true;
}

export function addValidPersistedDirectiveDefinitionNodeByData(
definitions: (MutableDefinitionNode | DefinitionNode)[],
export function buildValidPersistedDirectiveDefinitionNode(
data: PersistedDirectiveDefinitionData,
persistedDirectiveDefinitionByDirectiveName: Map<DirectiveName, DirectiveDefinitionNode>,
errors: Error[],
) {
): MutableDirectiveDefinitionNode | undefined {
const argumentNodes: MutableInputValueNode[] = [];
if (!addValidatedArgumentNodes(argumentNodes, data, persistedDirectiveDefinitionByDirectiveName, errors)) {
return;
return undefined;
}
definitions.push({
return {
arguments: argumentNodes,
kind: Kind.DIRECTIVE_DEFINITION,
locations: setToNameNodeArray(data.executableLocations),
locations: setToNameNodeArray(data.locations ?? data.executableLocations),
name: stringToNameNode(data.name),
repeatable: data.repeatable,
description: data.description,
});
};
}

export function addValidPersistedDirectiveDefinitionNodeByData(
definitions: (MutableDefinitionNode | DefinitionNode)[],
data: PersistedDirectiveDefinitionData,
persistedDirectiveDefinitionByDirectiveName: Map<DirectiveName, DirectiveDefinitionNode>,
errors: Error[],
) {
const node = buildValidPersistedDirectiveDefinitionNode(data, persistedDirectiveDefinitionByDirectiveName, errors);
if (node) {
definitions.push(node);
}
}

type InvalidFieldNames = {
Expand Down
1 change: 1 addition & 0 deletions composition/src/subgraph/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export type SubgraphConfig = {
};

export type InternalSubgraph = {
composedDirectiveDefinitionDataByDirectiveName: Map<DirectiveName, PersistedDirectiveDefinitionData>;
conditionalFieldDataByCoordinates: Map<string, ConditionalFieldData>;
configurationDataByTypeName: Map<TypeName, ConfigurationData>;
definitions: DocumentNode;
Expand Down
3 changes: 1 addition & 2 deletions composition/src/v1/constants/directive-definitions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ export const AUTHENTICATED_DEFINITION: DirectiveDefinitionNode = {
repeatable: false,
};

// @composeDirective is currently unimplemented
/* directive @composeDirective(name: String!) repeatable on SCHEMA */
// directive @composeDirective(name: String!) repeatable on SCHEMA
export const COMPOSE_DIRECTIVE_DEFINITION: DirectiveDefinitionNode = {
arguments: [
{
Expand Down
93 changes: 84 additions & 9 deletions composition/src/v1/federation/federation-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import {
undefinedTypeError,
unexpectedNonCompositeOutputTypeError,
unknownFieldDataError,
composeDirectiveRepeatableConflictError,
unknownFieldSubgraphNameError,
unknownNamedTypeError,
} from '../../errors/errors';
Expand Down Expand Up @@ -138,6 +139,7 @@ import {
} from '../../schema-building/types';
import {
addValidPersistedDirectiveDefinitionNodeByData,
buildValidPersistedDirectiveDefinitionNode,
areKindsEqual,
compareAndValidateInputValueDefaultValues,
generateDeprecatedDirective,
Expand Down Expand Up @@ -298,6 +300,7 @@ export class FederationFactory {
[SEMANTIC_NON_NULL, SEMANTIC_NON_NULL_DEFINITION],
[TAG, TAG_DEFINITION],
]);
composedDirectiveDefinitionDataByDirectiveName = new Map<DirectiveName, PersistedDirectiveDefinitionData>();
potentialPersistedDirectiveDefinitionDataByDirectiveName = new Map<string, PersistedDirectiveDefinitionData>();
referencedPersistedDirectiveNames = new Set<DirectiveName>();
routerDefinitions: Array<MutableDefinitionNode | DefinitionNode> = [];
Expand Down Expand Up @@ -1622,6 +1625,54 @@ export class FederationFactory {
* This method is always necessary, regardless of whether federating a source graph or contract graph.
* */
federateInternalSubgraphData() {
// Pre-pass: register composed directives before any type/field upserts so that
// extractPersistedDirectives can find them when walking type/field nodes.
for (const internalSubgraph of this.internalSubgraphBySubgraphName.values()) {
for (const [directiveName, data] of internalSubgraph.composedDirectiveDefinitionDataByDirectiveName) {
if (!this.persistedDirectiveDefinitionByDirectiveName.has(directiveName)) {
const node = internalSubgraph.directiveDefinitionByName.get(directiveName);
if (node) {
this.persistedDirectiveDefinitionByDirectiveName.set(directiveName, node);
}
}
const existing = this.composedDirectiveDefinitionDataByDirectiveName.get(directiveName);
if (!existing) {
const argumentDataByName = new Map<string, InputValueData>();
for (const inputValueData of data.argumentDataByName.values()) {
this.namedInputValueTypeNames.add(getTypeNodeNamedTypeName(inputValueData.type));
this.upsertInputValueData(argumentDataByName, inputValueData, `@${directiveName}`, false);
}
this.composedDirectiveDefinitionDataByDirectiveName.set(directiveName, {
argumentDataByName,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
executableLocations: new Set(data.executableLocations),
locations: data.locations ? new Set(data.locations) : undefined,
name: data.name,
repeatable: data.repeatable,
subgraphNames: new Set(data.subgraphNames),
description: data.description,
});
} else {
// Intersect locations so only mutually supported locations are emitted
if (existing.locations && data.locations) {
for (const loc of existing.locations) {
if (!data.locations.has(loc)) {
existing.locations.delete(loc);
}
}
}
setMutualExecutableLocations(existing, data.executableLocations);
Comment on lines +1660 to +1668

@coderabbitai coderabbitai Bot Mar 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject directives whose mutual location set goes empty.

This intersection can leave existing.locations empty and still keep the directive definition alive. composition/src/schema-building/utils.ts:559-576 then serializes that set verbatim, so both schemas can end up with a directive definition that has no locations. Please raise a composition error or drop the directive once the mutual location set reaches zero.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@composition/src/v1/federation/federation-factory.ts` around lines 1654 -
1662, The intersection step can leave existing.locations empty but still keep
the directive; modify the logic around existing.locations (the loop that checks
data.locations) and the subsequent call to setMutualExecutableLocations so that
if existing.locations becomes empty you either remove the directive from the
composition set or throw a composition error: after intersecting (the block
referencing existing.locations and data.locations) check existing.locations.size
=== 0 and then (a) drop the directive from whatever map/array holds it (so it
won't be serialized), or (b) raise a composition error with a clear message
indicating the directive lost all mutual locations; ensure this check occurs
before calling setMutualExecutableLocations so executable locations aren't
processed for a directive with no locations.

for (const inputValueData of data.argumentDataByName.values()) {
this.namedInputValueTypeNames.add(getTypeNodeNamedTypeName(inputValueData.type));
this.upsertInputValueData(existing.argumentDataByName, inputValueData, `@${directiveName}`, false);
}
setLongestDescription(existing, data);
if (existing.repeatable !== data.repeatable) {
this.errors.push(composeDirectiveRepeatableConflictError(directiveName, existing.subgraphNames));
}
addIterableToSet({ source: data.subgraphNames, target: existing.subgraphNames });
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
let subgraphNumber = 0;
let shouldSkipPersistedExecutableDirectives = false;
for (const internalSubgraph of this.internalSubgraphBySubgraphName.values()) {
Expand Down Expand Up @@ -1993,6 +2044,7 @@ export class FederationFactory {
}

pushParentDefinitionDataToDocumentDefinitions(interfaceImplementations: InterfaceImplementationData[]) {
const composedDirectiveNames = new Set(this.composedDirectiveDefinitionDataByDirectiveName.keys());

@coderabbitai coderabbitai Bot Mar 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

These client-schema helper calls currently strip composed directive usages.

getClientPersistedDirectiveNodes() in composition/src/schema-building/utils.ts:447-476 skips any directive whose name is present in composedDirectiveNames, and getClientSchemaFieldNodeByFieldData() forwards the same filter. Passing this set at all of these call sites means the client SDL keeps the directive definitions you add later, but drops the actual @composeDirective applications from types, fields, enum values, and arguments. That breaks the main client-schema path for this feature.

Also applies to: 2066-2066, 2113-2113, 2137-2137, 2183-2183, 2209-2209, 2253-2253, 2271-2271, 2290-2290, 2358-2358, 2379-2379

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@composition/src/v1/federation/federation-factory.ts` at line 2047, The code
is passing the composedDirectiveNames set into client-schema helper calls which
causes getClientPersistedDirectiveNodes and getClientSchemaFieldNodeByFieldData
to filter out actual `@composeDirective` usages; remove the composedDirectiveNames
filter from those calls so the helpers preserve directive applications.
Specifically, in federation-factory.ts stop passing the composedDirectiveNames
variable into getClientPersistedDirectiveNodes(...) and
getClientSchemaFieldNodeByFieldData(...), or pass undefined/empty set instead,
and apply the same change to the other call sites where composedDirectiveNames
is currently forwarded so directive usages are retained.

for (const [parentTypeName, parentDefinitionData] of this.parentDefinitionDataByTypeName) {
if (parentDefinitionData.extensionType !== ExtensionType.NONE) {
this.errors.push(noBaseDefinitionForExtensionError(kindToNodeType(parentDefinitionData.kind), parentTypeName));
Expand All @@ -2011,7 +2063,7 @@ export class FederationFactory {
const isValueInaccessible = isNodeDataInaccessible(enumValueData);
const clientEnumValueNode: MutableEnumValueNode = {
...enumValueData.node,
directives: getClientPersistedDirectiveNodes(enumValueData),
directives: getClientPersistedDirectiveNodes(enumValueData, composedDirectiveNames),
};
switch (mergeMethod) {
case MergeMethod.CONSISTENT:
Expand Down Expand Up @@ -2058,7 +2110,7 @@ export class FederationFactory {
}
this.clientDefinitions.push({
...parentDefinitionData.node,
directives: getClientPersistedDirectiveNodes(parentDefinitionData),
directives: getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames),
values: clientEnumValueNodes,
});
break;
Expand All @@ -2082,7 +2134,7 @@ export class FederationFactory {
}
clientInputValueNodes.push({
...inputValueData.node,
directives: getClientPersistedDirectiveNodes(inputValueData),
directives: getClientPersistedDirectiveNodes(inputValueData, composedDirectiveNames),
});
} else if (isTypeRequired(inputValueData.type)) {
invalidRequiredInputs.push({
Expand Down Expand Up @@ -2128,7 +2180,7 @@ export class FederationFactory {
}
this.clientDefinitions.push({
...parentDefinitionData.node,
directives: getClientPersistedDirectiveNodes(parentDefinitionData),
directives: getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames),
fields: clientInputValueNodes,
});
break;
Expand All @@ -2154,7 +2206,7 @@ export class FederationFactory {
if (isNodeDataInaccessible(fieldData)) {
continue;
}
clientSchemaFieldNodes.push(getClientSchemaFieldNodeByFieldData(fieldData));
clientSchemaFieldNodes.push(getClientSchemaFieldNodeByFieldData(fieldData, composedDirectiveNames));
graphFieldDataByFieldName.set(fieldName, this.fieldDataToGraphFieldData(fieldData));
}
if (isObject) {
Expand Down Expand Up @@ -2198,7 +2250,7 @@ export class FederationFactory {
}
this.clientDefinitions.push({
...parentDefinitionData.node,
directives: getClientPersistedDirectiveNodes(parentDefinitionData),
directives: getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames),
fields: clientSchemaFieldNodes,
});
break;
Expand All @@ -2216,7 +2268,7 @@ export class FederationFactory {
}
this.clientDefinitions.push({
...parentDefinitionData.node,
directives: getClientPersistedDirectiveNodes(parentDefinitionData),
directives: getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames),
});
break;
}
Expand All @@ -2235,7 +2287,7 @@ export class FederationFactory {
}
this.clientDefinitions.push({
...parentDefinitionData.node,
directives: getClientPersistedDirectiveNodes(parentDefinitionData),
directives: getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames),
types: clientMembers,
});
break;
Expand Down Expand Up @@ -2303,6 +2355,7 @@ export class FederationFactory {
validateInterfaceImplementationsAndPushToDocumentDefinitions(
interfaceImplementations: InterfaceImplementationData[],
) {
const composedDirectiveNames = new Set(this.composedDirectiveDefinitionDataByDirectiveName.keys());
for (const { data, clientSchemaFieldNodes } of interfaceImplementations) {
data.node.interfaces = this.getValidImplementedInterfaces(data);
this.routerDefinitions.push(this.getNodeForRouterSchemaByData(data));
Expand All @@ -2323,7 +2376,7 @@ export class FederationFactory {
* */
this.clientDefinitions.push({
...data.node,
directives: getClientPersistedDirectiveNodes(data),
directives: getClientPersistedDirectiveNodes(data, composedDirectiveNames),
fields: clientSchemaFieldNodes,
interfaces: clientInterfaces,
});
Expand Down Expand Up @@ -2859,6 +2912,17 @@ export class FederationFactory {
this.errors,
);
}
for (const data of this.composedDirectiveDefinitionDataByDirectiveName.values()) {
const node = buildValidPersistedDirectiveDefinitionNode(
data,
this.persistedDirectiveDefinitionByDirectiveName,
this.errors,
);
if (node) {
this.routerDefinitions.push(node);
this.clientDefinitions.push(node);
}
}
const definitionsWithInterfaces: InterfaceImplementationData[] = [];
this.pushParentDefinitionDataToDocumentDefinitions(definitionsWithInterfaces);
this.validateInterfaceImplementationsAndPushToDocumentDefinitions(definitionsWithInterfaces);
Expand Down Expand Up @@ -3182,6 +3246,17 @@ export class FederationFactory {
this.errors,
);
}
for (const data of this.composedDirectiveDefinitionDataByDirectiveName.values()) {
const node = buildValidPersistedDirectiveDefinitionNode(
data,
this.persistedDirectiveDefinitionByDirectiveName,
this.errors,
);
if (node) {
this.routerDefinitions.push(node);
this.clientDefinitions.push(node);
}
}
const interfaceImplementations: InterfaceImplementationData[] = [];
this.pushParentDefinitionDataToDocumentDefinitions(interfaceImplementations);
this.validateInterfaceImplementationsAndPushToDocumentDefinitions(interfaceImplementations);
Expand Down
Loading
Loading