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

add filtering of arguments #20

Merged
merged 2 commits into from
Mar 28, 2024
Merged

Conversation

yaacovCR
Copy link
Contributor

@yaacovCR yaacovCR commented Feb 2, 2024

@smyrick
Copy link

smyrick commented Feb 5, 2024

This ties into this discussion: graphql/composite-schemas-wg#129

Pretending that @internal is a thing I think this should broadly allow you to hide fields or optional args from the client schema. So if there are any transforms library that want to understand or have support for that composition directive, they can use that to hide required args and provide the data somehow to subschemas at runtime too.

But what about a composition that hides a required arg from a subschema? Will this just error at at runtime or do allow this but flag it as a warning at composition time, or block composition?

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Feb 6, 2024

@smyrick i would think the directive that drops an argument would only be allowed if the argument is optional or if it is paired with another directive that states where the source for the argument should come from (something external, see the discussion there).

And so there are two phases of validation, one that just checks the above, and then another phase of validation that makes sure that the ordering that the subschema ordering that ends up being required in order to fulfill all those arguments is actually possible, i.e. that all of the subschema fields are practically reachable/executable.

@smyrick
Copy link

smyrick commented Feb 7, 2024

So then for the overview document this is actually a good change but we will need to clarify further in the other spec sections what "hiding an argument" entails. 👍🏻

@michaelstaib michaelstaib merged commit 0d15a0f into graphql:main Mar 28, 2024
2 of 3 checks passed
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.

3 participants