Skip to content
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

[typescript] Non-null default fields are incorrectly optional #5112

Closed
jun-sheaf opened this issue Nov 21, 2020 · 5 comments
Closed

[typescript] Non-null default fields are incorrectly optional #5112

jun-sheaf opened this issue Nov 21, 2020 · 5 comments
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@jun-sheaf
Copy link
Contributor

Describe the bug

Default values are handled incorrectly.

To Reproduce
Steps to reproduce the behavior:
Suppose we have the schema

input SomeInput {
  test: String! = "test"
}

Then graphql-codegen will optionalize the argument test.

Expected behavior

The above should not be optional as String! is never null.

Environment:

  • OS: Mac OS Catalina
  • @graphql-codegen/...: Latest as of issue date
  • NodeJS: Latest as of issue date
@jun-sheaf jun-sheaf changed the title Non-null default fields are incorrectly optional [typescript] Non-null default fields are incorrectly optional Nov 21, 2020
@ardatan
Copy link
Collaborator

ardatan commented Nov 21, 2020

If you provide a default value for an input field, it means that input field is optional for the client side.
If test = null, test will be evaluated as "test" so this means test is nullable then optional for the client side.

@jun-sheaf
Copy link
Contributor Author

jun-sheaf commented Nov 21, 2020

Your reasoning is a little off. Test cannot be set to null on client side since test is a non-nullable type. In particular, it will error out, and not default to "test".

However, I understand what you mean. This issue is more about server implementations where the resolver should know the arguments are provided.

I've added a "defaultValue" option to the AvoidOptionalsConfig. Perhaps this will be a good compromise?

@ardatan
Copy link
Collaborator

ardatan commented Nov 21, 2020

I was talking about this behavior so in this case bar is optional there even if it is String!.
https://codesandbox.io/s/late-cache-npyqj?file=/src/index.js

@jun-sheaf
Copy link
Contributor Author

Yes, I am aware, but as I stated, bar is not actually null for the resolver. See here

Thus it would be better to have an avoidOptionals option for defaultValue for those who generate server types.

@dotansimha
Copy link
Owner

Merged and published. Thanks @jun-sheaf!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

3 participants