Skip to content
New issue

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

Sign up for GitHub

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

Jump to bottom

Add safe schema change for descriptions changing #4275

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Add safe schema change for descriptions changing #4275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 180 additions & 0 deletions src/utilities/__tests__/findSchemaChanges-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ describe('findSchemaChanges', () => {
]);
});

it('should detect a type changing description', () => {
const newSchema = buildSchema(`
"New Description"
type Type1
`);

const oldSchema = buildSchema(`
type Type1
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
type: SafeChangeType.DESCRIPTION_CHANGED,
description: 'Description of Type1 has changed to "New Description".',
},
]);
});

it('should detect if a field was added', () => {
const oldSchema = buildSchema(`
type Query {
Expand All @@ -43,6 +60,30 @@ describe('findSchemaChanges', () => {
]);
});

it('should detect a field changing description', () => {
const oldSchema = buildSchema(`
type Query {
foo: String
bar: String
}
`);

const newSchema = buildSchema(`
type Query {
foo: String
"New Description"
bar: String
}
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
type: SafeChangeType.DESCRIPTION_CHANGED,
description:
'Description of field Query.bar has changed to "New Description".',
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
},
]);
});

it('should detect if a default value was added', () => {
const oldSchema = buildSchema(`
type Query {
Expand Down Expand Up @@ -84,6 +125,30 @@ describe('findSchemaChanges', () => {
]);
});

it('should detect if an arg value changes description', () => {
const oldSchema = buildSchema(`
type Query {
foo(x: String!): String
}
`);

const newSchema = buildSchema(`
type Query {
foo(
"New Description"
x: String!
): String
}
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
type: SafeChangeType.DESCRIPTION_CHANGED,
description:
'Description of argument Query.foo(x) has changed to "New Description".',
},
]);
});

it('should detect if a directive was added', () => {
const oldSchema = buildSchema(`
type Query {
Expand All @@ -106,6 +171,31 @@ describe('findSchemaChanges', () => {
]);
});

it('should detect if a directive changes description', () => {
const oldSchema = buildSchema(`
directive @Foo on FIELD_DEFINITION

type Query {
foo: String
}
`);

const newSchema = buildSchema(`
"New Description"
directive @Foo on FIELD_DEFINITION

type Query {
foo: String
}
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
description: 'Description of @Foo has changed to "New Description".',
type: SafeChangeType.DESCRIPTION_CHANGED,
},
]);
});

it('should detect if a directive becomes repeatable', () => {
const oldSchema = buildSchema(`
directive @Foo on FIELD_DEFINITION
Expand Down Expand Up @@ -177,4 +267,94 @@ describe('findSchemaChanges', () => {
},
]);
});

it('should detect if a directive arg changes description', () => {
const oldSchema = buildSchema(`
directive @Foo(
arg1: String
) on FIELD_DEFINITION

type Query {
foo: String
}
`);

const newSchema = buildSchema(`
directive @Foo(
"New Description"
arg1: String
) on FIELD_DEFINITION

type Query {
foo: String
}
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
description:
'Description of @Foo(Foo) has changed to "New Description".',
type: SafeChangeType.DESCRIPTION_CHANGED,
},
]);
});

it('should detect if an enum member changes description', () => {
const oldSchema = buildSchema(`
enum Foo {
TEST
}

type Query {
foo: String
}
`);

const newSchema = buildSchema(`
enum Foo {
"New Description"
TEST
}

type Query {
foo: String
}
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
description:
'Description of enum value Foo.TEST has changed to "New Description".',
type: SafeChangeType.DESCRIPTION_CHANGED,
},
]);
});

it('should detect if an input field changes description', () => {
const oldSchema = buildSchema(`
input Foo {
x: String
}

type Query {
foo: String
}
`);

const newSchema = buildSchema(`
input Foo {
"New Description"
x: String
}

type Query {
foo: String
}
`);
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
{
description:
'Description of input-field Foo.x has changed to "New Description".',
type: SafeChangeType.DESCRIPTION_CHANGED,
},
]);
});
});
61 changes: 58 additions & 3 deletions src/utilities/findSchemaChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export type DangerousChangeType =
(typeof DangerousChangeType)[keyof typeof DangerousChangeType];

export const SafeChangeType = {
DESCRIPTION_CHANGED: 'DESCRIPTION_CHANGED' as const,
TYPE_ADDED: 'TYPE_ADDED' as const,
OPTIONAL_INPUT_FIELD_ADDED: 'OPTIONAL_INPUT_FIELD_ADDED' as const,
OPTIONAL_ARG_ADDED: 'OPTIONAL_ARG_ADDED' as const,
Expand Down Expand Up @@ -194,6 +195,15 @@ function findDirectiveChanges(
});
}

for (const [oldArg, newArg] of argsDiff.persisted) {
if (oldArg.description !== newArg.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of @${oldDirective.name}(${oldDirective.name}) has changed to "${newArg.description}".`,
});
}
}

if (oldDirective.isRepeatable && !newDirective.isRepeatable) {
schemaChanges.push({
type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED,
Expand All @@ -206,6 +216,13 @@ function findDirectiveChanges(
});
}

if (oldDirective.description !== newDirective.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of @${oldDirective.name} has changed to "${newDirective.description}".`,
});
}

for (const location of oldDirective.locations) {
if (!newDirective.locations.includes(location)) {
schemaChanges.push({
Expand Down Expand Up @@ -256,6 +273,13 @@ function findTypeChanges(
}

for (const [oldType, newType] of typesDiff.persisted) {
if (oldType.description !== newType.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of ${oldType.name} has changed to "${newType.description}".`,
});
}

if (isEnumType(oldType) && isEnumType(newType)) {
schemaChanges.push(...findEnumTypeChanges(oldType, newType));
} else if (isUnionType(oldType) && isUnionType(newType)) {
Expand Down Expand Up @@ -328,14 +352,21 @@ function findInputObjectTypeChanges(
`Field ${oldType}.${oldField.name} changed type from ` +
`${String(oldField.type)} to ${String(newField.type)}.`,
});
} else {
} 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)}.`,
});
}

if (oldField.description !== newField.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of input-field ${newType}.${newField.name} has changed to "${newField.description}".`,
});
}
}

return schemaChanges;
Expand Down Expand Up @@ -368,7 +399,7 @@ function findUnionTypeChanges(
function findEnumTypeChanges(
oldType: GraphQLEnumType,
newType: GraphQLEnumType,
): Array<BreakingChange | DangerousChange> {
): Array<SchemaChange> {
const schemaChanges = [];
const valuesDiff = diff(oldType.getValues(), newType.getValues());

Expand All @@ -386,6 +417,15 @@ function findEnumTypeChanges(
});
}

for (const [oldValue, newValue] of valuesDiff.persisted) {
if (oldValue.description !== newValue.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of enum value ${oldType}.${oldValue.name} has changed to "${newValue.description}".`,
});
}
}

return schemaChanges;
}

Expand Down Expand Up @@ -459,6 +499,13 @@ function findFieldChanges(
`${String(oldField.type)} to ${String(newField.type)}.`,
});
}

if (oldField.description !== newField.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of field ${oldType}.${oldField.name} has changed to "${newField.description}".`,
});
}
}

return schemaChanges;
Expand All @@ -484,6 +531,7 @@ function findArgChanges(
oldArg.type,
newArg.type,
);

if (!isSafe) {
schemaChanges.push({
type: BreakingChangeType.ARG_CHANGED_KIND,
Expand Down Expand Up @@ -520,14 +568,21 @@ function findArgChanges(
type: SafeChangeType.ARG_DEFAULT_VALUE_ADDED,
description: `${oldType}.${oldField.name}(${oldArg.name}:) added a defaultValue ${newValueStr}.`,
});
} else {
} else if (oldArg.type.toString() !== newArg.type.toString()) {
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)}.`,
});
}

if (oldArg.description !== newArg.description) {
schemaChanges.push({
type: SafeChangeType.DESCRIPTION_CHANGED,
description: `Description of argument ${oldType}.${oldField.name}(${oldArg.name}) has changed to "${newArg.description}".`,
});
}
}

for (const newArg of argsDiff.added) {
Expand Down
Loading