feat: deduplicate Child field context switches and scalar fieldContex…#4086
feat: deduplicate Child field context switches and scalar fieldContex…#4086syssam wants to merge 6 commits into99designs:masterfrom
Conversation
3c6cc35 to
086b1bf
Compare
…t boilerplate Reduces generated code by ~23% on large schemas with zero runtime performance impact. Two optimizations: 1. Shared childFields_* functions: Extract the Child switch into a shared function generated once per return type, instead of inlining the same switch in every fieldContext_* function. 2. newLeafFieldContext helper: Replace 11-line boilerplate for scalar field context functions with a 3-line helper call. Tested on a production schema with 325 entities (~22,000 fieldContext functions): - Before: 2,308,013 lines - After: 1,778,987 lines (-529,026 lines, -22.9%) Closes 99designs#4085
086b1bf to
8323543
Compare
|
@StevenACoffman any feedback on this pr? |
|
@syssam I keep thinking you are still in progress on this, since when I run the checks on it, it does not fully pass everything (like fmt-and-lint). I would prefer to minimize logic in *.gotpl files as it is much more difficult to test and maintain than code in traditional *.go files. Your PR adds more new *.gotpl logic which makes me hesitate a bit, even though it does provide the other benefits you mentioned. |
…egenerate - Move newLeafFieldContext from .gotpl templates into graphql.NewScalarFieldContext() to reduce template logic and improve testability - Add unit tests for NewScalarFieldContext, UniqueChildFieldTypes, ChildFieldContextTypeName - Add integration tests and go:generate directive for childfielddedup testserver - Regenerate all files after merge from master (fixes check-generate CI failure) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@StevenACoffman The All checks verified locally with both Go 1.25 and Go 1.26:
Ready for re-review when CI runs are approved. |
|
@StevenACoffman what do you think new version? |
| dir: . | ||
| package: childfielddedup | ||
| model: | ||
| filename: models-gen.go |
There was a problem hiding this comment.
Not your fault, or the job of this PR, but I'm just now noticing that the -gen in the name models-gen.go (copied all over from ancient days) is oddly inconsistent with our more usual convention of using .generated.
codegen/testserver/benchmark/gqlgen.yml
codegen/testserver/nullabledirectives/gqlgen.yml
codegen/testserver/followschema/gqlgen.yml
codegen/testserver/singlefile/gqlgen.yml
codegen/testserver/generated_test.go
codegen/testserver/usefunctionsyntaxforexecutioncontext/gqlgen.yml
…t boilerplate
Reduces generated code by ~23% on large schemas with zero runtime performance impact.
Three optimizations:
Shared childFields_* functions: Extract the Child switch into a shared function generated once per return type in root_.generated.go, instead of inlining the same switch in every fieldContext_* function.
newLeafFieldContext helper: Replace 11-line boilerplate for scalar field context functions with a 3-line helper call.
Move root resolver interfaces (QueryResolver, MutationResolver, SubscriptionResolver) to root_.generated.go in follow-schema layout, instead of placing them in arbitrary per-schema files.
Tested on a production schema with 325 entities (~22,000 fieldContext functions):
Describe your PR and link to any relevant issues.
I have: