From 4c059f66717618d5ad75639c7445c49f0833dc3b Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Mon, 14 Oct 2024 08:24:58 +0200 Subject: [PATCH] Alternative approach for findSchemaChanges --- src/index.ts | 3 + .../__tests__/findBreakingChanges-test.ts | 2 +- .../__tests__/findSchemaChanges-test.ts | 180 ++++++++++++++++++ ...reakingChanges.ts => findSchemaChanges.ts} | 110 ++++++++++- src/utilities/index.ts | 10 +- 5 files changed, 295 insertions(+), 10 deletions(-) create mode 100644 src/utilities/__tests__/findSchemaChanges-test.ts rename src/utilities/{findBreakingChanges.ts => findSchemaChanges.ts} (82%) diff --git a/src/index.ts b/src/index.ts index 853a3a2ec0..883e67847a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -472,8 +472,10 @@ export { // Compares two GraphQLSchemas and detects breaking changes. BreakingChangeType, DangerousChangeType, + SafeChangeType, findBreakingChanges, findDangerousChanges, + findSchemaChanges, } from './utilities/index.js'; export type { @@ -501,6 +503,7 @@ export type { IntrospectionDirective, BuildSchemaOptions, BreakingChange, + SafeChange, DangerousChange, TypedQueryDocumentNode, } from './utilities/index.js'; diff --git a/src/utilities/__tests__/findBreakingChanges-test.ts b/src/utilities/__tests__/findBreakingChanges-test.ts index 8d7a3f8a56..dd2e1dc7fb 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.ts +++ b/src/utilities/__tests__/findBreakingChanges-test.ts @@ -16,7 +16,7 @@ import { DangerousChangeType, findBreakingChanges, findDangerousChanges, -} from '../findBreakingChanges.js'; +} from '../findSchemaChanges.js'; describe('findBreakingChanges', () => { it('should detect if a type was removed or not', () => { diff --git a/src/utilities/__tests__/findSchemaChanges-test.ts b/src/utilities/__tests__/findSchemaChanges-test.ts new file mode 100644 index 0000000000..60e5465dd5 --- /dev/null +++ b/src/utilities/__tests__/findSchemaChanges-test.ts @@ -0,0 +1,180 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { buildSchema } from '../buildASTSchema.js'; +import { findSchemaChanges, SafeChangeType } from '../findSchemaChanges.js'; + +describe('findSchemaChanges', () => { + it('should detect if a type was added', () => { + const newSchema = buildSchema(` + type Type1 + type Type2 + `); + + const oldSchema = buildSchema(` + type Type1 + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + type: SafeChangeType.TYPE_ADDED, + description: 'Type2 was added.', + }, + ]); + }); + + it('should detect if a field was added', () => { + const oldSchema = buildSchema(` + type Query { + foo: String + } + `); + + const newSchema = buildSchema(` + type Query { + foo: String + bar: String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + type: SafeChangeType.FIELD_ADDED, + description: 'Field Query.bar was added.', + }, + ]); + }); + + it('should detect if a default value was added', () => { + const oldSchema = buildSchema(` + type Query { + foo(x: String): String + } + `); + + const newSchema = buildSchema(` + type Query { + foo(x: String = "bar"): String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + type: SafeChangeType.ARG_DEFAULT_VALUE_ADDED, + description: 'Query.foo(x:) added a defaultValue "bar".', + }, + ]); + }); + + it('should detect if an arg value changes safely', () => { + const oldSchema = buildSchema(` + type Query { + foo(x: String!): String + } + `); + + const newSchema = buildSchema(` + type Query { + foo(x: String): String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + type: SafeChangeType.ARG_CHANGED_KIND_SAFE, + description: + 'Argument Query.foo(x:) has changed type from String! to String.', + }, + ]); + }); + + it('should detect if a directive was added', () => { + const oldSchema = buildSchema(` + type Query { + foo: String + } + `); + + const newSchema = buildSchema(` + directive @Foo on FIELD_DEFINITION + + type Query { + foo: String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + description: 'Directive @Foo was added.', + type: SafeChangeType.DIRECTIVE_ADDED, + }, + ]); + }); + + it('should detect if a directive becomes repeatable', () => { + const oldSchema = buildSchema(` + directive @Foo on FIELD_DEFINITION + + type Query { + foo: String + } + `); + + const newSchema = buildSchema(` + directive @Foo repeatable on FIELD_DEFINITION + + type Query { + foo: String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + description: 'Repeatable flag was added to @Foo.', + type: SafeChangeType.DIRECTIVE_REPEATABLE_ADDED, + }, + ]); + }); + + it('should detect if a directive adds locations', () => { + const oldSchema = buildSchema(` + directive @Foo on FIELD_DEFINITION + + type Query { + foo: String + } + `); + + const newSchema = buildSchema(` + directive @Foo on FIELD_DEFINITION | QUERY + + type Query { + foo: String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + description: 'QUERY was added to @Foo.', + type: SafeChangeType.DIRECTIVE_LOCATION_ADDED, + }, + ]); + }); + + it('should detect if a directive arg gets added', () => { + const oldSchema = buildSchema(` + directive @Foo on FIELD_DEFINITION + + type Query { + foo: String + } + `); + + const newSchema = buildSchema(` + directive @Foo(arg1: String) on FIELD_DEFINITION + + type Query { + foo: String + } + `); + expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([ + { + description: 'An optional argument @Foo(arg1:) was added.', + type: SafeChangeType.OPTIONAL_DIRECTIVE_ARG_ADDED, + }, + ]); + }); +}); diff --git a/src/utilities/findBreakingChanges.ts b/src/utilities/findSchemaChanges.ts similarity index 82% rename from src/utilities/findBreakingChanges.ts rename to src/utilities/findSchemaChanges.ts index e6f4f79055..909e3161e6 100644 --- a/src/utilities/findBreakingChanges.ts +++ b/src/utilities/findSchemaChanges.ts @@ -65,6 +65,21 @@ enum DangerousChangeType { } export { DangerousChangeType }; +enum SafeChangeType { + TYPE_ADDED = 'TYPE_ADDED', + OPTIONAL_INPUT_FIELD_ADDED = 'OPTIONAL_INPUT_FIELD_ADDED', + OPTIONAL_ARG_ADDED = 'OPTIONAL_ARG_ADDED', + DIRECTIVE_ADDED = 'DIRECTIVE_ADDED', + FIELD_ADDED = 'FIELD_ADDED', + DIRECTIVE_REPEATABLE_ADDED = 'DIRECTIVE_REPEATABLE_ADDED', + DIRECTIVE_LOCATION_ADDED = 'DIRECTIVE_LOCATION_ADDED', + OPTIONAL_DIRECTIVE_ARG_ADDED = 'OPTIONAL_DIRECTIVE_ARG_ADDED', + FIELD_CHANGED_KIND_SAFE = 'FIELD_CHANGED_KIND_SAFE', + ARG_CHANGED_KIND_SAFE = 'ARG_CHANGED_KIND_SAFE', + ARG_DEFAULT_VALUE_ADDED = 'ARG_DEFAULT_VALUE_ADDED', +} +export { SafeChangeType }; + export interface BreakingChange { type: BreakingChangeType; description: string; @@ -75,9 +90,18 @@ export interface DangerousChange { description: string; } +export interface SafeChange { + type: SafeChangeType; + description: string; +} + +export type SchemaChange = SafeChange | DangerousChange | BreakingChange; + /** * Given two schemas, returns an Array containing descriptions of all the types * of breaking changes covered by the other functions down below. + * + * @deprecated Please use `findSchemaChanges` instead. Will be removed in v18. */ export function findBreakingChanges( oldSchema: GraphQLSchema, @@ -92,6 +116,8 @@ export function findBreakingChanges( /** * Given two schemas, returns an Array containing descriptions of all the types * of potentially dangerous changes covered by the other functions down below. + * + * @deprecated Please use `findSchemaChanges` instead. Will be removed in v18. */ export function findDangerousChanges( oldSchema: GraphQLSchema, @@ -103,10 +129,10 @@ export function findDangerousChanges( ); } -function findSchemaChanges( +export function findSchemaChanges( oldSchema: GraphQLSchema, newSchema: GraphQLSchema, -): Array { +): Array { return [ ...findTypeChanges(oldSchema, newSchema), ...findDirectiveChanges(oldSchema, newSchema), @@ -116,7 +142,7 @@ function findSchemaChanges( function findDirectiveChanges( oldSchema: GraphQLSchema, newSchema: GraphQLSchema, -): Array { +): Array { const schemaChanges = []; const directivesDiff = diff( @@ -131,6 +157,13 @@ function findDirectiveChanges( }); } + for (const newDirective of directivesDiff.added) { + schemaChanges.push({ + type: SafeChangeType.DIRECTIVE_ADDED, + description: `Directive @${newDirective.name} was added.`, + }); + } + for (const [oldDirective, newDirective] of directivesDiff.persisted) { const argsDiff = diff(oldDirective.args, newDirective.args); @@ -140,6 +173,11 @@ function findDirectiveChanges( type: BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED, description: `A required argument @${oldDirective.name}(${newArg.name}:) was added.`, }); + } else { + schemaChanges.push({ + type: SafeChangeType.OPTIONAL_DIRECTIVE_ARG_ADDED, + description: `An optional argument @${oldDirective.name}(${newArg.name}:) was added.`, + }); } } @@ -155,6 +193,11 @@ function findDirectiveChanges( type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, description: `Repeatable flag was removed from @${oldDirective.name}.`, }); + } else if (newDirective.isRepeatable && !oldDirective.isRepeatable) { + schemaChanges.push({ + type: SafeChangeType.DIRECTIVE_REPEATABLE_ADDED, + description: `Repeatable flag was added to @${oldDirective.name}.`, + }); } for (const location of oldDirective.locations) { @@ -165,6 +208,15 @@ function findDirectiveChanges( }); } } + + for (const location of newDirective.locations) { + if (!oldDirective.locations.includes(location)) { + schemaChanges.push({ + type: SafeChangeType.DIRECTIVE_LOCATION_ADDED, + description: `${location} was added to @${oldDirective.name}.`, + }); + } + } } return schemaChanges; @@ -173,7 +225,7 @@ function findDirectiveChanges( function findTypeChanges( oldSchema: GraphQLSchema, newSchema: GraphQLSchema, -): Array { +): Array { const schemaChanges = []; const typesDiff = diff( @@ -190,6 +242,13 @@ function findTypeChanges( }); } + for (const newType of typesDiff.added) { + schemaChanges.push({ + type: SafeChangeType.TYPE_ADDED, + description: `${newType} was added.`, + }); + } + for (const [oldType, newType] of typesDiff.persisted) { if (isEnumType(oldType) && isEnumType(newType)) { schemaChanges.push(...findEnumTypeChanges(oldType, newType)); @@ -223,7 +282,7 @@ function findTypeChanges( function findInputObjectTypeChanges( oldType: GraphQLInputObjectType, newType: GraphQLInputObjectType, -): Array { +): Array { const schemaChanges = []; const fieldsDiff = diff( Object.values(oldType.getFields()), @@ -263,6 +322,13 @@ function findInputObjectTypeChanges( `Field ${oldType}.${oldField.name} changed type from ` + `${String(oldField.type)} to ${String(newField.type)}.`, }); + } else { + schemaChanges.push({ + type: SafeChangeType.FIELD_CHANGED_KIND_SAFE, + description: + `Field ${oldType}.${oldField.name} changed type from ` + + `${String(oldField.type)} to ${String(newField.type)}.`, + }); } } @@ -344,7 +410,7 @@ function findImplementedInterfacesChanges( function findFieldChanges( oldType: GraphQLObjectType | GraphQLInterfaceType, newType: GraphQLObjectType | GraphQLInterfaceType, -): Array { +): Array { const schemaChanges = []; const fieldsDiff = diff( Object.values(oldType.getFields()), @@ -358,6 +424,13 @@ function findFieldChanges( }); } + for (const newField of fieldsDiff.added) { + schemaChanges.push({ + type: SafeChangeType.FIELD_ADDED, + description: `Field ${oldType}.${newField.name} was added.`, + }); + } + for (const [oldField, newField] of fieldsDiff.persisted) { schemaChanges.push(...findArgChanges(oldType, oldField, newField)); @@ -372,6 +445,13 @@ function findFieldChanges( `Field ${oldType}.${oldField.name} changed type from ` + `${String(oldField.type)} to ${String(newField.type)}.`, }); + } else if (oldField.type.toString() !== newField.type.toString()) { + schemaChanges.push({ + type: SafeChangeType.FIELD_CHANGED_KIND_SAFE, + description: + `Field ${oldType}.${oldField.name} changed type from ` + + `${String(oldField.type)} to ${String(newField.type)}.`, + }); } } @@ -382,7 +462,7 @@ function findArgChanges( oldType: GraphQLObjectType | GraphQLInterfaceType, oldField: GraphQLField, newField: GraphQLField, -): Array { +): Array { const schemaChanges = []; const argsDiff = diff(oldField.args, newField.args); @@ -425,6 +505,22 @@ function findArgChanges( }); } } + } else if ( + newArg.defaultValue !== undefined && + oldArg.defaultValue === undefined + ) { + const newValueStr = stringifyValue(newArg.defaultValue, newArg.type); + schemaChanges.push({ + type: SafeChangeType.ARG_DEFAULT_VALUE_ADDED, + description: `${oldType}.${oldField.name}(${oldArg.name}:) added a defaultValue ${newValueStr}.`, + }); + } else { + schemaChanges.push({ + type: SafeChangeType.ARG_CHANGED_KIND_SAFE, + description: + `Argument ${oldType}.${oldField.name}(${oldArg.name}:) has changed type from ` + + `${String(oldArg.type)} to ${String(newArg.type)}.`, + }); } } diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 12dba542dc..0d2ac8d8e7 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -104,10 +104,16 @@ export { export { BreakingChangeType, DangerousChangeType, + SafeChangeType, findBreakingChanges, findDangerousChanges, -} from './findBreakingChanges.js'; -export type { BreakingChange, DangerousChange } from './findBreakingChanges.js'; + findSchemaChanges, +} from './findSchemaChanges.js'; +export type { + BreakingChange, + DangerousChange, + SafeChange, +} from './findSchemaChanges.js'; // Wrapper type that contains DocumentNode and types that can be deduced from it. export type { TypedQueryDocumentNode } from './typedQueryDocumentNode.js';