Skip to content
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.

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

fix(merge): Don't merge/override arguments for repeatable directives #5216

Merged
merged 1 commit into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
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