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

Improve validation error message when field names conflict #363

Merged
merged 2 commits into from
Apr 26, 2016
Merged

Improve validation error message when field names conflict #363

merged 2 commits into from
Apr 26, 2016

Conversation

robzhu
Copy link
Contributor

@robzhu robzhu commented Apr 21, 2016

When I submit a query with overlapping field names like so:

node {
echo(val: "test"),
echo(val: "foo")
}

I get the following validation error:
"Fields "echo" conflict because they have differing arguments."

This PR modifies the error message to suggest using aliases when this occurs:
"Fields "echo" conflict because they have differing arguments. Use aliases on the fields to fetch both if this was intentional."

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling aa72089 on robzhu:master into * on graphql:master*.

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@yuzhi
Copy link
Contributor

yuzhi commented Apr 25, 2016

If the reason is an array, Use aliases on the fields to fetch both if this was intentional. is going to be printed a lot of times. Is that intentional?

@leebyron
Copy link
Contributor

leebyron commented Apr 25, 2016

Agree with @yuzhi - can you take a look at some of the output messages?

I patched this in to test. For example https://github.com/graphql/graphql-js/blob/master/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js#L308 now produces:

Fields "field" conflict because subfields "deepField" conflict because subfields "x" conflict because a and b are different fields. Use aliases on the fields to fetch both if this was intentional.. Use aliases on the fields to fetch both if this was intentional.. Use aliases on the fields to fetch both if this was intentional.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 7d2f191 on robzhu:master into * on graphql:master*.

@leebyron
Copy link
Contributor

Ace

@leebyron leebyron merged commit 6103d2d into graphql:master Apr 26, 2016
@robzhu
Copy link
Contributor Author

robzhu commented Apr 26, 2016

Thanks for the quick review and merge!

sogko added a commit to sogko/graphql that referenced this pull request Jun 1, 2016
graphql/graphql-js#363

* Improve validation error message when field names conflict

* Remove extra hint and add unit test
chris-ramon pushed a commit to graphql-go/graphql that referenced this pull request Mar 15, 2017
graphql/graphql-js#363

* Improve validation error message when field names conflict

* Remove extra hint and add unit test
samuel pushed a commit to sprucehealth/graphql that referenced this pull request Mar 28, 2017
graphql/graphql-js#363

* Improve validation error message when field names conflict

* Remove extra hint and add unit test
mattstern31 pushed a commit to mattstern31/graphql-gqllero-repository that referenced this pull request Nov 10, 2022
graphql/graphql-js#363

* Improve validation error message when field names conflict

* Remove extra hint and add unit test
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.

5 participants