Closed zholti closed 11 months ago
@benjaminjkraft should weigh in too, bit it seems like a good idea to me! Can you add the underscore to all vars, not justdata
?
@benjaminjkraft should weigh in too, bit it seems like a good idea to me! Can you add the underscore to all vars, not just
data
?
Did that for the local "err" as well. The collision with query/mutation args affects only this template as far as I can see. Which other vars in which templates can be affected?
I could see you passing in a var called client
to say which client to use. But I agree we don't have to handle it now if it's user facing
I could see you passing in a var called
client
to say which client to use. But I agree we don't have to handle it now if it's user facing
Passing a client is already there, no? Depending on Config.ClientGetter, either the one passed as arg or the local one will be constructed.
But you do have a point - no matter if the client is passed or constructed, an arg called "client" will collide with it in both cases.
As far as I can tell this is the only template that uses user-defined variable names. To be complete we should do the same for
req
andresp
. If you get a chance to do that that would be great. (In theory we should also doctx
andclient
, but since those are user-visible, I'd prefer if we can change them only if needed. And they're probably less likely names to conflict with, since context is a Go thing and how would you pass a client over GraphQL.)
In my latest commit, I renamed client to client_, since I can imagine that there might be schemas with fields taking such an arg. Should I also do the same treatment on ctx? This is less likely to collide with argument names, but still...
If we're gonna do client_
, we may as well do the same for ctx_
!
This PR makes THIS ISSUE a lot less likely to occur. The local name "data" in generated functions was colliding with arguments called "data".
Note that no explicit name collision check was added, but "data" inside the function was just renamed to "data_" to make it less likely to collide with arguments.
I have: