Skip to content

Commit

Permalink
Test case mergetypedefs overriding existing fields (#5075)
Browse files Browse the repository at this point in the history
* add test cases for overriding existing fields, queries and mutations

* add test cases for overriding existing fields using extend

* add onFieldTypeConflict to fix field type conflicts manually

* added changeset
  • Loading branch information
simplecommerce authored Feb 28, 2023
1 parent 57d2b6d commit 04e3ecb
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 43 deletions.
5 changes: 5 additions & 0 deletions .changeset/stale-icons-wink.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-tools/merge': minor
---

Added onFieldTypeConflict option to handle manual conflicts for mergeTypeDef and reverseArguments option to select left side arguments if specified.
13 changes: 9 additions & 4 deletions packages/merge/src/typedefs-mergers/arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,24 @@ export function mergeArguments(
args2: InputValueDefinitionNode[],
config?: Config
): InputValueDefinitionNode[] {
const result = deduplicateArguments([...args2, ...args1].filter(isSome));
const result = deduplicateArguments([...args2, ...args1].filter(isSome), config);
if (config && config.sort) {
result.sort(compareNodes);
}
return result;
}

function deduplicateArguments(args: ReadonlyArray<InputValueDefinitionNode>): InputValueDefinitionNode[] {
function deduplicateArguments(
args: ReadonlyArray<InputValueDefinitionNode>,
config?: Config
): InputValueDefinitionNode[] {
return args.reduce<InputValueDefinitionNode[]>((acc, current) => {
const dup = acc.find(arg => arg.name.value === current.name.value);
const dupIndex = acc.findIndex(arg => arg.name.value === current.name.value);

if (!dup) {
if (dupIndex === -1) {
return acc.concat([current]);
} else if (!config?.reverseArguments) {
acc[dupIndex] = current;
}

return acc;
Expand Down
81 changes: 42 additions & 39 deletions packages/merge/src/typedefs-mergers/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,23 @@ import { mergeDirectives } from './directives.js';
import { compareNodes } from '@graphql-tools/utils';
import { mergeArguments } from './arguments.js';

function fieldAlreadyExists(fieldsArr: ReadonlyArray<any>, otherField: any, config?: Config): boolean {
const result: FieldDefinitionNode | null = fieldsArr.find(field => field.name.value === otherField.name.value);
type FieldDefNode = FieldDefinitionNode | InputValueDefinitionNode;
type NamedDefNode = { name: NameNode };

if (result && !config?.ignoreFieldConflicts) {
const t1 = extractType(result.type);
const t2 = extractType(otherField.type);
export type OnFieldTypeConflict = (
existingField: FieldDefNode,
otherField: FieldDefNode,
type: NamedDefNode,
ignoreNullability: boolean | undefined
) => FieldDefNode;

if (t1.name.value !== t2.name.value) {
throw new Error(
`Field "${otherField.name.value}" already defined with a different type. Declared as "${t1.name.value}", but you tried to override with "${t2.name.value}"`
);
}
}
function fieldAlreadyExists(fieldsArr: ReadonlyArray<any>, otherField: any): [FieldDefNode, number] {
const resultIndex: number | null = fieldsArr.findIndex(field => field.name.value === otherField.name.value);

return !!result;
return [resultIndex > -1 ? fieldsArr[resultIndex] : null, resultIndex];
}

export function mergeFields<T extends FieldDefinitionNode | InputValueDefinitionNode>(
export function mergeFields<T extends FieldDefNode>(
type: { name: NameNode },
f1: ReadonlyArray<T> | undefined,
f2: ReadonlyArray<T> | undefined,
Expand All @@ -34,24 +33,17 @@ export function mergeFields<T extends FieldDefinitionNode | InputValueDefinition
}
if (f1 != null) {
for (const field of f1) {
if (fieldAlreadyExists(result, field, config)) {
const existing: any = result.find((f: any) => f.name.value === (field as any).name.value);

if (!config?.ignoreFieldConflicts) {
if (config?.throwOnConflict) {
preventConflicts(type, existing, field, false);
} else {
preventConflicts(type, existing, field, true);
}

if (isNonNullTypeNode(field.type) && !isNonNullTypeNode(existing.type)) {
existing.type = field.type;
}
}

existing.arguments = mergeArguments(field['arguments'] || [], existing.arguments || [], config);
existing.directives = mergeDirectives(field.directives, existing.directives, config);
existing.description = field.description || existing.description;
const [existing, existingIndex] = fieldAlreadyExists(result, field);

if (existing && !config?.ignoreFieldConflicts) {
const newField: any =
(config?.onFieldTypeConflict && config.onFieldTypeConflict(existing, field, type, config?.throwOnConflict)) ||
preventConflicts(type, existing, field, config?.throwOnConflict);

newField.arguments = mergeArguments(field['arguments'] || [], existing['arguments'] || [], config);
newField.directives = mergeDirectives(field.directives, existing.directives, config);
newField.description = field.description || existing.description;
result[existingIndex] = newField;
} else {
result.push(field);
}
Expand All @@ -67,18 +59,29 @@ export function mergeFields<T extends FieldDefinitionNode | InputValueDefinition
return result;
}

function preventConflicts(
type: { name: NameNode },
a: FieldDefinitionNode | InputValueDefinitionNode,
b: FieldDefinitionNode | InputValueDefinitionNode,
ignoreNullability = false
) {
function preventConflicts(type: NamedDefNode, a: FieldDefNode, b: FieldDefNode, ignoreNullability = false) {
const aType = printTypeNode(a.type);
const bType = printTypeNode(b.type);

if (aType !== bType && !safeChangeForFieldType(a.type, b.type, ignoreNullability)) {
throw new Error(`Field '${type.name.value}.${a.name.value}' changed type from '${aType}' to '${bType}'`);
if (aType !== bType) {
const t1 = extractType(a.type);
const t2 = extractType(b.type);

if (t1.name.value !== t2.name.value) {
throw new Error(
`Field "${b.name.value}" already defined with a different type. Declared as "${t1.name.value}", but you tried to override with "${t2.name.value}"`
);
}
if (!safeChangeForFieldType(a.type, b.type, !ignoreNullability)) {
throw new Error(`Field '${type.name.value}.${a.name.value}' changed type from '${aType}' to '${bType}'`);
}
}

if (isNonNullTypeNode(b.type) && !isNonNullTypeNode(a.type)) {
(a as any).type = b.type;
}

return a;
}

function safeChangeForFieldType(oldType: TypeNode, newType: TypeNode, ignoreNullability = false): boolean {
Expand Down
18 changes: 18 additions & 0 deletions packages/merge/src/typedefs-mergers/merge-typedefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
printWithComments,
} from '@graphql-tools/utils';
import { DEFAULT_OPERATION_TYPE_NAME_MAP } from './schema-def.js';
import { OnFieldTypeConflict } from './fields.js';

type Omit<T, K extends keyof any> = Pick<T, Exclude<keyof T, K>>;

Expand Down Expand Up @@ -75,6 +76,23 @@ export interface Config extends ParseOptions, GetDocumentNodeFromSchemaOptions {
convertExtensions?: boolean;
consistentEnumMerge?: boolean;
ignoreFieldConflicts?: boolean;
/**
* Called if types of the same fields are different
*
* Default: false
*
* @example:
* Given:
* ```graphql
* type User { a: String }
* type User { a: Int }
* ```
*
* Instead of throwing `already defined with a different type` error,
* `onFieldTypeConflict` function is called.
*/
onFieldTypeConflict?: OnFieldTypeConflict;
reverseArguments?: boolean;
}

/**
Expand Down
81 changes: 81 additions & 0 deletions packages/merge/tests/merge-typedefs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,87 @@ describe('Merge TypeDefs', () => {
`)
);
});

it('should call onFieldTypeConflict if there are two different types', () => {
const onFieldTypeConflict = jest.fn().mockImplementation((_, r) => r);
const merged = mergeTypeDefs(['type MyType { field: Int! }', 'type MyType { field: String }'], {
onFieldTypeConflict,
});

expect(stripWhitespaces(print(merged))).toBe(
stripWhitespaces(/* GraphQL */ `
type MyType {
field: String
}
`)
);
});

it('should call onFieldTypeConflict if there are two same types but with different nullability', () => {
const onFieldTypeConflict = jest.fn().mockImplementation((_, r) => r);
const merged = mergeTypeDefs(['type MyType { field: Int! }', 'type MyType { field: Int }'], {
onFieldTypeConflict,
});

expect(stripWhitespaces(print(merged))).toBe(
stripWhitespaces(/* GraphQL */ `
type MyType {
field: Int
}
`)
);
});

it('should call onFieldTypeConflict if there are two same mutations with different types', () => {
const onFieldTypeConflict = jest.fn().mockImplementation((_, r) => r);
const merged = mergeTypeDefs(
[
'type Mutation { doSomething(argA: Int!, argB: Int!, argC: Int, argD: Int!, argE: Int!): Boolean! } schema { mutation: Mutation }',
'type Mutation { doSomething(argA: Int!, argB: Int, argC: Int!, argD: String, argF: Boolean): Boolean! } schema { mutation: Mutation }',
],
{
onFieldTypeConflict,
}
);

expect(stripWhitespaces(print(merged))).toBe(
stripWhitespaces(/* GraphQL */ `
type Mutation {
doSomething(argA: Int!, argB: Int, argC: Int!, argD: String, argE: Int!, argF: Boolean): Boolean!
}
schema {
mutation: Mutation
}
`)
);
});

it('should call onFieldTypeConflict if there are two same mutations with different types but preserve original arguments types', () => {
const onFieldTypeConflict = jest.fn().mockImplementation((l, _) => l);
const merged = mergeTypeDefs(
[
'type Mutation { doSomething(argA: Int!, argB: Int!, argC: Int, argD: Int!, argE: Int!): Boolean! } schema { mutation: Mutation }',
'type Mutation { doSomething(argA: Int!, argB: Int, argC: Int!, argD: String, argF: Boolean): Boolean! } schema { mutation: Mutation }',
],
{
onFieldTypeConflict,
reverseArguments: true,
}
);

expect(stripWhitespaces(print(merged))).toBe(
stripWhitespaces(/* GraphQL */ `
type Mutation {
doSomething(argA: Int!, argB: Int!, argC: Int, argD: Int!, argE: Int!, argF: Boolean): Boolean!
}
schema {
mutation: Mutation
}
`)
);
});
});

describe('input arguments', () => {
Expand Down

0 comments on commit 04e3ecb

Please sign in to comment.