Repository navigation
Collect fields before generating operation classes - #144
Merged
Merged
Conversation
FieldCollector merges the fields of each operation per response path and concrete object type, resolving fragments itself. OperationGenerator walks that model without traversal state, adding __typename during emission. OperationStack and the duplicate check in ObjectLikeBuilder are gone. FoldFragments and AddTypename now only shape the document sent to the server. 🤖 Generated with Claude Code
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Introspection fields can break code generation, and document mutation defeats the intended source/wire separation.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Refactors operation code generation to collect merged fields before generating typed classes, replacing traversal state.
Changes:
- Adds
FieldCollector,Selection, andCollectedField. - Reworks operation/result class generation around collected selections.
- Separates source and wire document parameters.
| File | Description |
|---|---|
src/Codegen/Selection.php |
Models fields and nested selections. |
src/Codegen/CollectedField.php |
Represents a collected response field. |
src/Codegen/FieldCollector.php |
Collects and merges operation fields. |
src/Codegen/OperationGenerator.php |
Generates classes from collected selections. |
src/Codegen/Generator.php |
Supplies source and wire documents. |
src/Codegen/ObjectLikeBuilder.php |
Removes builder-level deduplication. |
src/Codegen/OperationStack.php |
Removes obsolete traversal state. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
FoldFragments and AddTypename modify nodes in place, so codegen read the folded document. 🤖 Generated with Claude Code
🤖 Generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OperationGeneratormerged fields while walking the document, kept consistent by a namespace stack and builder deduplication.This collects each operation into a merged model first, then generates classes from it without traversal state.
Generated code stays byte-identical
examples/*/expectedis unchanged.Old and new code also produced identical output for a schema covering merged fields across exclusive parents, covariant interface fields, aliased
__typenameand repeated fragment spreads.graphql-php has no CollectFields that codegen can reuse
ReferenceExecutor::collectFieldsis protected, works on one runtime type and needs runtime variables.OverlappingFieldsCanBeMerged::getFieldsAndFragmentNamesis protected and groups by static parent type.FoldFragments and AddTypename now only shape the document sent to the server
Codegen reads the document as written and adds
__typenameproperties itself.#145 removes
FoldFragments.Existing quirks are kept on purpose
Child classes are keyed by response path only, so
... on A { x { foo } } ... on B { x { bar } }merges into one class.For covariant interface fields, the type of the first occurrence wins.
Each is now a local change in
FieldCollector, but changes output.Conditional fields from https://github.com//pull/79 need one flag per field
collectFieldscan pass a$conditionalflag down, and eachCollectedFieldcan OR in! $conditional.That marks fields that are always present, without counting selections.
🤖 Generated with Claude Code