Skip to content

Commit cad75ff

Browse files
committed
fix simplifyChanges for sibling changes
1 parent d415188 commit cad75ff

File tree

3 files changed

+116
-1
lines changed

3 files changed

+116
-1
lines changed

.changeset/red-beds-study.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-inspector/core': patch
3+
---
4+
5+
Fix simplifyChanges for sibling changes. E.g. if User.foo and User.bar were added, then the later would be filtered out when it shouldn't be filtered.

packages/core/__tests__/diff/rules/simplify-changes.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,4 +269,112 @@ describe('simplifyChanges rule', () => {
269269
expect(change.type).toEqual('FIELD_DEPRECATION_REMOVED');
270270
expect(change.message).toEqual("Field 'Foo.bar' is no longer deprecated");
271271
});
272+
273+
test('same node contains multiple changes', async () => {
274+
const a = buildSchema(/* GraphQL */ `
275+
type Query {
276+
users: [User!]
277+
}
278+
279+
enum UserRole {
280+
ADMIN
281+
EDITOR
282+
}
283+
284+
type User {
285+
id: ID!
286+
name: String!
287+
email: String!
288+
role: UserRole
289+
}
290+
291+
`);
292+
293+
const b = buildSchema(/* GraphQL */ `
294+
type Query {
295+
users: [User!]
296+
}
297+
298+
enum UserRole {
299+
ADMIN
300+
EDITOR
301+
VIEWER
302+
}
303+
304+
type User {
305+
id: ID!
306+
name: String!
307+
address: String
308+
role: UserRole!
309+
}
310+
311+
`);
312+
313+
const changes = await diff(a, b, [simplifyChanges]);
314+
// `User` has 3 fields that change
315+
expect(changes).toHaveLength(4);
316+
expect(changes).toMatchInlineSnapshot(`
317+
[
318+
{
319+
"criticality": {
320+
"level": "DANGEROUS",
321+
"reason": "Adding an enum value may break existing clients that were not programming defensively against an added case when querying an enum.",
322+
},
323+
"message": "Enum value 'VIEWER' was added to enum 'UserRole'",
324+
"meta": {
325+
"addedDirectiveDescription": null,
326+
"addedEnumValueName": "VIEWER",
327+
"addedToNewType": false,
328+
"enumName": "UserRole",
329+
},
330+
"path": "UserRole.VIEWER",
331+
"type": "ENUM_VALUE_ADDED",
332+
},
333+
{
334+
"criticality": {
335+
"level": "BREAKING",
336+
"reason": "Removing a field is a breaking change. It is preferable to deprecate the field before removing it. This applies to removed union fields as well, since removal breaks client operations that contain fragments that reference the removed type through direct (... on RemovedType) or indirect means such as __typename in the consumers.",
337+
},
338+
"message": "Field 'email' was removed from object type 'User'",
339+
"meta": {
340+
"isRemovedFieldDeprecated": false,
341+
"removedFieldName": "email",
342+
"typeName": "User",
343+
"typeType": "object type",
344+
},
345+
"path": "User.email",
346+
"type": "FIELD_REMOVED",
347+
},
348+
{
349+
"criticality": {
350+
"level": "NON_BREAKING",
351+
},
352+
"message": "Field 'address' was added to object type 'User'",
353+
"meta": {
354+
"addedFieldName": "address",
355+
"addedFieldReturnType": "String",
356+
"typeName": "User",
357+
"typeType": "object type",
358+
},
359+
"path": "User.address",
360+
"type": "FIELD_ADDED",
361+
},
362+
{
363+
"criticality": {
364+
"level": "NON_BREAKING",
365+
},
366+
"message": "Field 'User.role' changed type from 'UserRole' to 'UserRole!'",
367+
"meta": {
368+
"fieldName": "role",
369+
"isSafeFieldTypeChange": true,
370+
"newFieldType": "UserRole!",
371+
"oldFieldType": "UserRole",
372+
"typeName": "User",
373+
},
374+
"path": "User.role",
375+
"type": "FIELD_TYPE_CHANGED",
376+
},
377+
]
378+
`);
379+
})
272380
});

packages/core/src/diff/rules/simplify-changes.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ export const simplifyChanges: Rule = ({ changes }) => {
5353
const filteredChanges = changes.filter(({ path, type }) => {
5454
if (path) {
5555
const parent = parentPath(path);
56-
const matches = changePaths.filter(matchedPath => matchedPath.startsWith(parent));
56+
// check if parent or the current node was changed using a
57+
// "simple change" (aka a change that encapsulates a group of changes)
58+
const matches = changePaths.filter(matchedPath => matchedPath === parent || matchedPath === path);
5759
const hasChangedParent = matches.length > 0;
5860

5961
if (simpleChangeTypes.has(type)) {

0 commit comments

Comments
 (0)