Skip to content

388 fix optional argument handling #404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 21, 2020

Conversation

vojtapol
Copy link
Member

@vojtapol vojtapol commented May 21, 2020

Resolves #388

Checklist

  • Pull requests follows the contribution guide
  • New or modified functionality is covered by tests

Description

Changes the default behavior of Optional mapping when used as an input parameter.
This is an adjustment to my previous PR on this topic #310.

Both options are now supported and the behavior is configurable via the boolean switch inputArgumentOptionalDetectOmission:

  • when true an omitted GraphQL argument during query is converted to null and an explicit GraphQL null is converted to Optional.empty() (294 handle optional parameters #310)
  • when false both an omitted GraphQL argument and explicit null is converted to Optional.empty(); therefore the value is never null; this is the new default!

I am choosing to make this a breaking change and make the second option the default for these reasons:

  1. Omission detection never worked in input objects which was confusing.
  2. Omitting arguments is not well supported by various client libraries such as Apollo. It is hard to store reusable query templates when omitting arguments.
  3. The expectation is that an Optional is never null.

The old behavior will be kept for those users that do not care about the reasons above and want to continue using it. But they will have to opt-in to it by setting inputArgumentOptionalDetectOmission to true.

@vojtapol vojtapol merged commit a004243 into master May 21, 2020
@oryan-block oryan-block deleted the 388-fix-optional-argument-handling branch September 6, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Best practices for handling optional arguments?
2 participants