diff --git a/.changeset/seven-jars-yell.md b/.changeset/seven-jars-yell.md new file mode 100644 index 0000000000..3750809802 --- /dev/null +++ b/.changeset/seven-jars-yell.md @@ -0,0 +1,6 @@ +--- +'@graphql-inspector/core': major +--- + +"diff" includes all nested changes when a node is added. Some change types have had additional meta fields added. +On deprecation add with a reason, a separate "fieldDeprecationReasonAdded" change is no longer included. diff --git a/packages/core/__tests__/diff/directive-usage.test.ts b/packages/core/__tests__/diff/directive-usage.test.ts index 7b2117046e..a8d54406aa 100644 --- a/packages/core/__tests__/diff/directive-usage.test.ts +++ b/packages/core/__tests__/diff/directive-usage.test.ts @@ -28,6 +28,29 @@ describe('directive-usage', () => { expect(change.message).toEqual("Directive 'external' was added to field 'Query.a'"); }); + test('added directive on added field', async () => { + const a = buildSchema(/* GraphQL */ ` + type Query { + _: String + } + `); + const b = buildSchema(/* GraphQL */ ` + directive @external on FIELD_DEFINITION + + type Query { + _: String + a: String @external + } + `); + + const changes = await diff(a, b); + const change = findFirstChangeByPath(changes, 'Query.a.external'); + + expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); + expect(change.type).toEqual('DIRECTIVE_USAGE_FIELD_DEFINITION_ADDED'); + expect(change.message).toEqual("Directive 'external' was added to field 'Query.a'"); + }); + test('removed directive', async () => { const a = buildSchema(/* GraphQL */ ` directive @external on FIELD_DEFINITION diff --git a/packages/core/__tests__/diff/enum.test.ts b/packages/core/__tests__/diff/enum.test.ts index a049332db4..3b950766da 100644 --- a/packages/core/__tests__/diff/enum.test.ts +++ b/packages/core/__tests__/diff/enum.test.ts @@ -3,6 +3,54 @@ import { CriticalityLevel, diff, DiffRule } from '../../src/index.js'; import { findFirstChangeByPath } from '../../utils/testing.js'; describe('enum', () => { + test('added', async () => { + const a = buildSchema(/* GraphQL */ ` + type Query { + fieldA: String + } + `); + + const b = buildSchema(/* GraphQL */ ` + type Query { + fieldA: String + } + + enum enumA { + """ + A is the first letter in the alphabet + """ + A + B + } + `); + + const changes = await diff(a, b); + expect(changes.length).toEqual(4); + + { + const change = findFirstChangeByPath(changes, 'enumA'); + expect(change.meta).toMatchObject({ + addedTypeKind: 'EnumTypeDefinition', + addedTypeName: 'enumA', + }); + expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); + expect(change.criticality.reason).not.toBeDefined(); + expect(change.message).toEqual(`Type 'enumA' was added`); + } + + { + const change = findFirstChangeByPath(changes, 'enumA.A'); + expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); + expect(change.criticality.reason).not.toBeDefined(); + expect(change.message).toEqual(`Enum value 'A' was added to enum 'enumA'`); + expect(change.meta).toMatchObject({ + addedEnumValueName: 'A', + enumName: 'enumA', + addedToNewType: true, + }); + } + }); + test('value added', async () => { const a = buildSchema(/* GraphQL */ ` type Query { diff --git a/packages/core/__tests__/diff/input.test.ts b/packages/core/__tests__/diff/input.test.ts index 8f6d2719cf..dd3946b8f9 100644 --- a/packages/core/__tests__/diff/input.test.ts +++ b/packages/core/__tests__/diff/input.test.ts @@ -1,6 +1,6 @@ import { buildSchema } from 'graphql'; import { CriticalityLevel, diff, DiffRule } from '../../src/index.js'; -import { findFirstChangeByPath } from '../../utils/testing.js'; +import { findChangesByPath, findFirstChangeByPath } from '../../utils/testing.js'; describe('input', () => { describe('fields', () => { @@ -38,6 +38,61 @@ describe('input', () => { "Input field 'd' of type 'String' was added to input object type 'Foo'", ); }); + + test('added with a default value', async () => { + const a = buildSchema(/* GraphQL */ ` + input Foo { + a: String! + } + `); + const b = buildSchema(/* GraphQL */ ` + input Foo { + a: String! + b: String! = "B" + } + `); + + const change = findFirstChangeByPath(await diff(a, b), 'Foo.b'); + expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous); + expect(change.type).toEqual('INPUT_FIELD_ADDED'); + expect(change.meta).toMatchObject({ + addedFieldDefault: '"B"', + addedInputFieldName: 'b', + addedInputFieldType: 'String!', + addedToNewType: false, + inputName: 'Foo', + isAddedInputFieldTypeNullable: false, + }); + expect(change.message).toEqual( + `Input field 'b' of type 'String!' with default value '"B"' was added to input object type 'Foo'`, + ); + }); + + test('added to an added input', async () => { + const a = buildSchema(/* GraphQL */ ` + type Query { + _: String + } + `); + const b = buildSchema(/* GraphQL */ ` + type Query { + _: String + } + + input Foo { + a: String! + } + `); + + const change = findFirstChangeByPath(await diff(a, b), 'Foo.a'); + + expect(change.type).toEqual('INPUT_FIELD_ADDED'); + expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); + expect(change.message).toEqual( + "Input field 'a' of type 'String!' was added to input object type 'Foo'", + ); + }); + test('removed', async () => { const a = buildSchema(/* GraphQL */ ` input Foo { diff --git a/packages/core/__tests__/diff/interface.test.ts b/packages/core/__tests__/diff/interface.test.ts index 153fb1bcea..bb39e72b08 100644 --- a/packages/core/__tests__/diff/interface.test.ts +++ b/packages/core/__tests__/diff/interface.test.ts @@ -169,24 +169,24 @@ describe('interface', () => { const changes = await diff(a, b); const change = { a: findFirstChangeByPath(changes, 'Foo.a'), - b: findChangesByPath(changes, 'Foo.b')[1], - c: findChangesByPath(changes, 'Foo.c')[1], + b: findFirstChangeByPath(changes, 'Foo.b'), + c: findFirstChangeByPath(changes, 'Foo.c'), }; // Changed - expect(change.a.criticality.level).toEqual(CriticalityLevel.NonBreaking); expect(change.a.type).toEqual('FIELD_DEPRECATION_REASON_CHANGED'); + expect(change.a.criticality.level).toEqual(CriticalityLevel.NonBreaking); expect(change.a.message).toEqual( "Deprecation reason on field 'Foo.a' has changed from 'OLD' to 'NEW'", ); // Removed - expect(change.b.criticality.level).toEqual(CriticalityLevel.NonBreaking); - expect(change.b.type).toEqual('FIELD_DEPRECATION_REASON_REMOVED'); - expect(change.b.message).toEqual("Deprecation reason was removed from field 'Foo.b'"); + expect(change.b.type).toEqual('FIELD_DEPRECATION_REMOVED'); + expect(change.b.criticality.level).toEqual(CriticalityLevel.Dangerous); + expect(change.b.message).toEqual("Field 'Foo.b' is no longer deprecated"); // Added + expect(change.c.type).toEqual('FIELD_DEPRECATION_ADDED'); expect(change.c.criticality.level).toEqual(CriticalityLevel.NonBreaking); - expect(change.c.type).toEqual('FIELD_DEPRECATION_REASON_ADDED'); - expect(change.c.message).toEqual("Field 'Foo.c' has deprecation reason 'CCC'"); + expect(change.c.message).toEqual("Field 'Foo.c' is deprecated"); }); test('deprecation added / removed', async () => { @@ -219,4 +219,32 @@ describe('interface', () => { expect(change.b.message).toEqual("Field 'Foo.b' is deprecated"); }); }); + + test('deprecation added w/reason', async () => { + const a = buildSchema(/* GraphQL */ ` + interface Foo { + a: String! + } + `); + const b = buildSchema(/* GraphQL */ ` + interface Foo { + a: String! @deprecated(reason: "A is the first letter.") + } + `); + + const changes = await diff(a, b); + + expect(findChangesByPath(changes, 'Foo.a')).toHaveLength(1); + const change = findFirstChangeByPath(changes, 'Foo.a'); + + // added + expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); + expect(change.type).toEqual('FIELD_DEPRECATION_ADDED'); + expect(change.message).toEqual("Field 'Foo.a' is deprecated"); + expect(change.meta).toMatchObject({ + deprecationReason: 'A is the first letter.', + fieldName: 'a', + typeName: 'Foo', + }); + }); }); diff --git a/packages/core/__tests__/diff/object.test.ts b/packages/core/__tests__/diff/object.test.ts index caecacf22d..6e15624e6b 100644 --- a/packages/core/__tests__/diff/object.test.ts +++ b/packages/core/__tests__/diff/object.test.ts @@ -31,11 +31,18 @@ describe('object', () => { } `); - const change = findFirstChangeByPath(await diff(a, b), 'B'); - const mutation = findFirstChangeByPath(await diff(a, b), 'Mutation'); + const changes = await diff(a, b); + expect(changes).toHaveLength(4); + + const change = findFirstChangeByPath(changes, 'B'); + const mutation = findFirstChangeByPath(changes, 'Mutation'); expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); expect(mutation.criticality.level).toEqual(CriticalityLevel.NonBreaking); + expect(change.meta).toMatchObject({ + addedTypeKind: 'ObjectTypeDefinition', + addedTypeName: 'B', + }); }); describe('interfaces', () => { @@ -63,7 +70,8 @@ describe('object', () => { b: String! } - interface C { + interface C implements B { + b: String! c: String! } @@ -74,11 +82,43 @@ describe('object', () => { } `); - const change = findFirstChangeByPath(await diff(a, b), 'Foo'); + const changes = await diff(a, b); + + { + const change = findFirstChangeByPath(changes, 'Foo'); + expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous); + expect(change.type).toEqual('OBJECT_TYPE_INTERFACE_ADDED'); + expect(change.message).toEqual("'Foo' object implements 'C' interface"); + expect(change.meta).toMatchObject({ + addedInterfaceName: 'C', + objectTypeName: 'Foo', + }); + } + + const cChanges = findChangesByPath(changes, 'C'); + expect(cChanges).toHaveLength(2); + { + const change = cChanges[0]; + expect(change.type).toEqual('TYPE_ADDED'); + expect(change.meta).toMatchObject({ + addedTypeKind: 'InterfaceTypeDefinition', + addedTypeName: 'C', + }); + } + + { + const change = cChanges[1]; + expect(change.type).toEqual('OBJECT_TYPE_INTERFACE_ADDED'); + expect(change.meta).toMatchObject({ + addedInterfaceName: 'B', + objectTypeName: 'C', + }); + } - expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous); - expect(change.type).toEqual('OBJECT_TYPE_INTERFACE_ADDED'); - expect(change.message).toEqual("'Foo' object implements 'C' interface"); + { + const change = findFirstChangeByPath(changes, 'C.b'); + expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking); + } }); test('removed', async () => { @@ -290,24 +330,24 @@ describe('object', () => { const changes = await diff(a, b); const change = { a: findFirstChangeByPath(changes, 'Foo.a'), - b: findChangesByPath(changes, 'Foo.b')[1], - c: findChangesByPath(changes, 'Foo.c')[1], + b: findFirstChangeByPath(changes, 'Foo.b'), + c: findFirstChangeByPath(changes, 'Foo.c'), }; // Changed - expect(change.a.criticality.level).toEqual(CriticalityLevel.NonBreaking); expect(change.a.type).toEqual('FIELD_DEPRECATION_REASON_CHANGED'); + expect(change.a.criticality.level).toEqual(CriticalityLevel.NonBreaking); expect(change.a.message).toEqual( "Deprecation reason on field 'Foo.a' has changed from 'OLD' to 'NEW'", ); // Removed - expect(change.b.criticality.level).toEqual(CriticalityLevel.NonBreaking); - expect(change.b.type).toEqual('FIELD_DEPRECATION_REASON_REMOVED'); - expect(change.b.message).toEqual("Deprecation reason was removed from field 'Foo.b'"); + expect(change.b.type).toEqual('FIELD_DEPRECATION_REMOVED'); + expect(change.b.criticality.level).toEqual(CriticalityLevel.Dangerous); + expect(change.b.message).toEqual("Field 'Foo.b' is no longer deprecated"); // Added + expect(change.c.type).toEqual('FIELD_DEPRECATION_ADDED'); expect(change.c.criticality.level).toEqual(CriticalityLevel.NonBreaking); - expect(change.c.type).toEqual('FIELD_DEPRECATION_REASON_ADDED'); - expect(change.c.message).toEqual("Field 'Foo.c' has deprecation reason 'CCC'"); + expect(change.c.message).toEqual("Field 'Foo.c' is deprecated"); }); test('deprecation added / removed', async () => { diff --git a/packages/core/__tests__/diff/rules/ignore-nested-additions.test.ts b/packages/core/__tests__/diff/rules/ignore-nested-additions.test.ts new file mode 100644 index 0000000000..c7079705fa --- /dev/null +++ b/packages/core/__tests__/diff/rules/ignore-nested-additions.test.ts @@ -0,0 +1,144 @@ +import { buildSchema } from 'graphql'; +import { ignoreNestedAdditions } from '../../../src/diff/rules/index.js'; +import { ChangeType, CriticalityLevel, diff } from '../../../src/index.js'; +import { findFirstChangeByPath } from '../../../utils/testing.js'; + +describe('ignoreNestedAdditions rule', () => { + test('added field on new object', async () => { + const a = buildSchema(/* GraphQL */ ` + scalar A + `); + const b = buildSchema(/* GraphQL */ ` + scalar A + type Foo { + a(b: String): String! @deprecated(reason: "As a test") + } + `); + + const changes = await diff(a, b, [ignoreNestedAdditions]); + expect(changes).toHaveLength(1); + + const added = findFirstChangeByPath(changes, 'Foo.a'); + expect(added).toBe(undefined); + }); + + test('added field on new interface', async () => { + const a = buildSchema(/* GraphQL */ ` + scalar A + `); + const b = buildSchema(/* GraphQL */ ` + scalar A + interface Foo { + a(b: String): String! @deprecated(reason: "As a test") + } + `); + + const changes = await diff(a, b, [ignoreNestedAdditions]); + expect(changes).toHaveLength(1); + + const added = findFirstChangeByPath(changes, 'Foo.a'); + expect(added).toBe(undefined); + }); + + test('added value on new enum', async () => { + const a = buildSchema(/* GraphQL */ ` + scalar A + `); + const b = buildSchema(/* GraphQL */ ` + scalar A + """ + Here is a new enum named B + """ + enum B { + """ + It has newly added values + """ + C @deprecated(reason: "With deprecations") + D + } + `); + + const changes = await diff(a, b, [ignoreNestedAdditions]); + + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ + criticality: { + level: CriticalityLevel.NonBreaking, + }, + message: "Type 'B' was added", + meta: { + addedTypeKind: 'EnumTypeDefinition', + addedTypeName: 'B', + }, + path: 'B', + type: ChangeType.TypeAdded, + }); + }); + + test('added argument / directive / deprecation / reason on new field', async () => { + const a = buildSchema(/* GraphQL */ ` + scalar A + type Foo { + a: String! + } + `); + const b = buildSchema(/* GraphQL */ ` + scalar A + type Foo { + a: String! + b(b: String): String! @deprecated(reason: "As a test") + } + `); + + const changes = await diff(a, b, [ignoreNestedAdditions]); + expect(changes).toHaveLength(1); + + const added = findFirstChangeByPath(changes, 'Foo.b'); + expect(added.type).toBe(ChangeType.FieldAdded); + }); + + test('added type / directive / directive argument on new union', async () => { + const a = buildSchema(/* GraphQL */ ` + scalar A + `); + const b = buildSchema(/* GraphQL */ ` + scalar A + directive @special(reason: String) on UNION + + type Foo { + a: String! + } + + union FooUnion @special(reason: "As a test") = Foo + `); + + const changes = await diff(a, b, [ignoreNestedAdditions]); + expect(changes).toHaveLength(3); + + { + const added = findFirstChangeByPath(changes, 'FooUnion'); + expect(added.type).toBe(ChangeType.TypeAdded); + } + + { + const added = findFirstChangeByPath(changes, 'Foo'); + expect(added.type).toBe(ChangeType.TypeAdded); + } + }); + + test('added argument / location / description on new directive', async () => { + const a = buildSchema(/* GraphQL */ ` + scalar A + `); + const b = buildSchema(/* GraphQL */ ` + scalar A + directive @special(reason: String) on UNION | FIELD_DEFINITION + `); + + const changes = await diff(a, b, [ignoreNestedAdditions]); + expect(changes).toHaveLength(1); + + const added = findFirstChangeByPath(changes, '@special'); + expect(added.type).toBe(ChangeType.DirectiveAdded); + }); +}); diff --git a/packages/core/__tests__/diff/schema.test.ts b/packages/core/__tests__/diff/schema.test.ts index 90392ece63..29f7356242 100644 --- a/packages/core/__tests__/diff/schema.test.ts +++ b/packages/core/__tests__/diff/schema.test.ts @@ -1,6 +1,7 @@ import { buildClientSchema, buildSchema, introspectionFromSchema } from 'graphql'; import { Change, CriticalityLevel, diff } from '../../src/index.js'; import { findBestMatch } from '../../src/utils/string.js'; +import { findChangesByPath, findFirstChangeByPath } from '../../utils/testing.js'; test('same schema', async () => { const schemaA = buildSchema(/* GraphQL */ ` @@ -820,9 +821,9 @@ test('adding root type should not be breaking', async () => { `); const changes = await diff(schemaA, schemaB); - const subscription = changes[0]; + expect(changes).toHaveLength(2); - expect(changes).toHaveLength(1); + const subscription = findFirstChangeByPath(changes, 'Subscription'); expect(subscription).toBeDefined(); expect(subscription!.criticality.level).toEqual(CriticalityLevel.NonBreaking); }); diff --git a/packages/core/src/diff/argument.ts b/packages/core/src/diff/argument.ts index a93652c860..0902790042 100644 --- a/packages/core/src/diff/argument.ts +++ b/packages/core/src/diff/argument.ts @@ -17,44 +17,49 @@ import { AddChange } from './schema.js'; export function changesInArgument( type: GraphQLObjectType | GraphQLInterfaceType, field: GraphQLField, - oldArg: GraphQLArgument, + oldArg: GraphQLArgument | null, newArg: GraphQLArgument, addChange: AddChange, ) { - if (isNotEqual(oldArg.description, newArg.description)) { + if (isNotEqual(oldArg?.description, newArg.description)) { addChange(fieldArgumentDescriptionChanged(type, field, oldArg, newArg)); } - if (isNotEqual(oldArg.defaultValue, newArg.defaultValue)) { - if (Array.isArray(oldArg.defaultValue) && Array.isArray(newArg.defaultValue)) { + if (isNotEqual(oldArg?.defaultValue, newArg.defaultValue)) { + if (Array.isArray(oldArg?.defaultValue) && Array.isArray(newArg.defaultValue)) { const diff = diffArrays(oldArg.defaultValue, newArg.defaultValue); if (diff.length > 0) { addChange(fieldArgumentDefaultChanged(type, field, oldArg, newArg)); } - } else if (JSON.stringify(oldArg.defaultValue) !== JSON.stringify(newArg.defaultValue)) { + } else if (JSON.stringify(oldArg?.defaultValue) !== JSON.stringify(newArg.defaultValue)) { addChange(fieldArgumentDefaultChanged(type, field, oldArg, newArg)); } } - if (isNotEqual(oldArg.type.toString(), newArg.type.toString())) { + if (isNotEqual(oldArg?.type.toString(), newArg.type.toString())) { addChange(fieldArgumentTypeChanged(type, field, oldArg, newArg)); } - if (oldArg.astNode?.directives && newArg.astNode?.directives) { - compareLists(oldArg.astNode.directives || [], newArg.astNode.directives || [], { + if (newArg.astNode?.directives) { + compareLists(oldArg?.astNode?.directives || [], newArg.astNode.directives || [], { onAdded(directive) { addChange( - directiveUsageAdded(Kind.ARGUMENT, directive, { - argument: newArg, - field, - type, - }), + directiveUsageAdded( + Kind.ARGUMENT, + directive, + { + argument: newArg, + field, + type, + }, + oldArg === null, + ), ); }, onRemoved(directive) { addChange( - directiveUsageRemoved(Kind.ARGUMENT, directive, { argument: oldArg, field, type }), + directiveUsageRemoved(Kind.ARGUMENT, directive, { argument: oldArg!, field, type }), ); }, }); diff --git a/packages/core/src/diff/changes/argument.ts b/packages/core/src/diff/changes/argument.ts index 91109c475f..049147b433 100644 --- a/packages/core/src/diff/changes/argument.ts +++ b/packages/core/src/diff/changes/argument.ts @@ -33,7 +33,7 @@ export function fieldArgumentDescriptionChangedFromMeta( export function fieldArgumentDescriptionChanged( type: GraphQLObjectType | GraphQLInterfaceType, field: GraphQLField, - oldArg: GraphQLArgument, + oldArg: GraphQLArgument | null, newArg: GraphQLArgument, ): Change { return fieldArgumentDescriptionChangedFromMeta({ @@ -41,8 +41,8 @@ export function fieldArgumentDescriptionChanged( meta: { typeName: type.name, fieldName: field.name, - argumentName: oldArg.name, - oldDescription: oldArg.description ?? null, + argumentName: newArg.name, + oldDescription: oldArg?.description ?? null, newDescription: newArg.description ?? null, }, }); @@ -75,7 +75,7 @@ export function fieldArgumentDefaultChangedFromMeta(args: FieldArgumentDefaultCh export function fieldArgumentDefaultChanged( type: GraphQLObjectType | GraphQLInterfaceType, field: GraphQLField, - oldArg: GraphQLArgument, + oldArg: GraphQLArgument | null, newArg: GraphQLArgument, ): Change { const meta: FieldArgumentDefaultChangedChange['meta'] = { @@ -84,7 +84,7 @@ export function fieldArgumentDefaultChanged( argumentName: newArg.name, }; - if (oldArg.defaultValue !== undefined) { + if (oldArg?.defaultValue !== undefined) { meta.oldDefaultValue = safeString(oldArg.defaultValue); } if (newArg.defaultValue !== undefined) { @@ -127,7 +127,7 @@ export function fieldArgumentTypeChangedFromMeta(args: FieldArgumentTypeChangedC export function fieldArgumentTypeChanged( type: GraphQLObjectType | GraphQLInterfaceType, field: GraphQLField, - oldArg: GraphQLArgument, + oldArg: GraphQLArgument | null, newArg: GraphQLArgument, ): Change { return fieldArgumentTypeChangedFromMeta({ @@ -136,9 +136,9 @@ export function fieldArgumentTypeChanged( typeName: type.name, fieldName: field.name, argumentName: newArg.name, - oldArgumentType: oldArg.type.toString(), + oldArgumentType: oldArg?.type.toString() ?? '', newArgumentType: newArg.type.toString(), - isSafeArgumentTypeChange: safeChangeForInputValue(oldArg.type, newArg.type), + isSafeArgumentTypeChange: !oldArg || safeChangeForInputValue(oldArg.type, newArg.type), }, }); } diff --git a/packages/core/src/diff/changes/change.ts b/packages/core/src/diff/changes/change.ts index 8ef6ea7181..fc62e7a010 100644 --- a/packages/core/src/diff/changes/change.ts +++ b/packages/core/src/diff/changes/change.ts @@ -1,3 +1,5 @@ +import { Kind } from 'graphql'; + export enum CriticalityLevel { Breaking = 'BREAKING', NonBreaking = 'NON_BREAKING', @@ -159,6 +161,9 @@ export type DirectiveAddedChange = { type: typeof ChangeType.DirectiveAdded; meta: { addedDirectiveName: string; + addedDirectiveRepeatable: boolean; + addedDirectiveLocations: string[]; + addedDirectiveDescription: string | null; }; }; @@ -193,6 +198,10 @@ export type DirectiveArgumentAddedChange = { directiveName: string; addedDirectiveArgumentName: string; addedDirectiveArgumentTypeIsNonNull: boolean; + addedToNewDirective: boolean; + addedDirectiveArgumentDescription: string | null; + addedDirectiveArgumentType: string; + addedDirectiveDefaultValue?: string /* | null */; }; }; @@ -252,6 +261,8 @@ export type EnumValueAddedChange = { meta: { enumName: string; addedEnumValueName: string; + addedToNewType: boolean; + addedDirectiveDescription: string | null; }; }; @@ -311,6 +322,7 @@ export type FieldAddedChange = { typeName: string; addedFieldName: string; typeType: string; + addedFieldReturnType: string; }; }; @@ -346,6 +358,7 @@ export type FieldDeprecationAddedChange = { meta: { typeName: string; fieldName: string; + deprecationReason: string; }; }; @@ -401,6 +414,7 @@ export type DirectiveUsageUnionMemberAddedChange = { unionName: string; addedUnionMemberTypeName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -422,6 +436,7 @@ export type FieldArgumentAddedChange = { addedArgumentType: string; hasDefaultValue: boolean; isAddedFieldArgumentBreaking: boolean; + addedToNewField: boolean; }; }; @@ -453,6 +468,8 @@ export type InputFieldAddedChange = { addedInputFieldName: string; isAddedInputFieldTypeNullable: boolean; addedInputFieldType: string; + addedFieldDefault?: string; + addedToNewType: boolean; }; }; @@ -512,6 +529,7 @@ export type ObjectTypeInterfaceAddedChange = { meta: { objectTypeName: string; addedInterfaceName: string; + addedToNewType: boolean; }; }; @@ -558,11 +576,26 @@ export type TypeRemovedChange = { }; }; +type TypeAddedMeta = { + addedTypeName: string; + addedTypeKind: K; +}; + +type InputAddedMeta = TypeAddedMeta & { + addedTypeIsOneOf: boolean; +}; + export type TypeAddedChange = { type: typeof ChangeType.TypeAdded; - meta: { - addedTypeName: string; - }; + meta: + | InputAddedMeta + | TypeAddedMeta< + | Kind.ENUM_TYPE_DEFINITION + | Kind.OBJECT_TYPE_DEFINITION + | Kind.INTERFACE_TYPE_DEFINITION + | Kind.UNION_TYPE_DEFINITION + | Kind.SCALAR_TYPE_DEFINITION + >; }; export type TypeKindChangedChange = { @@ -614,6 +647,7 @@ export type UnionMemberAddedChange = { meta: { unionName: string; addedUnionMemberTypeName: string; + addedToNewType: boolean; }; }; @@ -624,6 +658,7 @@ export type DirectiveUsageEnumAddedChange = { meta: { enumName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -641,6 +676,7 @@ export type DirectiveUsageEnumValueAddedChange = { enumName: string; enumValueName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -672,6 +708,7 @@ export type DirectiveUsageInputObjectAddedChange = { isAddedInputFieldTypeNullable: boolean; addedInputFieldType: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -681,6 +718,7 @@ export type DirectiveUsageInputFieldDefinitionAddedChange = { inputObjectName: string; inputFieldName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -716,6 +754,7 @@ export type DirectiveUsageScalarAddedChange = { meta: { scalarName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -732,6 +771,7 @@ export type DirectiveUsageObjectAddedChange = { meta: { objectName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -748,6 +788,7 @@ export type DirectiveUsageInterfaceAddedChange = { meta: { interfaceName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -756,6 +797,7 @@ export type DirectiveUsageSchemaAddedChange = { meta: { addedDirectiveName: string; schemaTypeName: string; + addedToNewType: boolean; }; }; @@ -773,6 +815,7 @@ export type DirectiveUsageFieldDefinitionAddedChange = { typeName: string; fieldName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; @@ -792,6 +835,7 @@ export type DirectiveUsageArgumentDefinitionChange = { fieldName: string; argumentName: string; addedDirectiveName: string; + addedToNewType: boolean; }; }; diff --git a/packages/core/src/diff/changes/directive-usage.ts b/packages/core/src/diff/changes/directive-usage.ts index 00e0b165d4..0ca2ea863a 100644 --- a/packages/core/src/diff/changes/directive-usage.ts +++ b/packages/core/src/diff/changes/directive-usage.ts @@ -138,7 +138,9 @@ export function directiveUsageArgumentDefinitionAddedFromMeta( ) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to argument '${args.meta.argumentName}'`, }, type: ChangeType.DirectiveUsageArgumentDefinitionAdded, @@ -188,7 +190,9 @@ function buildDirectiveUsageInputObjectAddedMessage( export function directiveUsageInputObjectAddedFromMeta(args: DirectiveUsageInputObjectAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to input object '${args.meta.inputObjectName}'`, }, type: ChangeType.DirectiveUsageInputObjectAdded, @@ -228,7 +232,9 @@ function buildDirectiveUsageInterfaceAddedMessage( export function directiveUsageInterfaceAddedFromMeta(args: DirectiveUsageInterfaceAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to interface '${args.meta.interfaceName}'`, }, type: ChangeType.DirectiveUsageInterfaceAdded, @@ -268,7 +274,9 @@ export function directiveUsageInputFieldDefinitionAddedFromMeta( ) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to input field '${args.meta.inputFieldName}'`, }, type: ChangeType.DirectiveUsageInputFieldDefinitionAdded, @@ -314,7 +322,9 @@ function buildDirectiveUsageObjectAddedMessage( export function directiveUsageObjectAddedFromMeta(args: DirectiveUsageObjectAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to object '${args.meta.objectName}'`, }, type: ChangeType.DirectiveUsageObjectAdded, @@ -350,7 +360,9 @@ function buildDirectiveUsageEnumAddedMessage(args: DirectiveUsageEnumAddedChange export function directiveUsageEnumAddedFromMeta(args: DirectiveUsageEnumAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to enum '${args.meta.enumName}'`, }, type: ChangeType.DirectiveUsageEnumAdded, @@ -390,7 +402,9 @@ export function directiveUsageFieldDefinitionAddedFromMeta( ) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to field '${args.meta.fieldName}'`, }, type: ChangeType.DirectiveUsageFieldDefinitionAdded, @@ -430,7 +444,9 @@ function buildDirectiveUsageEnumValueAddedMessage( export function directiveUsageEnumValueAddedFromMeta(args: DirectiveUsageEnumValueAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to enum value '${args.meta.enumName}.${args.meta.enumValueName}'`, }, type: ChangeType.DirectiveUsageEnumValueAdded, @@ -468,7 +484,9 @@ function buildDirectiveUsageSchemaAddedMessage( export function directiveUsageSchemaAddedFromMeta(args: DirectiveUsageSchemaAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to schema '${args.meta.schemaTypeName}'`, }, type: ChangeType.DirectiveUsageSchemaAdded, @@ -506,7 +524,9 @@ function buildDirectiveUsageScalarAddedMessage( export function directiveUsageScalarAddedFromMeta(args: DirectiveUsageScalarAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to scalar '${args.meta.scalarName}'`, }, type: ChangeType.DirectiveUsageScalarAdded, @@ -544,7 +564,9 @@ function buildDirectiveUsageUnionMemberAddedMessage( export function directiveUsageUnionMemberAddedFromMeta(args: DirectiveUsageUnionMemberAddedChange) { return { criticality: { - level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), + level: args.meta.addedToNewType + ? CriticalityLevel.NonBreaking + : addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous), reason: `Directive '${args.meta.addedDirectiveName}' was added to union member '${args.meta.unionName}.${args.meta.addedUnionMemberTypeName}'`, }, type: ChangeType.DirectiveUsageUnionMemberAdded, @@ -579,6 +601,7 @@ export function directiveUsageAdded( kind: K, directive: ConstDirectiveNode, payload: KindToPayload[K]['input'], + addedToNewType: boolean, ): Change { if (isOfKind(kind, Kind.ARGUMENT, payload)) { return directiveUsageArgumentDefinitionAddedFromMeta({ @@ -588,6 +611,7 @@ export function directiveUsageAdded( argumentName: payload.argument.name, fieldName: payload.field.name, typeName: payload.type.name, + addedToNewType, }, }); } @@ -598,6 +622,7 @@ export function directiveUsageAdded( addedDirectiveName: directive.name.value, inputFieldName: payload.field.name, inputObjectName: payload.type.name, + addedToNewType, }, }); } @@ -610,6 +635,7 @@ export function directiveUsageAdded( addedInputFieldType: payload.name, inputObjectName: payload.name, isAddedInputFieldTypeNullable: kind === Kind.INPUT_VALUE_DEFINITION, + addedToNewType, }, }); } @@ -619,6 +645,7 @@ export function directiveUsageAdded( meta: { addedDirectiveName: directive.name.value, interfaceName: payload.name, + addedToNewType, }, }); } @@ -628,6 +655,7 @@ export function directiveUsageAdded( meta: { objectName: payload.name, addedDirectiveName: directive.name.value, + addedToNewType, }, }); } @@ -637,6 +665,7 @@ export function directiveUsageAdded( meta: { enumName: payload.name, addedDirectiveName: directive.name.value, + addedToNewType, }, }); } @@ -647,6 +676,7 @@ export function directiveUsageAdded( addedDirectiveName: directive.name.value, fieldName: payload.field.name, typeName: payload.parentType.name, + addedToNewType, }, }); } @@ -657,6 +687,7 @@ export function directiveUsageAdded( addedDirectiveName: directive.name.value, addedUnionMemberTypeName: payload.name, unionName: payload.name, + addedToNewType, }, }); } @@ -667,6 +698,7 @@ export function directiveUsageAdded( enumName: payload.type.name, enumValueName: payload.value.name, addedDirectiveName: directive.name.value, + addedToNewType, }, }); } @@ -676,6 +708,7 @@ export function directiveUsageAdded( meta: { addedDirectiveName: directive.name.value, schemaTypeName: payload.getQueryType()?.name || '', + addedToNewType, }, }); } @@ -685,6 +718,7 @@ export function directiveUsageAdded( meta: { scalarName: payload.name, addedDirectiveName: directive.name.value, + addedToNewType, }, }); } diff --git a/packages/core/src/diff/changes/directive.ts b/packages/core/src/diff/changes/directive.ts index 06392d1bdd..4c38187e84 100644 --- a/packages/core/src/diff/changes/directive.ts +++ b/packages/core/src/diff/changes/directive.ts @@ -70,6 +70,9 @@ export function directiveAdded( type: ChangeType.DirectiveAdded, meta: { addedDirectiveName: directive.name, + addedDirectiveDescription: directive.description ?? null, + addedDirectiveLocations: directive.locations.map(l => safeString(l)), + addedDirectiveRepeatable: directive.isRepeatable, }, }); } @@ -95,14 +98,14 @@ export function directiveDescriptionChangedFromMeta(args: DirectiveDescriptionCh } export function directiveDescriptionChanged( - oldDirective: GraphQLDirective, + oldDirective: GraphQLDirective | null, newDirective: GraphQLDirective, ): Change { return directiveDescriptionChangedFromMeta({ type: ChangeType.DirectiveDescriptionChanged, meta: { - directiveName: oldDirective.name, - oldDirectiveDescription: oldDirective.description ?? null, + directiveName: newDirective.name, + oldDirectiveDescription: oldDirective?.description ?? null, newDirectiveDescription: newDirective.description ?? null, }, }); @@ -172,19 +175,25 @@ export function directiveLocationRemoved( } const directiveArgumentAddedBreakingReason = `A directive could be in use of a client application. Adding a non-nullable argument will break those clients.`; -const directiveArgumentNonBreakingReason = `A directive could be in use of a client application. Adding a non-nullable argument will break those clients.`; +const directiveArgumentNonBreakingReason = `A directive could be in use of a client application. Adding a nullable argument will not break those clients.`; +const directiveArgumentNewReason = `Refer to the directive usage for the breaking status. If the directive is new and therefore unused, then adding an argument does not risk breaking clients.`; export function directiveArgumentAddedFromMeta(args: DirectiveArgumentAddedChange) { return { - criticality: args.meta.addedDirectiveArgumentTypeIsNonNull + criticality: args.meta.addedToNewDirective ? { - level: CriticalityLevel.Breaking, - reason: directiveArgumentAddedBreakingReason, - } - : { level: CriticalityLevel.NonBreaking, - reason: directiveArgumentNonBreakingReason, - }, + reason: directiveArgumentNewReason, + } + : args.meta.addedDirectiveArgumentTypeIsNonNull + ? { + level: CriticalityLevel.Breaking, + reason: directiveArgumentAddedBreakingReason, + } + : { + level: CriticalityLevel.NonBreaking, + reason: directiveArgumentNonBreakingReason, + }, type: ChangeType.DirectiveArgumentAdded, message: `Argument '${args.meta.addedDirectiveArgumentName}' was added to directive '${args.meta.directiveName}'`, path: `@${args.meta.directiveName}`, @@ -195,13 +204,18 @@ export function directiveArgumentAddedFromMeta(args: DirectiveArgumentAddedChang export function directiveArgumentAdded( directive: GraphQLDirective, arg: GraphQLArgument, + addedToNewDirective: boolean, ): Change { return directiveArgumentAddedFromMeta({ type: ChangeType.DirectiveArgumentAdded, meta: { directiveName: directive.name, addedDirectiveArgumentName: arg.name, + addedDirectiveArgumentType: arg.type.toString(), + addedDirectiveDefaultValue: safeString(arg.defaultValue), addedDirectiveArgumentTypeIsNonNull: isNonNullType(arg.type), + addedDirectiveArgumentDescription: arg.description ?? null, + addedToNewDirective, }, }); } @@ -262,15 +276,15 @@ export function directiveArgumentDescriptionChangedFromMeta( export function directiveArgumentDescriptionChanged( directive: GraphQLDirective, - oldArg: GraphQLArgument, + oldArg: GraphQLArgument | null, newArg: GraphQLArgument, ): Change { return directiveArgumentDescriptionChangedFromMeta({ type: ChangeType.DirectiveArgumentDescriptionChanged, meta: { directiveName: directive.name, - directiveArgumentName: oldArg.name, - oldDirectiveArgumentDescription: oldArg.description ?? null, + directiveArgumentName: newArg.name, + oldDirectiveArgumentDescription: oldArg?.description ?? null, newDirectiveArgumentDescription: newArg.description ?? null, }, }); @@ -304,14 +318,14 @@ export function directiveArgumentDefaultValueChangedFromMeta( export function directiveArgumentDefaultValueChanged( directive: GraphQLDirective, - oldArg: GraphQLArgument, + oldArg: GraphQLArgument | null, newArg: GraphQLArgument, ): Change { const meta: DirectiveArgumentDefaultValueChangedChange['meta'] = { directiveName: directive.name, - directiveArgumentName: oldArg.name, + directiveArgumentName: newArg.name, }; - if (oldArg.defaultValue !== undefined) { + if (oldArg?.defaultValue !== undefined) { meta.oldDirectiveArgumentDefaultValue = safeString(oldArg.defaultValue); } if (newArg.defaultValue !== undefined) { @@ -359,8 +373,8 @@ export function directiveArgumentTypeChanged( type: ChangeType.DirectiveArgumentTypeChanged, meta: { directiveName: directive.name, - directiveArgumentName: oldArg.name, - oldDirectiveArgumentType: oldArg.type.toString(), + directiveArgumentName: newArg.name, + oldDirectiveArgumentType: oldArg?.type.toString() ?? '', newDirectiveArgumentType: newArg.type.toString(), isSafeDirectiveArgumentTypeChange: safeChangeForInputValue(oldArg.type, newArg.type), }, diff --git a/packages/core/src/diff/changes/enum.ts b/packages/core/src/diff/changes/enum.ts index cf6c74dd39..a34e6c6232 100644 --- a/packages/core/src/diff/changes/enum.ts +++ b/packages/core/src/diff/changes/enum.ts @@ -54,11 +54,13 @@ function buildEnumValueAddedMessage(args: EnumValueAddedChange) { const enumValueAddedCriticalityDangerousReason = `Adding an enum value may break existing clients that were not programming defensively against an added case when querying an enum.`; export function enumValueAddedFromMeta(args: EnumValueAddedChange) { + /** Dangerous is there was a previous enum value */ + const isSafe = args.meta.addedToNewType; return { type: ChangeType.EnumValueAdded, criticality: { - level: CriticalityLevel.Dangerous, - reason: enumValueAddedCriticalityDangerousReason, + level: isSafe ? CriticalityLevel.NonBreaking : CriticalityLevel.Dangerous, + reason: isSafe ? undefined : enumValueAddedCriticalityDangerousReason, }, message: buildEnumValueAddedMessage(args), meta: args.meta, @@ -67,14 +69,17 @@ export function enumValueAddedFromMeta(args: EnumValueAddedChange) { } export function enumValueAdded( - newEnum: GraphQLEnumType, + type: GraphQLEnumType, value: GraphQLEnumValue, + addedToNewType: boolean, ): Change { return enumValueAddedFromMeta({ type: ChangeType.EnumValueAdded, meta: { - enumName: newEnum.name, + enumName: type.name, addedEnumValueName: value.name, + addedToNewType, + addedDirectiveDescription: value.description ?? null, }, }); } @@ -105,15 +110,15 @@ export function enumValueDescriptionChangedFromMeta( export function enumValueDescriptionChanged( newEnum: GraphQLEnumType, - oldValue: GraphQLEnumValue, + oldValue: GraphQLEnumValue | null, newValue: GraphQLEnumValue, ): Change { return enumValueDescriptionChangedFromMeta({ type: ChangeType.EnumValueDescriptionChanged, meta: { enumName: newEnum.name, - enumValueName: oldValue.name, - oldEnumValueDescription: oldValue.description ?? null, + enumValueName: newValue.name, + oldEnumValueDescription: oldValue?.description ?? null, newEnumValueDescription: newValue.description ?? null, }, }); @@ -177,14 +182,14 @@ export function enumValueDeprecationReasonAddedFromMeta( export function enumValueDeprecationReasonAdded( newEnum: GraphQLEnumType, - oldValue: GraphQLEnumValue, + _oldValue: GraphQLEnumValue | null, newValue: GraphQLEnumValue, ): Change { return enumValueDeprecationReasonAddedFromMeta({ type: ChangeType.EnumValueDeprecationReasonAdded, meta: { enumName: newEnum.name, - enumValueName: oldValue.name, + enumValueName: newValue.name, addedValueDeprecationReason: newValue.deprecationReason ?? '', }, }); diff --git a/packages/core/src/diff/changes/field.ts b/packages/core/src/diff/changes/field.ts index 8e3fcbeaaa..781b6685f6 100644 --- a/packages/core/src/diff/changes/field.ts +++ b/packages/core/src/diff/changes/field.ts @@ -5,6 +5,7 @@ import { GraphQLObjectType, isInterfaceType, isNonNullType, + print, } from 'graphql'; import { safeChangeForField } from '../../utils/graphql.js'; import { @@ -90,6 +91,7 @@ export function fieldAdded( typeName: type.name, addedFieldName: field.name, typeType: entity, + addedFieldReturnType: field.astNode?.type ? print(field.astNode?.type) : '', }, }); } @@ -210,6 +212,7 @@ export function fieldDeprecationAdded( meta: { typeName: type.name, fieldName: field.name, + deprecationReason: field.deprecationReason ?? '', }, }); } @@ -218,6 +221,7 @@ export function fieldDeprecationRemovedFromMeta(args: FieldDeprecationRemovedCha return { type: ChangeType.FieldDeprecationRemoved, criticality: { + // @todo: Add a reason for why is this dangerous... Why is it?? level: CriticalityLevel.Dangerous, }, message: `Field '${args.meta.typeName}.${args.meta.fieldName}' is no longer deprecated`, @@ -373,9 +377,11 @@ export function fieldArgumentAddedFromMeta(args: FieldArgumentAddedChange) { return { type: ChangeType.FieldArgumentAdded, criticality: { - level: args.meta.isAddedFieldArgumentBreaking - ? CriticalityLevel.Breaking - : CriticalityLevel.Dangerous, + level: args.meta.addedToNewField + ? CriticalityLevel.NonBreaking + : args.meta.isAddedFieldArgumentBreaking + ? CriticalityLevel.Breaking + : CriticalityLevel.Dangerous, }, message: buildFieldArgumentAddedMessage(args.meta), meta: args.meta, @@ -387,6 +393,7 @@ export function fieldArgumentAdded( type: GraphQLObjectType | GraphQLInterfaceType, field: GraphQLField, arg: GraphQLArgument, + addedToNewField: boolean, ): Change { const isBreaking = isNonNullType(arg.type) && typeof arg.defaultValue === 'undefined'; @@ -398,6 +405,7 @@ export function fieldArgumentAdded( addedArgumentName: arg.name, addedArgumentType: arg.type.toString(), hasDefaultValue: arg.defaultValue != null, + addedToNewField, isAddedFieldArgumentBreaking: isBreaking, }, }); diff --git a/packages/core/src/diff/changes/input.ts b/packages/core/src/diff/changes/input.ts index a4c0395800..bbb793581c 100644 --- a/packages/core/src/diff/changes/input.ts +++ b/packages/core/src/diff/changes/input.ts @@ -50,21 +50,25 @@ export function inputFieldRemoved( } export function buildInputFieldAddedMessage(args: InputFieldAddedChange['meta']) { - return `Input field '${args.addedInputFieldName}' of type '${args.addedInputFieldType}' was added to input object type '${args.inputName}'`; + return `Input field '${args.addedInputFieldName}' of type '${args.addedInputFieldType}'${args.addedFieldDefault ? ` with default value '${args.addedFieldDefault}'` : ''} was added to input object type '${args.inputName}'`; } export function inputFieldAddedFromMeta(args: InputFieldAddedChange) { return { type: ChangeType.InputFieldAdded, - criticality: args.meta.isAddedInputFieldTypeNullable + criticality: args.meta.addedToNewType ? { - level: CriticalityLevel.Dangerous, + level: CriticalityLevel.NonBreaking, } - : { - level: CriticalityLevel.Breaking, - reason: - 'Adding a required input field to an existing input object type is a breaking change because it will cause existing uses of this input object type to error.', - }, + : args.meta.isAddedInputFieldTypeNullable || args.meta.addedFieldDefault !== undefined + ? { + level: CriticalityLevel.Dangerous, + } + : { + level: CriticalityLevel.Breaking, + reason: + 'Adding a required input field to an existing input object type is a breaking change because it will cause existing uses of this input object type to error.', + }, message: buildInputFieldAddedMessage(args.meta), meta: args.meta, path: [args.meta.inputName, args.meta.addedInputFieldName].join('.'), @@ -74,6 +78,7 @@ export function inputFieldAddedFromMeta(args: InputFieldAddedChange) { export function inputFieldAdded( input: GraphQLInputObjectType, field: GraphQLInputField, + addedToNewType: boolean, ): Change { return inputFieldAddedFromMeta({ type: ChangeType.InputFieldAdded, @@ -82,6 +87,10 @@ export function inputFieldAdded( addedInputFieldName: field.name, isAddedInputFieldTypeNullable: !isNonNullType(field.type), addedInputFieldType: field.type.toString(), + ...(field.defaultValue === undefined + ? {} + : { addedFieldDefault: safeString(field.defaultValue) }), + addedToNewType, }, }); } @@ -189,13 +198,14 @@ function buildInputFieldDefaultValueChangedMessage( } export function inputFieldDefaultValueChangedFromMeta(args: InputFieldDefaultValueChangedChange) { + const criticality = { + level: CriticalityLevel.Dangerous, + reason: + 'Changing the default value for an argument may change the runtime behavior of a field if it was never provided.', + }; return { type: ChangeType.InputFieldDefaultValueChanged, - criticality: { - level: CriticalityLevel.Dangerous, - reason: - 'Changing the default value for an argument may change the runtime behavior of a field if it was never provided.', - }, + criticality, message: buildInputFieldDefaultValueChangedMessage(args.meta), meta: args.meta, path: [args.meta.inputName, args.meta.inputFieldName].join('.'), @@ -209,7 +219,7 @@ export function inputFieldDefaultValueChanged( ): Change { const meta: InputFieldDefaultValueChangedChange['meta'] = { inputName: input.name, - inputFieldName: oldField.name, + inputFieldName: newField.name, }; if (oldField.defaultValue !== undefined) { @@ -256,7 +266,7 @@ export function inputFieldTypeChanged( type: ChangeType.InputFieldTypeChanged, meta: { inputName: input.name, - inputFieldName: oldField.name, + inputFieldName: newField.name, oldInputFieldType: oldField.type.toString(), newInputFieldType: newField.type.toString(), isInputFieldTypeChangeSafe: safeChangeForInputValue(oldField.type, newField.type), diff --git a/packages/core/src/diff/changes/object.ts b/packages/core/src/diff/changes/object.ts index babbc79da2..36ab895464 100644 --- a/packages/core/src/diff/changes/object.ts +++ b/packages/core/src/diff/changes/object.ts @@ -15,7 +15,7 @@ export function objectTypeInterfaceAddedFromMeta(args: ObjectTypeInterfaceAddedC return { type: ChangeType.ObjectTypeInterfaceAdded, criticality: { - level: CriticalityLevel.Dangerous, + level: args.meta.addedToNewType ? CriticalityLevel.NonBreaking : CriticalityLevel.Dangerous, reason: 'Adding an interface to an object type may break existing clients that were not programming defensively against a new possible type.', }, @@ -27,13 +27,15 @@ export function objectTypeInterfaceAddedFromMeta(args: ObjectTypeInterfaceAddedC export function objectTypeInterfaceAdded( iface: GraphQLInterfaceType, - type: GraphQLObjectType, + type: GraphQLObjectType | GraphQLInterfaceType, + addedToNewType: boolean, ): Change { return objectTypeInterfaceAddedFromMeta({ type: ChangeType.ObjectTypeInterfaceAdded, meta: { objectTypeName: type.name, addedInterfaceName: iface.name, + addedToNewType, }, }); } @@ -58,7 +60,7 @@ export function objectTypeInterfaceRemovedFromMeta(args: ObjectTypeInterfaceRemo export function objectTypeInterfaceRemoved( iface: GraphQLInterfaceType, - type: GraphQLObjectType, + type: GraphQLObjectType | GraphQLInterfaceType, ): Change { return objectTypeInterfaceRemovedFromMeta({ type: ChangeType.ObjectTypeInterfaceRemoved, diff --git a/packages/core/src/diff/changes/type.ts b/packages/core/src/diff/changes/type.ts index 7149969da4..0d64b17ad6 100644 --- a/packages/core/src/diff/changes/type.ts +++ b/packages/core/src/diff/changes/type.ts @@ -1,4 +1,12 @@ -import { GraphQLNamedType } from 'graphql'; +import { + isEnumType, + isInputObjectType, + isInterfaceType, + isObjectType, + isUnionType, + Kind, + type GraphQLNamedType, +} from 'graphql'; import { getKind } from '../../utils/graphql.js'; import { Change, @@ -53,12 +61,44 @@ export function typeAddedFromMeta(args: TypeAddedChange) { } as const; } +function addedTypeMeta(type: GraphQLNamedType): TypeAddedChange['meta'] { + if (isEnumType(type)) { + return { + addedTypeKind: Kind.ENUM_TYPE_DEFINITION, + addedTypeName: type.name, + }; + } + if (isObjectType(type) || isInterfaceType(type)) { + return { + addedTypeKind: getKind(type) as any as + | Kind.INTERFACE_TYPE_DEFINITION + | Kind.OBJECT_TYPE_DEFINITION, + addedTypeName: type.name, + }; + } + if (isUnionType(type)) { + return { + addedTypeKind: Kind.UNION_TYPE_DEFINITION, + addedTypeName: type.name, + }; + } + if (isInputObjectType(type)) { + return { + addedTypeKind: Kind.INPUT_OBJECT_TYPE_DEFINITION, + addedTypeIsOneOf: type.isOneOf, + addedTypeName: type.name, + }; + } + return { + addedTypeKind: getKind(type) as any as Kind.SCALAR_TYPE_DEFINITION, + addedTypeName: type.name, + }; +} + export function typeAdded(type: GraphQLNamedType): Change { return typeAddedFromMeta({ type: ChangeType.TypeAdded, - meta: { - addedTypeName: type.name, - }, + meta: addedTypeMeta(type), }); } @@ -86,7 +126,7 @@ export function typeKindChanged( return typeKindChangedFromMeta({ type: ChangeType.TypeKindChanged, meta: { - typeName: oldType.name, + typeName: newType.name, newTypeKind: String(getKind(newType)), oldTypeKind: String(getKind(oldType)), }, diff --git a/packages/core/src/diff/changes/union.ts b/packages/core/src/diff/changes/union.ts index afede6b89b..9357d05042 100644 --- a/packages/core/src/diff/changes/union.ts +++ b/packages/core/src/diff/changes/union.ts @@ -45,7 +45,7 @@ function buildUnionMemberAddedMessage(args: UnionMemberAddedChange['meta']) { export function buildUnionMemberAddedMessageFromMeta(args: UnionMemberAddedChange) { return { criticality: { - level: CriticalityLevel.Dangerous, + level: args.meta.addedToNewType ? CriticalityLevel.NonBreaking : CriticalityLevel.Dangerous, reason: 'Adding a possible type to Unions may break existing clients that were not programming defensively against a new possible type.', }, @@ -59,12 +59,14 @@ export function buildUnionMemberAddedMessageFromMeta(args: UnionMemberAddedChang export function unionMemberAdded( union: GraphQLUnionType, type: GraphQLObjectType, + addedToNewType: boolean, ): Change { return buildUnionMemberAddedMessageFromMeta({ type: ChangeType.UnionMemberAdded, meta: { unionName: union.name, addedUnionMemberTypeName: type.name, + addedToNewType, }, }); } diff --git a/packages/core/src/diff/directive.ts b/packages/core/src/diff/directive.ts index 035325da99..d1b918ad21 100644 --- a/packages/core/src/diff/directive.ts +++ b/packages/core/src/diff/directive.ts @@ -13,17 +13,17 @@ import { import { AddChange } from './schema.js'; export function changesInDirective( - oldDirective: GraphQLDirective, + oldDirective: GraphQLDirective | null, newDirective: GraphQLDirective, addChange: AddChange, ) { - if (isNotEqual(oldDirective.description, newDirective.description)) { + if (isNotEqual(oldDirective?.description, newDirective.description)) { addChange(directiveDescriptionChanged(oldDirective, newDirective)); } const locations = { - added: diffArrays(newDirective.locations, oldDirective.locations), - removed: diffArrays(oldDirective.locations, newDirective.locations), + added: diffArrays(newDirective.locations, oldDirective?.locations ?? []), + removed: diffArrays(oldDirective?.locations ?? [], newDirective.locations), }; // locations added @@ -32,17 +32,17 @@ export function changesInDirective( // locations removed for (const location of locations.removed) - addChange(directiveLocationRemoved(oldDirective, location as any)); + addChange(directiveLocationRemoved(newDirective, location as any)); - compareLists(oldDirective.args, newDirective.args, { + compareLists(oldDirective?.args ?? [], newDirective.args, { onAdded(arg) { - addChange(directiveArgumentAdded(newDirective, arg)); + addChange(directiveArgumentAdded(newDirective, arg, oldDirective === null)); }, onRemoved(arg) { - addChange(directiveArgumentRemoved(oldDirective, arg)); + addChange(directiveArgumentRemoved(newDirective, arg)); }, onMutual(arg) { - changesInDirectiveArgument(oldDirective, arg.oldVersion, arg.newVersion, addChange); + changesInDirectiveArgument(newDirective, arg.oldVersion!, arg.newVersion, addChange); }, }); } @@ -53,11 +53,11 @@ function changesInDirectiveArgument( newArg: GraphQLArgument, addChange: AddChange, ) { - if (isNotEqual(oldArg.description, newArg.description)) { + if (isNotEqual(oldArg?.description, newArg.description)) { addChange(directiveArgumentDescriptionChanged(directive, oldArg, newArg)); } - if (isNotEqual(oldArg.defaultValue, newArg.defaultValue)) { + if (isNotEqual(oldArg?.defaultValue, newArg.defaultValue)) { addChange(directiveArgumentDefaultValueChanged(directive, oldArg, newArg)); } diff --git a/packages/core/src/diff/enum.ts b/packages/core/src/diff/enum.ts index 1be7b0dacf..7fef009f4e 100644 --- a/packages/core/src/diff/enum.ts +++ b/packages/core/src/diff/enum.ts @@ -1,4 +1,4 @@ -import { GraphQLEnumType, Kind } from 'graphql'; +import { GraphQLEnumType, GraphQLEnumValue, Kind } from 'graphql'; import { compareLists, isNotEqual, isVoid } from '../utils/compare.js'; import { directiveUsageAdded, directiveUsageRemoved } from './changes/directive-usage.js'; import { @@ -12,62 +12,81 @@ import { import { AddChange } from './schema.js'; export function changesInEnum( - oldEnum: GraphQLEnumType, + oldEnum: GraphQLEnumType | null, newEnum: GraphQLEnumType, addChange: AddChange, ) { - compareLists(oldEnum.getValues(), newEnum.getValues(), { + compareLists(oldEnum?.getValues() ?? [], newEnum.getValues(), { onAdded(value) { - addChange(enumValueAdded(newEnum, value)); + addChange(enumValueAdded(newEnum, value, oldEnum === null)); + changesInEnumValue({ newVersion: value, oldVersion: null }, newEnum, addChange); }, onRemoved(value) { - addChange(enumValueRemoved(oldEnum, value)); + addChange(enumValueRemoved(oldEnum!, value)); }, onMutual(value) { - const oldValue = value.oldVersion; - const newValue = value.newVersion; - - if (isNotEqual(oldValue.description, newValue.description)) { - addChange(enumValueDescriptionChanged(newEnum, oldValue, newValue)); - } - - if (isNotEqual(oldValue.deprecationReason, newValue.deprecationReason)) { - if (isVoid(oldValue.deprecationReason)) { - addChange(enumValueDeprecationReasonAdded(newEnum, oldValue, newValue)); - } else if (isVoid(newValue.deprecationReason)) { - addChange(enumValueDeprecationReasonRemoved(newEnum, oldValue, newValue)); - } else { - addChange(enumValueDeprecationReasonChanged(newEnum, oldValue, newValue)); - } - } - - compareLists(oldValue.astNode?.directives || [], newValue.astNode?.directives || [], { - onAdded(directive) { - addChange( - directiveUsageAdded(Kind.ENUM_VALUE_DEFINITION, directive, { - type: newEnum, - value: newValue, - }), - ); - }, - onRemoved(directive) { - addChange( - directiveUsageRemoved(Kind.ENUM_VALUE_DEFINITION, directive, { - type: oldEnum, - value: oldValue, - }), - ); - }, - }); + changesInEnumValue(value, newEnum, addChange); }, }); - compareLists(oldEnum.astNode?.directives || [], newEnum.astNode?.directives || [], { + compareLists(oldEnum?.astNode?.directives || [], newEnum.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.ENUM_TYPE_DEFINITION, directive, newEnum)); + addChange( + directiveUsageAdded(Kind.ENUM_TYPE_DEFINITION, directive, newEnum, oldEnum === null), + ); }, onRemoved(directive) { addChange(directiveUsageRemoved(Kind.ENUM_TYPE_DEFINITION, directive, newEnum)); }, }); } + +function changesInEnumValue( + value: { + newVersion: GraphQLEnumValue; + oldVersion: GraphQLEnumValue | null; + }, + newEnum: GraphQLEnumType, + addChange: AddChange, +) { + const oldValue = value.oldVersion; + const newValue = value.newVersion; + + if (isNotEqual(oldValue?.description, newValue.description)) { + addChange(enumValueDescriptionChanged(newEnum, oldValue, newValue)); + } + + if (isNotEqual(oldValue?.deprecationReason, newValue.deprecationReason)) { + if (isVoid(oldValue?.deprecationReason)) { + addChange(enumValueDeprecationReasonAdded(newEnum, oldValue, newValue)); + } else if (isVoid(newValue.deprecationReason)) { + addChange(enumValueDeprecationReasonRemoved(newEnum, oldValue, newValue)); + } else { + addChange(enumValueDeprecationReasonChanged(newEnum, oldValue, newValue)); + } + } + + compareLists(oldValue?.astNode?.directives || [], newValue.astNode?.directives || [], { + onAdded(directive) { + addChange( + directiveUsageAdded( + Kind.ENUM_VALUE_DEFINITION, + directive, + { + type: newEnum, + value: newValue, + }, + oldValue === null, + ), + ); + }, + onRemoved(directive) { + addChange( + directiveUsageRemoved(Kind.ENUM_VALUE_DEFINITION, directive, { + type: newEnum, + value: oldValue!, + }), + ); + }, + }); +} diff --git a/packages/core/src/diff/field.ts b/packages/core/src/diff/field.ts index ff9bf07c55..c6bd2642de 100644 --- a/packages/core/src/diff/field.ts +++ b/packages/core/src/diff/field.ts @@ -20,30 +20,30 @@ import { AddChange } from './schema.js'; export function changesInField( type: GraphQLObjectType | GraphQLInterfaceType, - oldField: GraphQLField, + oldField: GraphQLField | null, newField: GraphQLField, addChange: AddChange, ) { - if (isNotEqual(oldField.description, newField.description)) { - if (isVoid(oldField.description)) { + if (isNotEqual(oldField?.description, newField.description)) { + if (isVoid(oldField?.description)) { addChange(fieldDescriptionAdded(type, newField)); } else if (isVoid(newField.description)) { - addChange(fieldDescriptionRemoved(type, oldField)); + addChange(fieldDescriptionRemoved(type, oldField!)); } else { - addChange(fieldDescriptionChanged(type, oldField, newField)); + addChange(fieldDescriptionChanged(type, oldField!, newField)); } } - if (isNotEqual(isDeprecated(oldField), isDeprecated(newField))) { + if (!isVoid(oldField) && isNotEqual(isDeprecated(oldField), isDeprecated(newField))) { if (isDeprecated(newField)) { addChange(fieldDeprecationAdded(type, newField)); } else { addChange(fieldDeprecationRemoved(type, oldField)); } - } - - if (isNotEqual(oldField.deprecationReason, newField.deprecationReason)) { - if (isVoid(oldField.deprecationReason)) { + } else if (isVoid(oldField) && isDeprecated(newField)) { + addChange(fieldDeprecationAdded(type, newField)); + } else if (isNotEqual(oldField?.deprecationReason, newField.deprecationReason)) { + if (isVoid(oldField?.deprecationReason)) { addChange(fieldDeprecationReasonAdded(type, newField)); } else if (isVoid(newField.deprecationReason)) { addChange(fieldDeprecationReasonRemoved(type, oldField)); @@ -52,36 +52,41 @@ export function changesInField( } } - if (isNotEqual(oldField.type.toString(), newField.type.toString())) { - addChange(fieldTypeChanged(type, oldField, newField)); + if (!isVoid(oldField) && isNotEqual(oldField!.type.toString(), newField.type.toString())) { + addChange(fieldTypeChanged(type, oldField!, newField)); } - compareLists(oldField.args, newField.args, { + compareLists(oldField?.args ?? [], newField.args, { onAdded(arg) { - addChange(fieldArgumentAdded(type, newField, arg)); + addChange(fieldArgumentAdded(type, newField, arg, oldField === null)); }, onRemoved(arg) { - addChange(fieldArgumentRemoved(type, oldField, arg)); + addChange(fieldArgumentRemoved(type, newField, arg)); }, onMutual(arg) { - changesInArgument(type, oldField, arg.oldVersion, arg.newVersion, addChange); + changesInArgument(type, newField, arg.oldVersion, arg.newVersion, addChange); }, }); - compareLists(oldField.astNode?.directives || [], newField.astNode?.directives || [], { + compareLists(oldField?.astNode?.directives || [], newField.astNode?.directives || [], { onAdded(directive) { addChange( - directiveUsageAdded(Kind.FIELD_DEFINITION, directive, { - parentType: type, - field: newField, - }), + directiveUsageAdded( + Kind.FIELD_DEFINITION, + directive, + { + parentType: type, + field: newField, + }, + oldField === null, + ), ); }, onRemoved(arg) { addChange( directiveUsageRemoved(Kind.FIELD_DEFINITION, arg, { parentType: type, - field: oldField, + field: oldField!, }), ); }, diff --git a/packages/core/src/diff/input.ts b/packages/core/src/diff/input.ts index 7b45de560d..30049ada87 100644 --- a/packages/core/src/diff/input.ts +++ b/packages/core/src/diff/input.ts @@ -13,80 +13,95 @@ import { import { AddChange } from './schema.js'; export function changesInInputObject( - oldInput: GraphQLInputObjectType, + oldInput: GraphQLInputObjectType | null, newInput: GraphQLInputObjectType, addChange: AddChange, ) { - const oldFields = oldInput.getFields(); + const oldFields = oldInput?.getFields() ?? {}; const newFields = newInput.getFields(); compareLists(Object.values(oldFields), Object.values(newFields), { onAdded(field) { - addChange(inputFieldAdded(newInput, field)); + addChange(inputFieldAdded(newInput, field, oldInput === null)); + changesInInputField(newInput, null, field, addChange); }, onRemoved(field) { - addChange(inputFieldRemoved(oldInput, field)); + addChange(inputFieldRemoved(oldInput!, field)); }, onMutual(field) { - changesInInputField(oldInput, field.oldVersion, field.newVersion, addChange); + changesInInputField(newInput, field.oldVersion, field.newVersion, addChange); }, }); - compareLists(oldInput.astNode?.directives || [], newInput.astNode?.directives || [], { + compareLists(oldInput?.astNode?.directives || [], newInput.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.INPUT_OBJECT_TYPE_DEFINITION, directive, newInput)); + addChange( + directiveUsageAdded( + Kind.INPUT_OBJECT_TYPE_DEFINITION, + directive, + newInput, + oldInput === null, + ), + ); }, onRemoved(directive) { - addChange(directiveUsageRemoved(Kind.INPUT_OBJECT_TYPE_DEFINITION, directive, oldInput)); + addChange(directiveUsageRemoved(Kind.INPUT_OBJECT_TYPE_DEFINITION, directive, oldInput!)); }, }); } function changesInInputField( input: GraphQLInputObjectType, - oldField: GraphQLInputField, + oldField: GraphQLInputField | null, newField: GraphQLInputField, addChange: AddChange, ) { - if (isNotEqual(oldField.description, newField.description)) { - if (isVoid(oldField.description)) { + if (isNotEqual(oldField?.description, newField.description)) { + if (isVoid(oldField?.description)) { addChange(inputFieldDescriptionAdded(input, newField)); } else if (isVoid(newField.description)) { - addChange(inputFieldDescriptionRemoved(input, oldField)); + addChange(inputFieldDescriptionRemoved(input, oldField!)); } else { - addChange(inputFieldDescriptionChanged(input, oldField, newField)); + addChange(inputFieldDescriptionChanged(input, oldField!, newField)); } } - if (isNotEqual(oldField.defaultValue, newField.defaultValue)) { - if (Array.isArray(oldField.defaultValue) && Array.isArray(newField.defaultValue)) { - if (diffArrays(oldField.defaultValue, newField.defaultValue).length > 0) { + if (!isVoid(oldField)) { + if (isNotEqual(oldField?.defaultValue, newField.defaultValue)) { + if (Array.isArray(oldField?.defaultValue) && Array.isArray(newField.defaultValue)) { + if (diffArrays(oldField.defaultValue, newField.defaultValue).length > 0) { + addChange(inputFieldDefaultValueChanged(input, oldField, newField)); + } + } else if (JSON.stringify(oldField?.defaultValue) !== JSON.stringify(newField.defaultValue)) { addChange(inputFieldDefaultValueChanged(input, oldField, newField)); } - } else if (JSON.stringify(oldField.defaultValue) !== JSON.stringify(newField.defaultValue)) { - addChange(inputFieldDefaultValueChanged(input, oldField, newField)); } - } - if (isNotEqual(oldField.type.toString(), newField.type.toString())) { - addChange(inputFieldTypeChanged(input, oldField, newField)); + if (!isVoid(oldField) && isNotEqual(oldField.type.toString(), newField.type.toString())) { + addChange(inputFieldTypeChanged(input, oldField, newField)); + } } - if (oldField.astNode?.directives && newField.astNode?.directives) { - compareLists(oldField.astNode.directives || [], newField.astNode.directives || [], { + if (newField.astNode?.directives) { + compareLists(oldField?.astNode?.directives || [], newField.astNode.directives || [], { onAdded(directive) { addChange( - directiveUsageAdded(Kind.INPUT_VALUE_DEFINITION, directive, { - type: input, - field: newField, - }), + directiveUsageAdded( + Kind.INPUT_VALUE_DEFINITION, + directive, + { + type: input, + field: newField, + }, + oldField === null, + ), ); }, onRemoved(directive) { addChange( directiveUsageRemoved(Kind.INPUT_VALUE_DEFINITION, directive, { type: input, - field: oldField, + field: newField, }), ); }, diff --git a/packages/core/src/diff/interface.ts b/packages/core/src/diff/interface.ts index ac34f74b8a..0126222131 100644 --- a/packages/core/src/diff/interface.ts +++ b/packages/core/src/diff/interface.ts @@ -2,31 +2,55 @@ import { GraphQLInterfaceType, Kind } from 'graphql'; import { compareLists } from '../utils/compare.js'; import { directiveUsageAdded, directiveUsageRemoved } from './changes/directive-usage.js'; import { fieldAdded, fieldRemoved } from './changes/field.js'; +import { objectTypeInterfaceAdded, objectTypeInterfaceRemoved } from './changes/object.js'; import { changesInField } from './field.js'; import { AddChange } from './schema.js'; export function changesInInterface( - oldInterface: GraphQLInterfaceType, + oldInterface: GraphQLInterfaceType | null, newInterface: GraphQLInterfaceType, addChange: AddChange, ) { - compareLists(Object.values(oldInterface.getFields()), Object.values(newInterface.getFields()), { + const oldInterfaces = oldInterface?.getInterfaces() ?? []; + const newInterfaces = newInterface.getInterfaces(); + + compareLists(oldInterfaces, newInterfaces, { + onAdded(i) { + addChange(objectTypeInterfaceAdded(i, newInterface, oldInterface === null)); + }, + onRemoved(i) { + addChange(objectTypeInterfaceRemoved(i, oldInterface!)); + }, + }); + + const oldFields = oldInterface?.getFields() ?? {}; + const newFields = newInterface.getFields(); + + compareLists(Object.values(oldFields), Object.values(newFields), { onAdded(field) { addChange(fieldAdded(newInterface, field)); + changesInField(newInterface, null, field, addChange); }, onRemoved(field) { - addChange(fieldRemoved(oldInterface, field)); + addChange(fieldRemoved(oldInterface!, field)); }, onMutual(field) { - changesInField(oldInterface, field.oldVersion, field.newVersion, addChange); + changesInField(newInterface, field.oldVersion, field.newVersion, addChange); }, }); - compareLists(oldInterface.astNode?.directives || [], newInterface.astNode?.directives || [], { + compareLists(oldInterface?.astNode?.directives || [], newInterface.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.INTERFACE_TYPE_DEFINITION, directive, newInterface)); + addChange( + directiveUsageAdded( + Kind.INTERFACE_TYPE_DEFINITION, + directive, + newInterface, + oldInterface === null, + ), + ); }, onRemoved(directive) { - addChange(directiveUsageRemoved(Kind.INTERFACE_TYPE_DEFINITION, directive, oldInterface)); + addChange(directiveUsageRemoved(Kind.INTERFACE_TYPE_DEFINITION, directive, oldInterface!)); }, }); } diff --git a/packages/core/src/diff/object.ts b/packages/core/src/diff/object.ts index 12817e1f0f..c716fb98a1 100644 --- a/packages/core/src/diff/object.ts +++ b/packages/core/src/diff/object.ts @@ -7,43 +7,44 @@ import { changesInField } from './field.js'; import { AddChange } from './schema.js'; export function changesInObject( - oldType: GraphQLObjectType, + oldType: GraphQLObjectType | null, newType: GraphQLObjectType, addChange: AddChange, ) { - const oldInterfaces = oldType.getInterfaces(); + const oldInterfaces = oldType?.getInterfaces() ?? []; const newInterfaces = newType.getInterfaces(); - const oldFields = oldType.getFields(); + const oldFields = oldType?.getFields() ?? {}; const newFields = newType.getFields(); compareLists(oldInterfaces, newInterfaces, { onAdded(i) { - addChange(objectTypeInterfaceAdded(i, newType)); + addChange(objectTypeInterfaceAdded(i, newType, oldType === null)); }, onRemoved(i) { - addChange(objectTypeInterfaceRemoved(i, oldType)); + addChange(objectTypeInterfaceRemoved(i, oldType!)); }, }); compareLists(Object.values(oldFields), Object.values(newFields), { onAdded(f) { addChange(fieldAdded(newType, f)); + changesInField(newType, null, f, addChange); }, onRemoved(f) { - addChange(fieldRemoved(oldType, f)); + addChange(fieldRemoved(oldType!, f)); }, onMutual(f) { - changesInField(oldType, f.oldVersion, f.newVersion, addChange); + changesInField(newType, f.oldVersion, f.newVersion, addChange); }, }); - compareLists(oldType.astNode?.directives || [], newType.astNode?.directives || [], { + compareLists(oldType?.astNode?.directives || [], newType.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.OBJECT, directive, newType)); + addChange(directiveUsageAdded(Kind.OBJECT, directive, newType, oldType === null)); }, onRemoved(directive) { - addChange(directiveUsageRemoved(Kind.OBJECT, directive, oldType)); + addChange(directiveUsageRemoved(Kind.OBJECT, directive, oldType!)); }, }); } diff --git a/packages/core/src/diff/rules/ignore-nested-additions.ts b/packages/core/src/diff/rules/ignore-nested-additions.ts new file mode 100644 index 0000000000..af61f0054b --- /dev/null +++ b/packages/core/src/diff/rules/ignore-nested-additions.ts @@ -0,0 +1,60 @@ +import { ChangeType } from '../changes/change.js'; +import { Rule } from './types.js'; + +const additionChangeTypes = new Set([ + ChangeType.DirectiveAdded, + ChangeType.DirectiveArgumentAdded, + ChangeType.DirectiveLocationAdded, + ChangeType.DirectiveUsageArgumentDefinitionAdded, + ChangeType.DirectiveUsageEnumAdded, + ChangeType.DirectiveUsageEnumValueAdded, + ChangeType.DirectiveUsageFieldAdded, + ChangeType.DirectiveUsageFieldDefinitionAdded, + ChangeType.DirectiveUsageInputFieldDefinitionAdded, + ChangeType.DirectiveUsageInputObjectAdded, + ChangeType.DirectiveUsageInterfaceAdded, + ChangeType.DirectiveUsageObjectAdded, + ChangeType.DirectiveUsageScalarAdded, + ChangeType.DirectiveUsageSchemaAdded, + ChangeType.DirectiveUsageUnionMemberAdded, + ChangeType.EnumValueAdded, + ChangeType.EnumValueDeprecationReasonAdded, + ChangeType.FieldAdded, + ChangeType.FieldArgumentAdded, + ChangeType.FieldDeprecationAdded, + ChangeType.FieldDeprecationReasonAdded, + ChangeType.FieldDescriptionAdded, + ChangeType.InputFieldAdded, + ChangeType.InputFieldDescriptionAdded, + ChangeType.ObjectTypeInterfaceAdded, + ChangeType.TypeAdded, + ChangeType.TypeDescriptionAdded, + ChangeType.UnionMemberAdded, +]); + +const parentPath = (path: string) => { + const lastDividerIndex = path.lastIndexOf('.'); + return lastDividerIndex === -1 ? path : path.substring(0, lastDividerIndex); +}; + +export const ignoreNestedAdditions: Rule = ({ changes }) => { + // Track which paths contained changes that represent additions to the schema + const additionPaths: string[] = []; + + const filteredChanges = changes.filter(({ path, type }) => { + if (path) { + const parent = parentPath(path); + const matches = additionPaths.filter(matchedPath => matchedPath.includes(parent)); + const hasAddedParent = matches.length > 0; + + if (additionChangeTypes.has(type)) { + additionPaths.push(path); + } + + return !hasAddedParent; + } + return true; + }); + + return filteredChanges; +}; diff --git a/packages/core/src/diff/rules/index.ts b/packages/core/src/diff/rules/index.ts index fb9f10a602..70db723148 100644 --- a/packages/core/src/diff/rules/index.ts +++ b/packages/core/src/diff/rules/index.ts @@ -4,3 +4,4 @@ export * from './ignore-description-changes.js'; export * from './safe-unreachable.js'; export * from './suppress-removal-of-deprecated-field.js'; export * from './ignore-usage-directives.js'; +export * from './ignore-nested-additions.js'; diff --git a/packages/core/src/diff/scalar.ts b/packages/core/src/diff/scalar.ts index 020752b322..fd3ba88586 100644 --- a/packages/core/src/diff/scalar.ts +++ b/packages/core/src/diff/scalar.ts @@ -4,16 +4,18 @@ import { directiveUsageAdded, directiveUsageRemoved } from './changes/directive- import { AddChange } from './schema.js'; export function changesInScalar( - oldScalar: GraphQLScalarType, + oldScalar: GraphQLScalarType | null, newScalar: GraphQLScalarType, addChange: AddChange, ) { - compareLists(oldScalar.astNode?.directives || [], newScalar.astNode?.directives || [], { + compareLists(oldScalar?.astNode?.directives || [], newScalar.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.SCALAR_TYPE_DEFINITION, directive, newScalar)); + addChange( + directiveUsageAdded(Kind.SCALAR_TYPE_DEFINITION, directive, newScalar, oldScalar === null), + ); }, onRemoved(directive) { - addChange(directiveUsageRemoved(Kind.SCALAR_TYPE_DEFINITION, directive, oldScalar)); + addChange(directiveUsageRemoved(Kind.SCALAR_TYPE_DEFINITION, directive, oldScalar!)); }, }); } diff --git a/packages/core/src/diff/schema.ts b/packages/core/src/diff/schema.ts index 0badd64085..92b7d37b70 100644 --- a/packages/core/src/diff/schema.ts +++ b/packages/core/src/diff/schema.ts @@ -53,6 +53,7 @@ export function diffSchema(oldSchema: GraphQLSchema, newSchema: GraphQLSchema): { onAdded(type) { addChange(typeAdded(type)); + changesInType(null, type, addChange); }, onRemoved(type) { addChange(typeRemoved(type)); @@ -66,6 +67,7 @@ export function diffSchema(oldSchema: GraphQLSchema, newSchema: GraphQLSchema): compareLists(oldSchema.getDirectives(), newSchema.getDirectives(), { onAdded(directive) { addChange(directiveAdded(directive)); + changesInDirective(null, directive, addChange); }, onRemoved(directive) { addChange(directiveRemoved(directive)); @@ -77,7 +79,7 @@ export function diffSchema(oldSchema: GraphQLSchema, newSchema: GraphQLSchema): compareLists(oldSchema.astNode?.directives || [], newSchema.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.SCHEMA_DEFINITION, directive, newSchema)); + addChange(directiveUsageAdded(Kind.SCHEMA_DEFINITION, directive, newSchema, false)); }, onRemoved(directive) { addChange(directiveUsageRemoved(Kind.SCHEMA_DEFINITION, directive, oldSchema)); @@ -123,30 +125,35 @@ function changesInSchema(oldSchema: GraphQLSchema, newSchema: GraphQLSchema, add } } -function changesInType(oldType: GraphQLNamedType, newType: GraphQLNamedType, addChange: AddChange) { - if (isEnumType(oldType) && isEnumType(newType)) { +function changesInType( + oldType: GraphQLNamedType | null, + newType: GraphQLNamedType, + addChange: AddChange, +) { + if ((isVoid(oldType) || isEnumType(oldType)) && isEnumType(newType)) { changesInEnum(oldType, newType, addChange); - } else if (isUnionType(oldType) && isUnionType(newType)) { + } else if ((isVoid(oldType) || isUnionType(oldType)) && isUnionType(newType)) { changesInUnion(oldType, newType, addChange); - } else if (isInputObjectType(oldType) && isInputObjectType(newType)) { + } else if ((isVoid(oldType) || isInputObjectType(oldType)) && isInputObjectType(newType)) { changesInInputObject(oldType, newType, addChange); - } else if (isObjectType(oldType) && isObjectType(newType)) { + } else if ((isVoid(oldType) || isObjectType(oldType)) && isObjectType(newType)) { changesInObject(oldType, newType, addChange); - } else if (isInterfaceType(oldType) && isInterfaceType(newType)) { + } else if ((isVoid(oldType) || isInterfaceType(oldType)) && isInterfaceType(newType)) { changesInInterface(oldType, newType, addChange); - } else if (isScalarType(oldType) && isScalarType(newType)) { + } else if ((isVoid(oldType) || isScalarType(oldType)) && isScalarType(newType)) { changesInScalar(oldType, newType, addChange); - } else { + } else if (!isVoid(oldType)) { + // no need to call if oldType is void since the type will be captured by the TypeAdded change. addChange(typeKindChanged(oldType, newType)); } - if (isNotEqual(oldType.description, newType.description)) { - if (isVoid(oldType.description)) { + if (isNotEqual(oldType?.description, newType.description)) { + if (isVoid(oldType?.description)) { addChange(typeDescriptionAdded(newType)); - } else if (isVoid(newType.description)) { + } else if (oldType && isVoid(newType.description)) { addChange(typeDescriptionRemoved(oldType)); } else { - addChange(typeDescriptionChanged(oldType, newType)); + addChange(typeDescriptionChanged(oldType!, newType)); } } } diff --git a/packages/core/src/diff/union.ts b/packages/core/src/diff/union.ts index 030539b675..6c0ed2e6f2 100644 --- a/packages/core/src/diff/union.ts +++ b/packages/core/src/diff/union.ts @@ -5,28 +5,30 @@ import { unionMemberAdded, unionMemberRemoved } from './changes/union.js'; import { AddChange } from './schema.js'; export function changesInUnion( - oldUnion: GraphQLUnionType, + oldUnion: GraphQLUnionType | null, newUnion: GraphQLUnionType, addChange: AddChange, ) { - const oldTypes = oldUnion.getTypes(); + const oldTypes = oldUnion?.getTypes() ?? []; const newTypes = newUnion.getTypes(); compareLists(oldTypes, newTypes, { onAdded(t) { - addChange(unionMemberAdded(newUnion, t)); + addChange(unionMemberAdded(newUnion, t, oldUnion === null)); }, onRemoved(t) { - addChange(unionMemberRemoved(oldUnion, t)); + addChange(unionMemberRemoved(oldUnion!, t)); }, }); - compareLists(oldUnion.astNode?.directives || [], newUnion.astNode?.directives || [], { + compareLists(oldUnion?.astNode?.directives || [], newUnion.astNode?.directives || [], { onAdded(directive) { - addChange(directiveUsageAdded(Kind.UNION_TYPE_DEFINITION, directive, newUnion)); + addChange( + directiveUsageAdded(Kind.UNION_TYPE_DEFINITION, directive, newUnion, oldUnion === null), + ); }, onRemoved(directive) { - addChange(directiveUsageRemoved(Kind.UNION_TYPE_DEFINITION, directive, oldUnion)); + addChange(directiveUsageRemoved(Kind.UNION_TYPE_DEFINITION, directive, oldUnion!)); }, }); } diff --git a/packages/core/src/utils/compare.ts b/packages/core/src/utils/compare.ts index 170a31df02..b8d37c43d4 100644 --- a/packages/core/src/utils/compare.ts +++ b/packages/core/src/utils/compare.ts @@ -45,7 +45,7 @@ export function isNotEqual(a: T, b: T): boolean { return !isEqual(a, b); } -export function isVoid(a: T): boolean { +export function isVoid(a: T): a is T & (null | undefined) { return typeof a === 'undefined' || a === null; } @@ -67,7 +67,7 @@ export function compareLists( callbacks?: { onAdded?(t: T): void; onRemoved?(t: T): void; - onMutual?(t: { newVersion: T; oldVersion: T }): void; + onMutual?(t: { newVersion: T; oldVersion: T | null }): void; }, ) { const oldMap = keyMap(oldList, ({ name }) => extractName(name)); diff --git a/packages/core/src/utils/graphql.ts b/packages/core/src/utils/graphql.ts index 8c94a5d1f6..4c803c4259 100644 --- a/packages/core/src/utils/graphql.ts +++ b/packages/core/src/utils/graphql.ts @@ -58,7 +58,7 @@ export function safeChangeForInputValue( newType: GraphQLInputType, ): boolean { if (!isWrappingType(oldType) && !isWrappingType(newType)) { - return oldType.toString() === newType.toString(); + return oldType?.toString() === newType.toString(); } if (isListType(oldType) && isListType(newType)) {