InputFieldAddedChange when adding a non-null type field with a default value is not a breaking change #2881
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: update cases. Reviewers: ureview JIRA Issues: RF-19036 Differential Revision: https://code.uberinternal.com/D17076739
clayne11
reviewed
Apr 5, 2025
| @@ -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); | |||
There was a problem hiding this comment.
Why is this dangerous? This seems simply non-breaking to me. What's the definition of dangerous vs critical?
There was a problem hiding this comment.
Dangerous is the original criticality level defined by core for this rule. So I would like to keep it as the same. For our internal rule, we will override it as non-breaking.
clayne11
reviewed
Apr 5, 2025
| 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'", |
There was a problem hiding this comment.
Should this message include include that is has a default?
Input field 'e' of type 'String!' with a default value was added to input object type 'Foo'
There was a problem hiding this comment.
ack. Updated the logic to handle this.
clayne11
approved these changes
Apr 8, 2025
|
Hi, @kamilkisiela Appreciate your eyes on this PR. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Updated InputFieldAddedChange logic to correctly handle non-null fields with default values. Previously, adding such fields was flagged as breaking; this change ensures they are recognized as not breaking.
Fixes # (#2880)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Updated input.test.ts to cover this change.
Provide instructions so we can reproduce.
Reproduce step is provided in reported issue.
Please also list any relevant details for your test configuration
Test Environment:
@graphql-inspector/...: