From 41940b29aa8b3f42b2dd40acc0854e1d9f74773c Mon Sep 17 00:00:00 2001 From: "jane.li" Date: Fri, 4 Apr 2025 18:52:10 +0000 Subject: [PATCH 1/3] input field added for non-null type with default value is non-breaking Summary: update cases. Reviewers: ureview JIRA Issues: RF-19036 Differential Revision: https://code.uberinternal.com/D17076739 --- packages/core/src/diff/changes/change.ts | 2 ++ packages/core/src/diff/changes/input.ts | 12 ++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/core/src/diff/changes/change.ts b/packages/core/src/diff/changes/change.ts index 8ef6ea7181..3d487929d4 100644 --- a/packages/core/src/diff/changes/change.ts +++ b/packages/core/src/diff/changes/change.ts @@ -453,6 +453,8 @@ export type InputFieldAddedChange = { addedInputFieldName: string; isAddedInputFieldTypeNullable: boolean; addedInputFieldType: string; + hasDefaultValue: boolean; + isAddedInputFieldBreaking: boolean; }; }; diff --git a/packages/core/src/diff/changes/input.ts b/packages/core/src/diff/changes/input.ts index a4c0395800..8809f34bf0 100644 --- a/packages/core/src/diff/changes/input.ts +++ b/packages/core/src/diff/changes/input.ts @@ -56,14 +56,14 @@ export function buildInputFieldAddedMessage(args: InputFieldAddedChange['meta']) export function inputFieldAddedFromMeta(args: InputFieldAddedChange) { return { type: ChangeType.InputFieldAdded, - criticality: args.meta.isAddedInputFieldTypeNullable + criticality: args.meta.isAddedInputFieldBreaking ? { - level: CriticalityLevel.Dangerous, - } - : { level: CriticalityLevel.Breaking, reason: 'Adding a required input field to an existing input object type is a breaking change because it will cause existing uses of this input object type to error.', + } + : { + level: CriticalityLevel.Dangerous, }, message: buildInputFieldAddedMessage(args.meta), meta: args.meta, @@ -75,6 +75,8 @@ export function inputFieldAdded( input: GraphQLInputObjectType, field: GraphQLInputField, ): Change { + const isBreaking = isNonNullType(field.type) && typeof field.defaultValue === 'undefined'; + return inputFieldAddedFromMeta({ type: ChangeType.InputFieldAdded, meta: { @@ -82,6 +84,8 @@ export function inputFieldAdded( addedInputFieldName: field.name, isAddedInputFieldTypeNullable: !isNonNullType(field.type), addedInputFieldType: field.type.toString(), + hasDefaultValue: field.defaultValue != null, + isAddedInputFieldBreaking: isBreaking, }, }); } From 57f44dc2d7f506d3e21725e51062cb2f2b2d65e6 Mon Sep 17 00:00:00 2001 From: "jane.li" Date: Fri, 4 Apr 2025 19:29:25 +0000 Subject: [PATCH 2/3] update test --- packages/core/__tests__/diff/input.test.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/core/__tests__/diff/input.test.ts b/packages/core/__tests__/diff/input.test.ts index 8f6d2719cf..c6591522f9 100644 --- a/packages/core/__tests__/diff/input.test.ts +++ b/packages/core/__tests__/diff/input.test.ts @@ -17,15 +17,18 @@ describe('input', () => { b: String! c: String! d: String + e: String! = "Eee" } `); + const changes = await diff(a, b); const change = { - c: findFirstChangeByPath(await diff(a, b), 'Foo.c'), - d: findFirstChangeByPath(await diff(a, b), 'Foo.d'), + c: findFirstChangeByPath(changes, 'Foo.c'), + d: findFirstChangeByPath(changes, 'Foo.d'), + e: findFirstChangeByPath(changes, 'Foo.e'), }; - // Non-nullable + // Non-nullable without default expect(change.c.criticality.level).toEqual(CriticalityLevel.Breaking); expect(change.c.type).toEqual('INPUT_FIELD_ADDED'); expect(change.c.message).toEqual( @@ -37,6 +40,12 @@ describe('input', () => { expect(change.d.message).toEqual( "Input field 'd' of type 'String' was added to input object type 'Foo'", ); + // Non-nullable with default + expect(change.e.criticality.level).toEqual(CriticalityLevel.Dangerous); + expect(change.e.type).toEqual('INPUT_FIELD_ADDED'); + expect(change.e.message).toEqual( + "Input field 'e' of type 'String!' was added to input object type 'Foo'", + ); }); test('removed', async () => { const a = buildSchema(/* GraphQL */ ` From 8b077ce33978454225336316ad091bd61a526924 Mon Sep 17 00:00:00 2001 From: "jane.li" Date: Tue, 8 Apr 2025 17:03:17 +0000 Subject: [PATCH 3/3] add default value suffix in message and update tests. --- packages/core/__tests__/diff/input.test.ts | 2 +- packages/core/src/diff/changes/input.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/__tests__/diff/input.test.ts b/packages/core/__tests__/diff/input.test.ts index c6591522f9..6680b1094f 100644 --- a/packages/core/__tests__/diff/input.test.ts +++ b/packages/core/__tests__/diff/input.test.ts @@ -44,7 +44,7 @@ describe('input', () => { expect(change.e.criticality.level).toEqual(CriticalityLevel.Dangerous); expect(change.e.type).toEqual('INPUT_FIELD_ADDED'); expect(change.e.message).toEqual( - "Input field 'e' of type 'String!' was added to input object type 'Foo'", + "Input field 'e' of type 'String!' with a default value was added to input object type 'Foo'", ); }); test('removed', async () => { diff --git a/packages/core/src/diff/changes/input.ts b/packages/core/src/diff/changes/input.ts index 8809f34bf0..f90194b582 100644 --- a/packages/core/src/diff/changes/input.ts +++ b/packages/core/src/diff/changes/input.ts @@ -50,7 +50,8 @@ export function inputFieldRemoved( } export function buildInputFieldAddedMessage(args: InputFieldAddedChange['meta']) { - return `Input field '${args.addedInputFieldName}' of type '${args.addedInputFieldType}' was added to input object type '${args.inputName}'`; + const defaultValueSuffix = args.hasDefaultValue ? ' with a default value' : ''; + return `Input field '${args.addedInputFieldName}' of type '${args.addedInputFieldType}'${defaultValueSuffix} was added to input object type '${args.inputName}'`; } export function inputFieldAddedFromMeta(args: InputFieldAddedChange) {