Prevent merging of fragment fields into selection tree if fragment field merging is disabled #571
+47
−14
Conversation
This file contains 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
✅ Docs Preview ReadyConfiguration{
"repoOverrides": {
"apollographql/apollo-ios-dev@main": {
"remote": {
"owner": "apollographql",
"repo": "apollo-ios-dev",
"branch": "fragment-field-merging-disabled-performance"
}
}
}
}
2 pages published. Build will be available for 30 days. |
✅ Deploy Preview for eclectic-pie-88a2ba ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for apollo-ios-docc canceled.
|
This was referenced Jan 6, 2025
|
Although this does allow codegen to complete for me, I'm not sure if this is a good idea: |
|
I'm not sure that issue is related to this feature. It could be, but I'd like to get more information from you in that thread @pm-dev. |
calvincestari
approved these changes
Jan 21, 2025
BobaFetters
approved these changes
Jan 21, 2025
BobaFetters
pushed a commit
that referenced
this pull request
Jan 21, 2025
…eld merging is disabled (#571)
BobaFetters
pushed a commit
to apollographql/apollo-ios-codegen
that referenced
this pull request
Jan 21, 2025
…eld merging is disabled (apollographql/apollo-ios-dev#571)
BobaFetters
pushed a commit
that referenced
this pull request
Jan 21, 2025
4bc820e4 Prevent merging of fragment fields into selection tree if fragment field merging is disabled (#571) git-subtree-dir: apollo-ios-codegen git-subtree-split: 4bc820e4c3b1fa53992328d84b6c2b684e28ba2b
BobaFetters
pushed a commit
that referenced
this pull request
Jan 21, 2025
…lds into selection tree if fragment field merging is disabled git-subtree-dir: apollo-ios-codegen git-subtree-mainline: b5a5e92 git-subtree-split: 4bc820e4c3b1fa53992328d84b6c2b684e28ba2b
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.
Currently, when fragment field merging is disabled we are preventing the fields from being merged into the
MergedSelectionswhile traversing the builtEntitySelectionTree. However, the performance issues described in apollographql/apollo-ios#3434 are actually caused during the merging of the fragment fields into theEntitySelectionTrees during the building of the operations.This PR prevents the fragment selection trees from being merged into the
EntitySelectionTreewhile building the operations if fragment field merging is disabled.This means that the entity trees are actually incomplete, but they still work for the purposes of Codegen in this scenario. Incomplete
EntitySelectionTrees would cause selection set initializers to be invalid. But this is not a problem because disabling fragment field merging is already incompatible with selection set initializers (and we have a validation step to ensure those options are not used together).