Skip to content

Conversation

UnAfraid
Copy link
Collaborator

@UnAfraid UnAfraid commented Jul 25, 2025

Added new function processArgField that can be reused for every field argument parsing instead of generating one for each field
(Whenever possible, in the case where directives are involved - falls back the old way)

I've tested this on pretty big schema (over 400 types/inputs) and it it results in -20k lines of code

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@coveralls
Copy link

Coverage Status

coverage: 73.167%. remained the same
when pulling 461189d on UnAfraid:feature/reuse-arg-process-func
into a245e6f on 99designs:master.

@UnAfraid UnAfraid changed the title Feature/reuse arg process func Reuse argument parsing function Jul 25, 2025
@StevenACoffman
Copy link
Collaborator

@UnAfraid This is a massive improvement! Thank you so much!

I noticed that the only conditional template difference between the two variations is due two is whether or not it is using $useFunctionSyntaxForExecutionContext :
codegen/generated!.gotpl

What do you think about changing the name of the ExecutionContext variant and exporting both variants so they don't need to be part of the template code anymore at all?

Screenshot 2025-07-26 at 12 43 08 PM

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Jul 26, 2025

@UnAfraid I'm going to merge this PR as it is, since it's already a huge improvement, but I would like to move as much logic out of templates as possible. If you are willing or interested in another PR to do this I would be hugely appreciative!

The code in gqlgen that is in a template receives significantly fewer contributions than the traditional library code. I believe it's primarily due to the difficulty for most developers (and tools) to understand and manipulate it.

If these functions were just exported functions in a library, we could unit test them, and they wouldn't be intimidating for any developer to help us improve. What do you think?

@StevenACoffman StevenACoffman merged commit d15f824 into 99designs:master Jul 26, 2025
18 checks passed
@UnAfraid UnAfraid deleted the feature/reuse-arg-process-func branch July 27, 2025 00:08
@UnAfraid
Copy link
Collaborator Author

@UnAfraid This is a massive improvement! Thank you so much!

I noticed that the only conditional template difference between the two variations is due two is whether or not it is using $useFunctionSyntaxForExecutionContext : codegen/generated!.gotpl

What do you think about changing the name of the ExecutionContext variant and exporting both variants so they don't need to be part of the template code anymore at all?

Screenshot 2025-07-26 at 12 43 08 PM

Yeah i am interested in moving more of the generated code to reusable functions, where would be a good place/package to place those in?

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Jul 27, 2025

@UnAfraid Thank you for volunteering! The easiest place to put these particular reusable functions is in "github.com/99designs/gqlgen/graphql" as all of the code that gets generated using those functions already at least imports:

	"github.com/99designs/gqlgen/graphql"
	"github.com/99designs/gqlgen/graphql/introspection"
	gqlparser "github.com/vektah/gqlparser/v2"
	"github.com/vektah/gqlparser/v2/ast"

Unless there's an import cycle, or something seems like it needs to belong elsewhere, I haven't been getting any complaints concerned with there being too much in that package, and reorganizing it is easier than moving things out of templates.

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