allow non-nullable first and last arguments in relay-connection-arguments-spec #203
Conversation
This file contains 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
cjoudrey
reviewed
Jan 21, 2020
README.md
Outdated
| @@ -236,6 +236,8 @@ More specifically: | |||
| - To enable forward pagination, two arguments are required: `first: Int` and `after: *`. | |||
| - To enable backward pagination, two arguments are required: `last: Int` and `before: *`. | |||
|
|
|||
| Note: If only forward pagination is enabled, the `first` argument be specified as non-nullable (i.e., `Int!` instead of `Int`). Similarly, if only backward pagination is enabled, the `last` argument can be specified as non-nullable. | |||
There was a problem hiding this comment.
Suggested change
| Note: If only forward pagination is enabled, the `first` argument be specified as non-nullable (i.e., `Int!` instead of `Int`). Similarly, if only backward pagination is enabled, the `last` argument can be specified as non-nullable. | |
| Note: If only forward pagination is enabled, the `first` argument can be specified as non-nullable (i.e., `Int!` instead of `Int`). Similarly, if only backward pagination is enabled, the `last` argument can be specified as non-nullable. |
| const type = isFirstArgumentNullable | ||
| ? firstArgument.type | ||
| : firstArgument.type.type; | ||
| if (type.kind != 'NamedType' || type.name.value != 'Int') { |
There was a problem hiding this comment.
I think you can simplify this entire block by using the unwrapType function that is defined at the top of this file. 😄
const type = unwrapType(firstArgument);| lastArgument.type.kind !== 'NonNullType'; | ||
| const type = isLastArgumentNullable | ||
| ? lastArgument.type | ||
| : lastArgument.type.type; |
| @@ -59,7 +59,7 @@ describe('RelayConnectionArgumentsSpec rule', () => { | |||
| ); | |||
| }); | |||
|
|
|||
| it('accepts connection with proper foward and backward pagination arguments', () => { | |||
| it('accepts connection with proper forward and backward pagination arguments', () => { | |||
There was a problem hiding this comment.
Ha! Nice catch! Thanks for the fix.
| @@ -122,7 +206,7 @@ describe('RelayConnectionArgumentsSpec rule', () => { | |||
| }, | |||
| ], | |||
| message: | |||
| 'Fields that support forward pagination must include a `last` argument that takes a non-negative integer as per the Relay spec.', | |||
| 'Fields that support backward pagination must include a `last` argument that takes a non-negative integer as per the Relay spec.', | |||
There was a problem hiding this comment.
Oops! 🤦♂ Thanks for catching this too. That previous message would be super confusing. 😂
|
Thanks for the review @cjoudrey! I've updated accordingly 😄 |
cjoudrey
approved these changes
Jan 27, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
This addresses #148.
The
firstargument type in a Relay connection can now be non-nullable if backward pagination is disabled (likewise forlastwhen forward pagination is disabled). So the following types are now permitted:The following, however, are not permitted: