-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Support includeUnusedVariables option for HttpLink. #7127
Conversation
Since this option defaults to false, HttpLink will now default to stripping unused variables before sending GraphQL requests to the server, rather than blindly allowing unused variables in the request. Unused variables are likely to trigger server-side validation errors, according to https://spec.graphql.org/draft/#sec-All-Variables-Used, but this includeUnusedVariables option can be useful if your server deviates from the GraphQL specification by not strictly enforcing that rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @benjamn (and good call on providing the includeUnusedVariables
option, just in case). 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@benjamn we currently have a link to sign requests to AWS API Gateway. This change broke our app as the request is being modified at the httplink, after it was signed by our custom signer link, hence rendering the signature invalid. I tried moving the signer link to after the http link, but that obviously didnt work as it's a terminating link. We're currently working around this by being very careful to not include any extra variables in our queries, but the actual error response for bad signature from AWS breaks apollo client, as its not valid json. any suggestions apart from not using not-accepted variables? |
@jcane86 Have you tried passing |
Ah, I havent tried that yet. I'll give it a go and let you know if I find
any issues.
thanks for the prompt response!
…On Wed, 20 Jan 2021 at 01:36, Ben Newman ***@***.***> wrote:
@jcane86 <https://github.com/jcane86> Have you tried passing includeUnusedVariables:
true to the HttpLink constructor? This would involve creating the new
HttpLink yourself and passing it into the ApolloClient constructor via
the link option (which it sounds like you must already be doing, to
support the signing link).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7127 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFSHCW737PQCLO4HFENHGTS2WYI3ANCNFSM4SFHDO3A>
.
|
Unused variables are very likely to trigger server-side validation errors, thanks to the All Variables Used validation rule.
Since this rule renders unused variables all but useless in GraphQL requests, we (namely @hwillson and I) are increasingly convinced that
HttpLink
should strip unused variables by default, with an option to prevent the stripping in unusual scenarios where your server deviates from the GraphQL specification by not strictly enforcing validation rules.This automatic stripping of unused variables has the benefit of allowing developers to provide "too many" variables to their queries, knowing
HttpLink
will save them from needless validation errors, automatically sending only the variables that are used, while omitting any extras. It also enables the use of client-only variables for additional control ofread
andmerge
function behavior. Finally, it unlocks query transformations that might remove the last usage of a variable, since that removal no longer leads to validation errors if application code continues to provide a value for the unused variable.This could technically be a breaking change, but I think it's acceptable in a minor release (v3.3) because the new behavior affects only queries that were previously invalid, and it can be easily disabled using the
includeUnusedVariables
option: