Prioritize count error over null error in @oneOf coercion#1891
Conversation
spawnia
left a comment
There was a problem hiding this comment.
- Can you fix PHPStan?
- Does graphql-js also implement it this way and does it have tests that cover this?
When a client sends multiple fields for a @OneOf input (e.g. one non-null and one null), the "must specify exactly one field" error is more actionable than "field X must be non-null", which misleadingly suggests providing a value for the null field. This aligns Value::coerceInputValue with the existing behavior in OneOfInputObjectsRule, which already checks count before null.
0fac11b to
eb8b2b0
Compare
|
Yeah, looks like it. We try to follow those tests closely and reference them in PhpDoc. Might do an audit or more elegant rework at some point (PHP 8 Attributes maybe?, permalinks instead of plain names?). |
|
@spawnia ready for review |
|
Looking good. Feel free to merge and release. |
|
Somehow I feel reluctant releasing on this project. I think it's because of the release workflow. Do you have plans to automate this so that it becomes easier? On my own projects I don't use a changelog (which makes things easier) and I let GitHub generate the release notes. So then it's Merge, wait for CI to go green, create a new release, choose version, generate release notes, done. |
|
I opened #1893, we can discuss the solution there or in an adjacent PR. |
When a client sends multiple fields for a @OneOf input (e.g. one non-null and one null), the "must specify exactly one field" error is more actionable than "field X must be non-null", which misleadingly suggests providing a value for the null field.
This aligns Value::coerceInputValue with the existing behavior in OneOfInputObjectsRule, which already checks count before null.