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

Do not add __typename to @client selection sets @export-ed as input variables. #4784

Merged

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented May 7, 2019

Fixes #4691.

@benjamn benjamn added this to the Release 2.6.0 milestone May 7, 2019
@benjamn benjamn requested a review from hwillson May 7, 2019 18:29
@benjamn benjamn self-assigned this May 7, 2019
@benjamn
Copy link
Member Author

benjamn commented May 7, 2019

@OurMajesty @supercranky Does the test included in this commit capture your use case(s) from #4691?

@benjamn benjamn changed the title Do not add __typename to @export-ed input variables. Do not add __typename to selection sets @export-ed as input variables. May 7, 2019
if (
isField(field) &&
field.directives &&
field.directives.some(d => d.name.value === 'export')
Copy link
Member Author

Choose a reason for hiding this comment

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

@hwillson Should this test be stricter? For example, we could require that the directive has an as argument that refers to a declared variable.

Copy link
Member

Choose a reason for hiding this comment

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

@benjamn I think it's okay like this. If we need to change this in the future, we should be able to pretty easily.

Copy link

Choose a reason for hiding this comment

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

This does not work in the case where the field contains nested fields:

filters @client @export(as: "filters") {
  priceFilters {
    value
  }

The typename will be removed from filters, but not from priceFilters.

I have been using this workaround, but it seems undesirable:

filters @client @export(as: "filters") {
  priceFilters @export {
    value
  }

@benjamn benjamn changed the title Do not add __typename to selection sets @export-ed as input variables. Do not add __typename to @client selection sets @export-ed as input variables. May 7, 2019
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Looks great @benjamn - thanks!

@benjamn benjamn merged commit 9c06ba8 into release-2.6.0 May 7, 2019
@benjamn benjamn deleted the issue-4691-avoid-__typename-for-input-variables branch May 7, 2019 19:35
@OurMajesty
Copy link

@OurMajesty @supercranky Does the test included in this commit capture your use case(s) from #4691?

Here's the exact request from the real application that caused the error:

#import 'graphql/fragments/Attachment'
#import 'graphql/fragments/Product'

query GetProducts($filter: ProductFilter, $limit: Int!, $offset: Int!) {
    currentFilter @client @export(as: "filter") {
        active
        recursiveCategoryId
        state {
            in
        }
        valid
    }
    productList(filter: $filter, limit: $limit, offset: $offset) {
        totalCount
        resultList {
            ...ProductFields
            client {
                id
                inn
                kpp
                name
                clientName @client
            }
            images {
                ...AttachmentFields
            }
        }
    }
}

I don't think it have important differences, but maybe it's helpful.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants