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

Update eagerly fetching section #2147

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

theverything
Copy link

@theverything theverything commented May 4, 2022

This fixes the docs so that the generate command will correctly generate the User resolver.

Discussions about this can be found here #2146 and here #2144

I have:

  • Updated any relevant documentation (see docs)

This fixes the docs so that the generate command will correctly generate the User resolver.
@frederikhors
Copy link
Collaborator

This is not a solution. Please read carefully the docs and other issues and questions.

@frederikhors
Copy link
Collaborator

This is not an issue at all.

@theverything
Copy link
Author

@frederikhors I'm not sure what you mean. The docs in the https://gqlgen.com/getting-started are incorrect, specifically the https://gqlgen.com/getting-started/#dont-eagerly-fetch-the-user. If you compare code snippets to the code in the completed tutorial here https://github.com/vektah/gqlgen-tutorials/tree/master/gettingstarted you can see that they are clearly different.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 74.628% when pulling 47ab3e1 on theverything:fix-getting-started-docs into 3228f36 on 99designs:master.

@StevenACoffman
Copy link
Collaborator

Can you compare your fix with #2157 and see if it addresses your concern fully?

@theverything
Copy link
Author

@StevenACoffman it does not. The PR does not address the issue properly nor does it address all of the issues.

Adding the resolver section to the gqlgen config is not necessary because once you update the Todo struct with the UserID instead of the User field, gqlgen will create the resolver for you.

This is not needed.

 Todo:
    fields:
      user:
        resolver: true

Its also missing the update the the CreateTodo function that is included in my PR.

@frederikhors
Copy link
Collaborator

@theverything, in gqlgen there are two different ways to do this. And they are both in docs.

@theverything
Copy link
Author

@frederikhors @StevenACoffman After the user finishes the the tutorial their code should look exactly like the code in the link referenced in the tutorial itself. Found here

https://github.com/99designs/gqlgen/blob/master/docs/content/getting-started.md?plain=1#L15

@frederikhors I understand there is multiple ways to create a resolver but why have the user use both solutions when only one is necessary

@scottopherson
Copy link

I was confused by the getting-started eager-fetching section as well. I came upon this PR while searching around to see if others were confused as well.

While searching around, this issue comment got me to look closer at the github README and see the explanation about the two different eager-fetching solutions. I haven't found this info yet on https://gqlgen.com docs. That particular README section and the link to the completed tutorial code cleared up this topic for me.

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.

5 participants