Skip to content

Commit cca3f98

Browse files
authored
Detect directive argument changes (#4277)
Resolves #4237 This mirrors the fields/input-object argument logic but specifically for directives.
1 parent 51e53d7 commit cca3f98

File tree

2 files changed

+197
-1
lines changed

2 files changed

+197
-1
lines changed

src/utilities/__tests__/findSchemaChanges-test.ts

Lines changed: 147 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

44
import { buildSchema } from '../buildASTSchema.js';
5-
import { findSchemaChanges, SafeChangeType } from '../findSchemaChanges.js';
5+
import {
6+
BreakingChangeType,
7+
DangerousChangeType,
8+
findSchemaChanges,
9+
SafeChangeType,
10+
} from '../findSchemaChanges.js';
611

712
describe('findSchemaChanges', () => {
813
it('should detect if a type was added', () => {
@@ -171,6 +176,147 @@ describe('findSchemaChanges', () => {
171176
]);
172177
});
173178

179+
it('should detect if a changes argument safely', () => {
180+
const oldSchema = buildSchema(`
181+
directive @Foo(foo: String!) on FIELD_DEFINITION
182+
183+
type Query {
184+
foo: String
185+
}
186+
`);
187+
188+
const newSchema = buildSchema(`
189+
directive @Foo(foo: String) on FIELD_DEFINITION
190+
191+
type Query {
192+
foo: String
193+
}
194+
`);
195+
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
196+
{
197+
description:
198+
'Argument @Foo(foo:) has changed type from String! to String.',
199+
type: SafeChangeType.ARG_CHANGED_KIND_SAFE,
200+
},
201+
]);
202+
});
203+
204+
it('should detect if a default value is added to an argument', () => {
205+
const oldSchema = buildSchema(`
206+
directive @Foo(foo: String) on FIELD_DEFINITION
207+
208+
type Query {
209+
foo: String
210+
}
211+
`);
212+
213+
const newSchema = buildSchema(`
214+
directive @Foo(foo: String = "Foo") on FIELD_DEFINITION
215+
216+
type Query {
217+
foo: String
218+
}
219+
`);
220+
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
221+
{
222+
description: '@Foo(foo:) added a defaultValue "Foo".',
223+
type: SafeChangeType.ARG_DEFAULT_VALUE_ADDED,
224+
},
225+
]);
226+
});
227+
228+
it('should detect if a default value is removed from an argument', () => {
229+
const newSchema = buildSchema(`
230+
directive @Foo(foo: String) on FIELD_DEFINITION
231+
232+
type Query {
233+
foo: String
234+
}
235+
`);
236+
237+
const oldSchema = buildSchema(`
238+
directive @Foo(foo: String = "Foo") on FIELD_DEFINITION
239+
240+
type Query {
241+
foo: String
242+
}
243+
`);
244+
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
245+
{
246+
description: '@Foo(foo:) defaultValue was removed.',
247+
type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE,
248+
},
249+
]);
250+
});
251+
252+
it('should detect if a default value is changed in an argument', () => {
253+
const oldSchema = buildSchema(`
254+
directive @Foo(foo: String = "Bar") on FIELD_DEFINITION
255+
256+
type Query {
257+
foo: String
258+
}
259+
`);
260+
261+
const newSchema = buildSchema(`
262+
directive @Foo(foo: String = "Foo") on FIELD_DEFINITION
263+
264+
type Query {
265+
foo: String
266+
}
267+
`);
268+
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
269+
{
270+
description: '@Foo(foo:) has changed defaultValue from "Bar" to "Foo".',
271+
type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE,
272+
},
273+
]);
274+
});
275+
276+
it('should detect if a directive argument does a breaking change', () => {
277+
const oldSchema = buildSchema(`
278+
directive @Foo(foo: String) on FIELD_DEFINITION
279+
280+
type Query {
281+
foo: String
282+
}
283+
`);
284+
285+
const newSchema = buildSchema(`
286+
directive @Foo(foo: String!) on FIELD_DEFINITION
287+
288+
type Query {
289+
foo: String
290+
}
291+
`);
292+
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([
293+
{
294+
description:
295+
'Argument @Foo(foo:) has changed type from String to String!.',
296+
type: BreakingChangeType.ARG_CHANGED_KIND,
297+
},
298+
]);
299+
});
300+
301+
it('should not detect if a directive argument default value does not change', () => {
302+
const oldSchema = buildSchema(`
303+
directive @Foo(foo: String = "FOO") on FIELD_DEFINITION
304+
305+
type Query {
306+
foo: String
307+
}
308+
`);
309+
310+
const newSchema = buildSchema(`
311+
directive @Foo(foo: String = "FOO") on FIELD_DEFINITION
312+
313+
type Query {
314+
foo: String
315+
}
316+
`);
317+
expect(findSchemaChanges(oldSchema, newSchema)).to.deep.equal([]);
318+
});
319+
174320
it('should detect if a directive changes description', () => {
175321
const oldSchema = buildSchema(`
176322
directive @Foo on FIELD_DEFINITION

src/utilities/findSchemaChanges.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,56 @@ function findDirectiveChanges(
196196
}
197197

198198
for (const [oldArg, newArg] of argsDiff.persisted) {
199+
const isSafe = isChangeSafeForInputObjectFieldOrFieldArg(
200+
oldArg.type,
201+
newArg.type,
202+
);
203+
204+
if (!isSafe) {
205+
schemaChanges.push({
206+
type: BreakingChangeType.ARG_CHANGED_KIND,
207+
description:
208+
`Argument @${oldDirective.name}(${oldArg.name}:) has changed type from ` +
209+
`${String(oldArg.type)} to ${String(newArg.type)}.`,
210+
});
211+
} else if (oldArg.defaultValue !== undefined) {
212+
if (newArg.defaultValue === undefined) {
213+
schemaChanges.push({
214+
type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE,
215+
description: `@${oldDirective.name}(${oldArg.name}:) defaultValue was removed.`,
216+
});
217+
} else {
218+
// Since we looking only for client's observable changes we should
219+
// compare default values in the same representation as they are
220+
// represented inside introspection.
221+
const oldValueStr = stringifyValue(oldArg.defaultValue, oldArg.type);
222+
const newValueStr = stringifyValue(newArg.defaultValue, newArg.type);
223+
224+
if (oldValueStr !== newValueStr) {
225+
schemaChanges.push({
226+
type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE,
227+
description: `@${oldDirective.name}(${oldArg.name}:) has changed defaultValue from ${oldValueStr} to ${newValueStr}.`,
228+
});
229+
}
230+
}
231+
} else if (
232+
newArg.defaultValue !== undefined &&
233+
oldArg.defaultValue === undefined
234+
) {
235+
const newValueStr = stringifyValue(newArg.defaultValue, newArg.type);
236+
schemaChanges.push({
237+
type: SafeChangeType.ARG_DEFAULT_VALUE_ADDED,
238+
description: `@${oldDirective.name}(${oldArg.name}:) added a defaultValue ${newValueStr}.`,
239+
});
240+
} else if (oldArg.type.toString() !== newArg.type.toString()) {
241+
schemaChanges.push({
242+
type: SafeChangeType.ARG_CHANGED_KIND_SAFE,
243+
description:
244+
`Argument @${oldDirective.name}(${oldArg.name}:) has changed type from ` +
245+
`${String(oldArg.type)} to ${String(newArg.type)}.`,
246+
});
247+
}
248+
199249
if (oldArg.description !== newArg.description) {
200250
schemaChanges.push({
201251
type: SafeChangeType.DESCRIPTION_CHANGED,

0 commit comments

Comments
 (0)