Skip to content

Commit

Permalink
fix(merge): Don't merge/override arguments for repeatable directives (#…
Browse files Browse the repository at this point in the history
…5216)

Signed-off-by: Dmitriy Lazarev <w@kich.dev>
  • Loading branch information
wKich authored May 8, 2023
1 parent ad43095 commit 33005c4
Show file tree
Hide file tree
Showing 14 changed files with 166 additions and 53 deletions.
5 changes: 5 additions & 0 deletions .changeset/cyan-ears-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-tools/merge': patch
---

Don't merge/override arguments for repeatable directives
21 changes: 16 additions & 5 deletions packages/merge/src/typedefs-mergers/directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ function directiveAlreadyExists(directivesArr: ReadonlyArray<DirectiveNode>, oth
return !!directivesArr.find(directive => directive.name.value === otherDirective.name.value);
}

function isRepeatableDirective(
directive: DirectiveNode,
directives?: Record<string, DirectiveDefinitionNode>
): boolean {
return !!directives?.[directive.name.value]?.repeatable;
}

function nameAlreadyExists(name: NameNode, namesArr: ReadonlyArray<NameNode>): boolean {
return namesArr.some(({ value }) => value === name.value);
}
Expand Down Expand Up @@ -39,12 +46,15 @@ function mergeArguments(a1: readonly ArgumentNode[], a2: readonly ArgumentNode[]
return result;
}

