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

Compiler: fix validation of required fields with default values #2903

Closed
wants to merge 1 commit into from
Closed

Compiler: fix validation of required fields with default values #2903

wants to merge 1 commit into from

Conversation

mrtnzlml
Copy link
Contributor

Previously, Relay Compiler threw an error incorrectly on fields with required values AND default values defined in the schema. This change takes the default values into account so that you don't have to specify them in the operation/fragment definition.

Closes: #2894

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tyao1 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one change

Previously, Relay Compiler threw an error incorrectly on fields with required values AND default values defined in the schema. This change takes the default values into account so that you don't have to specify them in the operation/fragment definition.

Closes: #2894
@mrtnzlml mrtnzlml requested a review from josephsavona October 24, 2019 04:26
Copy link
Contributor

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@mrtnzlml
Copy link
Contributor Author

Hi @tyao1 - would you please consider importing it again? I fixed the code review comments.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tyao1 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@tyao1 merged this pull request in fedb19c.

@alunyov
Copy link
Contributor

alunyov commented Oct 31, 2019

@mrtnzlml thank you for fixing this! I'll try to release 7.0.1 soon

@mrtnzlml mrtnzlml deleted the compiler-required-args-with-defaults-fix branch October 31, 2019 19:20
jstejada pushed a commit that referenced this pull request Dec 19, 2019
Summary:
Previously, Relay Compiler threw an error incorrectly on fields with required values AND default values defined in the schema. This change takes the default values into account so that you don't have to specify them in the operation/fragment definition.

Closes: #2894
Pull Request resolved: #2903

Reviewed By: josephsavona

Differential Revision: D18109039

Pulled By: tyao1

fbshipit-source-id: c801d2eb2c3e13cc6d879ce44b6dd158f2b25aa3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7.0.0 bug: required argument 'ABC' is missing on 'XYZ'
4 participants