Skip to content

Add patch package to apply diff changes to schemas #2893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
6 changes: 6 additions & 0 deletions .changeset/seven-jars-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@graphql-inspector/core': major
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be convinced that this is a minor patch because the changes to the output's format doesnt break anything. But the actual content of the changes has changed drastically which is why I thought we should be safe and declare a major change.

---

"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.
75 changes: 49 additions & 26 deletions packages/core/__tests__/diff/directive-usage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,36 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Query.a.external');
const change = findFirstChangeByPath(changes, 'Query.a.@external');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an indicator for directives is necessary to distinguish them from arguments. This makes the paths more meaningful and useful as lookups.


expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_FIELD_DEFINITION_ADDED');
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
Expand All @@ -44,7 +67,7 @@ describe('directive-usage', () => {
}
`);

const change = findFirstChangeByPath(await diff(a, b), 'Query.a.external');
const change = findFirstChangeByPath(await diff(a, b), 'Query.a.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_FIELD_DEFINITION_REMOVED');
Expand All @@ -68,7 +91,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Query.a.oneOf');
const change = findFirstChangeByPath(changes, 'Query.a.@oneOf');

expect(change.criticality.level).toEqual(CriticalityLevel.Breaking);
expect(change.type).toEqual('DIRECTIVE_USAGE_FIELD_DEFINITION_ADDED');
Expand All @@ -91,7 +114,7 @@ describe('directive-usage', () => {
}
`);

const change = findFirstChangeByPath(await diff(a, b), 'Query.a.oneOf');
const change = findFirstChangeByPath(await diff(a, b), 'Query.a.@oneOf');

expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
expect(change.type).toEqual('DIRECTIVE_USAGE_FIELD_DEFINITION_REMOVED');
Expand Down Expand Up @@ -128,7 +151,7 @@ describe('directive-usage', () => {
union Foo @external = A | B
`);

const change = findFirstChangeByPath(await diff(a, b), 'Foo.external');
const change = findFirstChangeByPath(await diff(a, b), 'Foo.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_UNION_MEMBER_ADDED');
Expand Down Expand Up @@ -164,7 +187,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_UNION_MEMBER_REMOVED');
Expand Down Expand Up @@ -199,7 +222,7 @@ describe('directive-usage', () => {
union Foo @oneOf = A | B
`);

const change = findFirstChangeByPath(await diff(a, b), 'Foo.oneOf');
const change = findFirstChangeByPath(await diff(a, b), 'Foo.@oneOf');

expect(change.criticality.level).toEqual(CriticalityLevel.Breaking);
expect(change.type).toEqual('DIRECTIVE_USAGE_UNION_MEMBER_ADDED');
Expand Down Expand Up @@ -235,7 +258,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.oneOf');
const change = findFirstChangeByPath(changes, 'Foo.@oneOf');

expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
expect(change.type).toEqual('DIRECTIVE_USAGE_UNION_MEMBER_REMOVED');
Expand Down Expand Up @@ -270,7 +293,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'enumA.external');
const change = findFirstChangeByPath(changes, 'enumA.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.criticality.reason).toBeDefined();
Expand Down Expand Up @@ -302,7 +325,7 @@ describe('directive-usage', () => {
}
`);

const change = findFirstChangeByPath(await diff(a, b), 'enumA.external');
const change = findFirstChangeByPath(await diff(a, b), 'enumA.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_ENUM_REMOVED');
Expand Down Expand Up @@ -338,7 +361,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'enumA.B.external');
const change = findFirstChangeByPath(changes, 'enumA.B.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand Down Expand Up @@ -373,7 +396,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'enumA.A.external');
const change = findFirstChangeByPath(changes, 'enumA.A.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -400,7 +423,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -424,7 +447,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -451,7 +474,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.a.external');
const change = findFirstChangeByPath(changes, 'Foo.a.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -477,7 +500,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.a.external');
const change = findFirstChangeByPath(changes, 'Foo.a.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -500,7 +523,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -518,7 +541,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
Expand All @@ -543,7 +566,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_OBJECT_ADDED');
Expand All @@ -564,7 +587,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_OBJECT_REMOVED');
Expand All @@ -588,7 +611,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_INTERFACE_ADDED');
Expand All @@ -610,7 +633,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, 'Foo.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_INTERFACE_REMOVED');
Expand All @@ -634,7 +657,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.a.a.external');
const change = findFirstChangeByPath(changes, 'Foo.a.a.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_ARGUMENT_DEFINITION_ADDED');
Expand All @@ -658,7 +681,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.a.a.external');
const change = findFirstChangeByPath(changes, 'Foo.a.a.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_ARGUMENT_DEFINITION_REMOVED');
Expand Down Expand Up @@ -690,7 +713,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, '.@external');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indicates the directive is applied to the schema object. This . is necessary to distinguish directive usages from directive definitions at the root level.


expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_SCHEMA_ADDED');
Expand All @@ -717,7 +740,7 @@ describe('directive-usage', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'Foo.external');
const change = findFirstChangeByPath(changes, '.@external');

expect(change.criticality.level).toEqual(CriticalityLevel.Dangerous);
expect(change.type).toEqual('DIRECTIVE_USAGE_SCHEMA_REMOVED');
Expand Down
12 changes: 6 additions & 6 deletions packages/core/__tests__/diff/directive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,13 @@ describe('directive', () => {
};

// Nullable
expect(change.a.criticality.level).toEqual(CriticalityLevel.NonBreaking);
expect(change.a.type).toEqual('DIRECTIVE_ARGUMENT_ADDED');
expect(change.a.message).toEqual(`Argument 'name' was added to directive 'a'`);
expect(change.a?.type).toEqual('DIRECTIVE_ARGUMENT_ADDED');
expect(change.a?.criticality.level).toEqual(CriticalityLevel.NonBreaking);
Copy link
Collaborator Author

@jdolle jdolle Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In several places I reordered things because when tweaking behavior and testing, t was not providing me enough insights to understand what was wrong. Placing the "type" first let me know which change type was being returned.

expect(change.a?.message).toEqual(`Argument 'name' was added to directive 'a'`);
// Non-nullable
expect(change.b.criticality.level).toEqual(CriticalityLevel.Breaking);
expect(change.b.type).toEqual('DIRECTIVE_ARGUMENT_ADDED');
expect(change.b.message).toEqual(`Argument 'name' was added to directive 'b'`);
expect(change.b?.type).toEqual('DIRECTIVE_ARGUMENT_ADDED');
expect(change.b?.criticality.level).toEqual(CriticalityLevel.Breaking);
expect(change.b?.message).toEqual(`Argument 'name' was added to directive 'b'`);
});

test('removed', async () => {
Expand Down
79 changes: 71 additions & 8 deletions packages/core/__tests__/diff/enum.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,56 @@
import { buildSchema } from 'graphql';
import { CriticalityLevel, diff, DiffRule } from '../../src/index.js';
import { findFirstChangeByPath } from '../../utils/testing.js';
import { ChangeType, CriticalityLevel, diff, DiffRule } from '../../src/index.js';
import { findChangesByPath, 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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test shows that enum additions also contain all nested changes within that enum, and that those changes are flagged as non-breaking.

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 {
Expand Down Expand Up @@ -130,7 +178,7 @@ describe('enum', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'enumA.A');
const change = findFirstChangeByPath(changes, 'enumA.A.@deprecated');

expect(changes.length).toEqual(1);
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
Expand Down Expand Up @@ -163,11 +211,26 @@ describe('enum', () => {
`);

const changes = await diff(a, b);
const change = findFirstChangeByPath(changes, 'enumA.A');

expect(changes.length).toEqual(2);
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
expect(change.message).toEqual(`Enum value 'enumA.A' was deprecated with reason 'New Reason'`);
expect(changes).toHaveLength(3);
const directiveChanges = findChangesByPath(changes, 'enumA.A.@deprecated');
expect(directiveChanges).toHaveLength(2);

for (const change of directiveChanges) {
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
if (change.type === ChangeType.EnumValueDeprecationReasonAdded) {
expect(change.message).toEqual(
`Enum value 'enumA.A' was deprecated with reason 'New Reason'`,
);
} else if (change.type === ChangeType.DirectiveUsageEnumValueAdded) {
expect(change.message).toEqual(`Directive 'deprecated' was added to enum value 'enumA.A'`);
}
}

{
const change = findFirstChangeByPath(changes, '[email protected]');
expect(change.type).toEqual(ChangeType.DirectiveUsageArgumentAdded);
expect(change.message).toEqual(`Argument 'reason' was added to '@deprecated'`);
}
});

test('deprecation reason removed', async () => {
Expand Down
Loading
Loading