Skip to content

Commit 088cd4f

Browse files
authored
fix simplifyChanges rule for sibling changes (#2921)
1 parent d415188 commit 088cd4f

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: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,4 +269,110 @@ 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+
const b = buildSchema(/* GraphQL */ `
293+
type Query {
294+
users: [User!]
295+
}
296+
297+
enum UserRole {
298+
ADMIN
299+
EDITOR
300+
VIEWER
301+
}
302+
303+
type User {
304+
id: ID!
305+
name: String!
306+
address: String
307+
role: UserRole!
308+
}
309+
`);
310+
311+
const changes = await diff(a, b, [simplifyChanges]);
312+
// `User` has 3 fields that change
313+
expect(changes).toHaveLength(4);
314+
expect(changes).toMatchInlineSnapshot(`
315+
[
316+
{
317+
"criticality": {
318+
"level": "DANGEROUS",
319+
"reason": "Adding an enum value may break existing clients that were not programming defensively against an added case when querying an enum.",
320+
},
321+
"message": "Enum value 'VIEWER' was added to enum 'UserRole'",
322+
"meta": {
323+
"addedDirectiveDescription": null,
324+
"addedEnumValueName": "VIEWER",
325+
"addedToNewType": false,
326+
"enumName": "UserRole",
327+
},
328+
"path": "UserRole.VIEWER",
329+
"type": "ENUM_VALUE_ADDED",
330+
},
331+
{
332+
"criticality": {
333+
"level": "BREAKING",
334+
"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.",
335+
},
336+
"message": "Field 'email' was removed from object type 'User'",
337+
"meta": {
338+
"isRemovedFieldDeprecated": false,
339+
"removedFieldName": "email",
340+
"typeName": "User",
341+
"typeType": "object type",
342+
},
343+
"path": "User.email",
344+
"type": "FIELD_REMOVED",
345+
},
346+
{
347+
"criticality": {
348+
"level": "NON_BREAKING",
349+
},
350+
"message": "Field 'address' was added to object type 'User'",
351+
"meta": {
352+
"addedFieldName": "address",
353+
"addedFieldReturnType": "String",
354+
"typeName": "User",
355+
"typeType": "object type",
356+
},
357+
"path": "User.address",
358+
"type": "FIELD_ADDED",
359+
},
360+
{
361+
"criticality": {
362+
"level": "NON_BREAKING",
363+
},
364+
"message": "Field 'User.role' changed type from 'UserRole' to 'UserRole!'",
365+
"meta": {
366+
"fieldName": "role",
367+
"isSafeFieldTypeChange": true,
368+
"newFieldType": "UserRole!",
369+
"oldFieldType": "UserRole",
370+
"typeName": "User",
371+
},
372+
"path": "User.role",
373+
"type": "FIELD_TYPE_CHANGED",
374+
},
375+
]
376+
`);
377+
});
272378
});

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ 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(
59+
matchedPath => matchedPath === parent || matchedPath === path,
60+
);
5761
const hasChangedParent = matches.length > 0;
5862

5963
if (simpleChangeTypes.has(type)) {

0 commit comments

Comments
 (0)