function deduplicateDirectives(directives: ReadonlyArray<DirectiveNode>): DirectiveNode[] {
function deduplicateDirectives(
directives: ReadonlyArray<DirectiveNode>,
definitions?: Record<string, DirectiveDefinitionNode>
): DirectiveNode[] {
return directives
.map((directive, i, all) => {
const firstAt = all.findIndex(d => d.name.value === directive.name.value);

if (firstAt !== i) {
if (firstAt !== i && !isRepeatableDirective(directive, definitions)) {
const dup = all[firstAt];

(directive as any).arguments = mergeArguments(directive.arguments as any, dup.arguments as any);
Expand All @@ -59,15 +69,16 @@ function deduplicateDirectives(directives: ReadonlyArray<DirectiveNode>): Direct
export function mergeDirectives(
d1: ReadonlyArray<DirectiveNode> = [],
d2: ReadonlyArray<DirectiveNode> = [],
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): DirectiveNode[] {
const reverseOrder: boolean | undefined = config && config.reverseDirectives;
const asNext = reverseOrder ? d1 : d2;
const asFirst = reverseOrder ? d2 : d1;
const result = deduplicateDirectives([...asNext]);
const result = deduplicateDirectives([...asNext], directives);

for (const directive of asFirst) {
if (directiveAlreadyExists(result, directive)) {
if (directiveAlreadyExists(result, directive) && !isRepeatableDirective(directive, directives)) {
const existingDirectiveIndex = result.findIndex(d => d.name.value === directive.name.value);
const existingDirective = result[existingDirectiveIndex];
(result[existingDirectiveIndex] as any).arguments = mergeArguments(
Expand Down
7 changes: 4 additions & 3 deletions packages/merge/src/typedefs-mergers/enum-values.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { EnumValueDefinitionNode } from 'graphql';
import { DirectiveDefinitionNode, EnumValueDefinitionNode } from 'graphql';
import { mergeDirectives } from './directives.js';
import { Config } from './merge-typedefs.js';
import { compareNodes } from '@graphql-tools/utils';

export function mergeEnumValues(
first: ReadonlyArray<EnumValueDefinitionNode> | undefined,
second: ReadonlyArray<EnumValueDefinitionNode> | undefined,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): EnumValueDefinitionNode[] {
if (config?.consistentEnumMerge) {
const reversed: Array<EnumValueDefinitionNode> = [];
Expand All @@ -28,7 +29,7 @@ export function mergeEnumValues(
if (enumValueMap.has(enumValue)) {
const firstValue: any = enumValueMap.get(enumValue);
firstValue.description = secondValue.description || firstValue.description;
firstValue.directives = mergeDirectives(secondValue.directives, firstValue.directives);
firstValue.directives = mergeDirectives(secondValue.directives, firstValue.directives, directives);
} else {
enumValueMap.set(enumValue, secondValue);
}
Expand Down
7 changes: 4 additions & 3 deletions packages/merge/src/typedefs-mergers/enum.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { EnumTypeDefinitionNode, EnumTypeExtensionNode, Kind } from 'graphql';
import { DirectiveDefinitionNode, EnumTypeDefinitionNode, EnumTypeExtensionNode, Kind } from 'graphql';
import { mergeDirectives } from './directives.js';
import { mergeEnumValues } from './enum-values.js';
import { Config } from './merge-typedefs.js';

export function mergeEnum(
e1: EnumTypeDefinitionNode | EnumTypeExtensionNode,
e2: EnumTypeDefinitionNode | EnumTypeExtensionNode,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): EnumTypeDefinitionNode | EnumTypeExtensionNode {
if (e2) {
return {
Expand All @@ -17,7 +18,7 @@ export function mergeEnum(
? 'EnumTypeDefinition'
: 'EnumTypeExtension',
loc: e1.loc,
directives: mergeDirectives(e1.directives, e2.directives, config),
directives: mergeDirectives(e1.directives, e2.directives, config, directives),
values: mergeEnumValues(e1.values, e2.values, config),
} as any;
}
Expand Down
7 changes: 4 additions & 3 deletions packages/merge/src/typedefs-mergers/fields.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Config } from './merge-typedefs.js';
import { FieldDefinitionNode, InputValueDefinitionNode, TypeNode, NameNode } from 'graphql';
import { FieldDefinitionNode, InputValueDefinitionNode, TypeNode, NameNode, DirectiveDefinitionNode } from 'graphql';
import { extractType, isWrappingTypeNode, isListTypeNode, isNonNullTypeNode, printTypeNode } from './utils.js';
import { mergeDirectives } from './directives.js';
import { compareNodes } from '@graphql-tools/utils';
Expand All @@ -25,7 +25,8 @@ export function mergeFields<T extends FieldDefNode>(
type: { name: NameNode },
f1: ReadonlyArray<T> | undefined,
f2: ReadonlyArray<T> | undefined,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): T[] {
const result: T[] = [];
if (f2 != null) {
Expand All @@ -41,7 +42,7 @@ export function mergeFields<T extends FieldDefNode>(
preventConflicts(type, existing, field, config?.throwOnConflict);

newField.arguments = mergeArguments(field['arguments'] || [], existing['arguments'] || [], config);
newField.directives = mergeDirectives(field.directives, existing.directives, config);
newField.directives = mergeDirectives(field.directives, existing.directives, config, directives);
newField.description = field.description || existing.description;
result[existingIndex] = newField;
} else {
Expand Down
13 changes: 10 additions & 3 deletions packages/merge/src/typedefs-mergers/input-type.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import { Config } from './merge-typedefs.js';
import { InputObjectTypeDefinitionNode, InputValueDefinitionNode, InputObjectTypeExtensionNode, Kind } from 'graphql';
import {
InputObjectTypeDefinitionNode,
InputValueDefinitionNode,
InputObjectTypeExtensionNode,
Kind,
DirectiveDefinitionNode,
} from 'graphql';
import { mergeFields } from './fields.js';
import { mergeDirectives } from './directives.js';

export function mergeInputType(
node: InputObjectTypeDefinitionNode | InputObjectTypeExtensionNode,
existingNode: InputObjectTypeDefinitionNode | InputObjectTypeExtensionNode,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): InputObjectTypeDefinitionNode | InputObjectTypeExtensionNode {
if (existingNode) {
try {
Expand All @@ -21,7 +28,7 @@ export function mergeInputType(
: 'InputObjectTypeExtension',
loc: node.loc,
fields: mergeFields<InputValueDefinitionNode>(node, node.fields, existingNode.fields, config),
directives: mergeDirectives(node.directives, existingNode.directives, config),
directives: mergeDirectives(node.directives, existingNode.directives, config, directives),
} as any;
} catch (e: any) {
throw new Error(`Unable to merge GraphQL input type "${node.name.value}": ${e.message}`);
Expand Down
7 changes: 4 additions & 3 deletions packages/merge/src/typedefs-mergers/interface.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { Config } from './merge-typedefs.js';
import { InterfaceTypeDefinitionNode, InterfaceTypeExtensionNode, Kind } from 'graphql';
import { DirectiveDefinitionNode, InterfaceTypeDefinitionNode, InterfaceTypeExtensionNode, Kind } from 'graphql';
import { mergeFields } from './fields.js';
import { mergeDirectives } from './directives.js';
import { mergeNamedTypeArray } from './merge-named-type-array.js';

export function mergeInterface(
node: InterfaceTypeDefinitionNode | InterfaceTypeExtensionNode,
existingNode: InterfaceTypeDefinitionNode | InterfaceTypeExtensionNode,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): InterfaceTypeDefinitionNode | InterfaceTypeExtensionNode {
if (existingNode) {
try {
Expand All @@ -22,7 +23,7 @@ export function mergeInterface(
: 'InterfaceTypeExtension',
loc: node.loc,
fields: mergeFields(node, node.fields, existingNode.fields, config),
directives: mergeDirectives(node.directives, existingNode.directives, config),
directives: mergeDirectives(node.directives, existingNode.directives, config, directives),
interfaces: node['interfaces']
? mergeNamedTypeArray(node['interfaces'], existingNode['interfaces'], config)
: undefined,
Expand Down
22 changes: 13 additions & 9 deletions packages/merge/src/typedefs-mergers/merge-nodes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Config } from './merge-typedefs.js';
import { DefinitionNode, Kind, SchemaDefinitionNode, SchemaExtensionNode } from 'graphql';
import { DefinitionNode, DirectiveDefinitionNode, Kind, SchemaDefinitionNode, SchemaExtensionNode } from 'graphql';
import { mergeType } from './type.js';
import { mergeEnum } from './enum.js';
import { mergeScalar } from './scalar.js';
Expand All @@ -20,8 +20,12 @@ export function isNamedDefinitionNode(definitionNode: DefinitionNode): definitio
return 'name' in definitionNode;
}

export function mergeGraphQLNodes(nodes: ReadonlyArray<DefinitionNode>, config?: Config): MergedResultMap {
const mergedResultMap = {} as MergedResultMap;
export function mergeGraphQLNodes(
nodes: ReadonlyArray<DefinitionNode>,
config?: Config,
directives: Record<string, DirectiveDefinitionNode> = {}
): MergedResultMap {
const mergedResultMap = directives as MergedResultMap;
for (const nodeDefinition of nodes) {
if (isNamedDefinitionNode(nodeDefinition)) {
const name = nodeDefinition.name?.value;
Expand All @@ -39,27 +43,27 @@ export function mergeGraphQLNodes(nodes: ReadonlyArray<DefinitionNode>, config?:
switch (nodeDefinition.kind) {
case Kind.OBJECT_TYPE_DEFINITION:
case Kind.OBJECT_TYPE_EXTENSION:
mergedResultMap[name] = mergeType(nodeDefinition, mergedResultMap[name] as any, config);
mergedResultMap[name] = mergeType(nodeDefinition, mergedResultMap[name] as any, config, directives);
break;
case Kind.ENUM_TYPE_DEFINITION:
case Kind.ENUM_TYPE_EXTENSION:
mergedResultMap[name] = mergeEnum(nodeDefinition, mergedResultMap[name] as any, config);
mergedResultMap[name] = mergeEnum(nodeDefinition, mergedResultMap[name] as any, config, directives);
break;
case Kind.UNION_TYPE_DEFINITION:
case Kind.UNION_TYPE_EXTENSION:
mergedResultMap[name] = mergeUnion(nodeDefinition, mergedResultMap[name] as any, config);
mergedResultMap[name] = mergeUnion(nodeDefinition, mergedResultMap[name] as any, config, directives);
break;
case Kind.SCALAR_TYPE_DEFINITION:
case Kind.SCALAR_TYPE_EXTENSION:
mergedResultMap[name] = mergeScalar(nodeDefinition, mergedResultMap[name] as any, config);
mergedResultMap[name] = mergeScalar(nodeDefinition, mergedResultMap[name] as any, config, directives);
break;
case Kind.INPUT_OBJECT_TYPE_DEFINITION:
case Kind.INPUT_OBJECT_TYPE_EXTENSION:
mergedResultMap[name] = mergeInputType(nodeDefinition, mergedResultMap[name] as any, config);
mergedResultMap[name] = mergeInputType(nodeDefinition, mergedResultMap[name] as any, config, directives);
break;
case Kind.INTERFACE_TYPE_DEFINITION:
case Kind.INTERFACE_TYPE_EXTENSION:
mergedResultMap[name] = mergeInterface(nodeDefinition, mergedResultMap[name] as any, config);
mergedResultMap[name] = mergeInterface(nodeDefinition, mergedResultMap[name] as any, config, directives);
break;
case Kind.DIRECTIVE_DEFINITION:
mergedResultMap[name] = mergeDirective(nodeDefinition, mergedResultMap[name] as any);
Expand Down
46 changes: 36 additions & 10 deletions packages/merge/src/typedefs-mergers/merge-typedefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import {
OperationTypeNode,
isDefinitionNode,
ParseOptions,
DirectiveDefinitionNode,
} from 'graphql';
import { CompareFn, defaultStringComparator, isSourceTypes, isStringTypes } from './utils.js';
import { MergedResultMap, mergeGraphQLNodes, schemaDefSymbol } from './merge-nodes.js';
import { mergeGraphQLNodes, schemaDefSymbol } from './merge-nodes.js';
import {
getDocumentNodeFromSchema,
GetDocumentNodeFromSchemaOptions,
Expand Down Expand Up @@ -135,40 +136,65 @@ export function mergeTypeDefs(typeSource: TypeSource, config?: Partial<Config>):
function visitTypeSources(
typeSource: TypeSource,
options: ParseOptions & GetDocumentNodeFromSchemaOptions,
allDirectives: DirectiveDefinitionNode[] = [],
allNodes: DefinitionNode[] = [],
visitedTypeSources = new Set<TypeSource>()
) {
if (typeSource && !visitedTypeSources.has(typeSource)) {
visitedTypeSources.add(typeSource);
if (typeof typeSource === 'function') {
visitTypeSources(typeSource(), options, allNodes, visitedTypeSources);
visitTypeSources(typeSource(), options, allDirectives, allNodes, visitedTypeSources);
} else if (Array.isArray(typeSource)) {
for (const type of typeSource) {
visitTypeSources(type, options, allNodes, visitedTypeSources);
visitTypeSources(type, options, allDirectives, allNodes, visitedTypeSources);
}
} else if (isSchema(typeSource)) {
const documentNode = getDocumentNodeFromSchema(typeSource, options);
visitTypeSources(documentNode.definitions as DefinitionNode[], options, allNodes, visitedTypeSources);
visitTypeSources(
documentNode.definitions as DefinitionNode[],
options,
allDirectives,
allNodes,
visitedTypeSources
);
} else if (isStringTypes(typeSource) || isSourceTypes(typeSource)) {
const documentNode = parse(typeSource, options);
visitTypeSources(documentNode.definitions as DefinitionNode[], options, allNodes, visitedTypeSources);
visitTypeSources(
documentNode.definitions as DefinitionNode[],
options,
allDirectives,
allNodes,
visitedTypeSources
);
} else if (typeof typeSource === 'object' && isDefinitionNode(typeSource)) {
allNodes.push(typeSource);
if (typeSource.kind === Kind.DIRECTIVE_DEFINITION) {
allDirectives.push(typeSource);
} else {
allNodes.push(typeSource);
}
} else if (isDocumentNode(typeSource)) {
visitTypeSources(typeSource.definitions as DefinitionNode[], options, allNodes, visitedTypeSources);
visitTypeSources(
typeSource.definitions as DefinitionNode[],
options,
allDirectives,
allNodes,
visitedTypeSources
);
} else {
throw new Error(`typeDefs must contain only strings, documents, schemas, or functions, got ${typeof typeSource}`);
}
}
return allNodes;
return { allDirectives, allNodes };
}

export function mergeGraphQLTypes(typeSource: TypeSource, config: Config): DefinitionNode[] {
resetComments();

const allNodes = visitTypeSources(typeSource, config);
const { allDirectives, allNodes } = visitTypeSources(typeSource, config);

const mergedDirectives = mergeGraphQLNodes(allDirectives, config) as Record<string, DirectiveDefinitionNode>;

const mergedNodes: MergedResultMap = mergeGraphQLNodes(allNodes, config);
const mergedNodes = mergeGraphQLNodes(allNodes, config, mergedDirectives);

if (config?.useSchemaDefinition) {
// XXX: right now we don't handle multiple schema definitions
Expand Down
7 changes: 4 additions & 3 deletions packages/merge/src/typedefs-mergers/scalar.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { Kind, ScalarTypeDefinitionNode, ScalarTypeExtensionNode } from 'graphql';
import { DirectiveDefinitionNode, Kind, ScalarTypeDefinitionNode, ScalarTypeExtensionNode } from 'graphql';
import { mergeDirectives } from './directives.js';
import { Config } from './merge-typedefs.js';

export function mergeScalar(
node: ScalarTypeDefinitionNode | ScalarTypeExtensionNode,
existingNode: ScalarTypeDefinitionNode | ScalarTypeExtensionNode,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): ScalarTypeDefinitionNode | ScalarTypeExtensionNode {
if (existingNode) {
return {
Expand All @@ -18,7 +19,7 @@ export function mergeScalar(
? 'ScalarTypeDefinition'
: 'ScalarTypeExtension',
loc: node.loc,
directives: mergeDirectives(node.directives, existingNode.directives, config),
directives: mergeDirectives(node.directives, existingNode.directives, config, directives),
} as any;
}

Expand Down
13 changes: 10 additions & 3 deletions packages/merge/src/typedefs-mergers/schema-def.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { Kind, OperationTypeDefinitionNode, SchemaDefinitionNode, SchemaExtensionNode } from 'graphql';
import {
DirectiveDefinitionNode,
Kind,
OperationTypeDefinitionNode,
SchemaDefinitionNode,
SchemaExtensionNode,
} from 'graphql';
import { mergeDirectives } from './directives.js';
import { Config } from './merge-typedefs.js';

Expand Down Expand Up @@ -26,7 +32,8 @@ function mergeOperationTypes(
export function mergeSchemaDefs(
node: SchemaDefinitionNode | SchemaExtensionNode,
existingNode: SchemaDefinitionNode | SchemaExtensionNode,
config?: Config
config?: Config,
directives?: Record<string, DirectiveDefinitionNode>
): SchemaDefinitionNode | SchemaExtensionNode {
if (existingNode) {
return {
Expand All @@ -35,7 +42,7 @@ export function mergeSchemaDefs(
? Kind.SCHEMA_DEFINITION
: Kind.SCHEMA_EXTENSION,
description: node['description'] || existingNode['description'],
directives: mergeDirectives(node.directives, existingNode.directives, config),
directives: mergeDirectives(node.directives, existingNode.directives, config, directives),
operationTypes: mergeOperationTypes(node.operationTypes, existingNode.operationTypes),
} as any;
}
Expand Down
Loading

0 comments on commit 33005c4

Please sign in to comment.