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

Fix duplicate fields in preloads #1161

Closed
wants to merge 1 commit into from
Closed

Fix duplicate fields in preloads #1161

wants to merge 1 commit into from

Conversation

nitin302
Copy link

@nitin302 nitin302 commented Apr 21, 2020

This is a documentation change, where we are looping over Selections Twice, which is causing duplicates. I've removed one nested preload to removed these duplicates.

@vektah
Copy link
Collaborator

vektah commented Apr 24, 2020

Thoughts @RichardLindhout ?

@RichardLindhout
Copy link
Contributor

There are cases where this is needed I think with fragments, maybe we need to remove duplicates but there was a case when something was inside fragments we need this. I think this issue was posted here. Maybe this is resolved now with newer versions?

Schermafbeelding 2020-04-24 om 11 06 52

My issue back then:
#954

More on that issue here:
#746

Maybe we could do a function where we only append if not exist yet. I did not see this problem before. Do you have a query which result in duplicates?

I generate type-safe preloads here:
https://github.com/web-ridge/gqlgen-sqlboiler/blob/master/examples/social-network/helpers/preload.go

Latest helpers:
https://github.com/web-ridge/gqlgen-sqlboiler/blob/master/helper/preload.go

@RichardLindhout
Copy link
Contributor

RichardLindhout commented Apr 24, 2020

I think if you don't include it, selections won't resolve nested fragment fields. However removing duplicates would be a good idea.

@nitin302
Copy link
Author

nitin302 commented Apr 24, 2020

query Search($input: NewSearch!) {
  search(input: $input) {
    chats {
      id
      lastMessage
      messagestats {
        noOfMessages
      }
    }
  }
}

This returns an array of Chats in a Search Response.

I was seeing duplicates, when I fetched the fields, as shown in documentation. Every nested field was duplicated.

I am not using any fragments, Maybe there no duplicates when using fragments. However, for simple queries, it causes duplicates, maybe you can remove duplicates (or insert in to the array if it is not present array).

@RichardLindhout
Copy link
Contributor

RichardLindhout commented Apr 24, 2020

Don't have time to test a.t.m.
@vektah Has something changed in the SelectionSet lately or in the new parser?

Maybe this instead of appending

func appendPreloadIfMissing(preloads []string, preloadToAdd string) []string {
    for _, preload := range preloads {
        if preload == preloadToAdd {
            return slice
        }
    }
    return append(preloads, preloadToAdd)
}

@RichardLindhout
Copy link
Contributor

Field are indeed duplicated but for me only column.Selections returned the right fields for me with fragments. SelectionSet did not

@RichardLindhout
Copy link
Contributor

This should fix it: #1189

@lwc lwc closed this in #1189 Jun 11, 2020
@nitin302 nitin302 deleted the patch-1 branch June 11, 2020 10:16
cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
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