Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent merging of fragment fields into selection tree if fragment field merging is disabled #571

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AnthonyMDev
Copy link
Contributor

Currently, when fragment field merging is disabled we are preventing the fields from being merged into the MergedSelections while traversing the built EntitySelectionTree. However, the performance issues described in apollographql/apollo-ios#3434 are actually caused during the merging of the fragment fields into the EntitySelectionTrees during the building of the operations.

This PR prevents the fragment selection trees from being merged into the EntitySelectionTree while 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).

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Jan 6, 2025

✅ Docs Preview Ready

Configuration
{
  "repoOverrides": {
    "apollographql/apollo-ios-dev@main": {
      "remote": {
        "owner": "apollographql",
        "repo": "apollo-ios-dev",
        "branch": "fragment-field-merging-disabled-performance"
      }
    }
  }
}
Name Link

Commit

4e76056

Preview

https://www.apollographql.com/docs/deploy-preview/06003a689142e0e3bbec

Build ID

06003a689142e0e3bbec

File Changes

0 new, 2 changed, 0 removed
* (developer-tools)/ios/(latest)/get-started.mdx
* (developer-tools)/ios/(latest)/index.mdx

Pages

2


2 pages published. Build will be available for 30 days.

Copy link

netlify bot commented Jan 6, 2025

Deploy Preview for eclectic-pie-88a2ba ready!

Name Link
🔨 Latest commit 4e76056
🔍 Latest deploy log https://app.netlify.com/sites/eclectic-pie-88a2ba/deploys/677c49199ec6a00008faa8ed
😎 Deploy Preview https://deploy-preview-571--eclectic-pie-88a2ba.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 6, 2025

Deploy Preview for apollo-ios-docc canceled.

Name Link
🔨 Latest commit 4e76056
🔍 Latest deploy log https://app.netlify.com/sites/apollo-ios-docc/deploys/677c4919a175cf0008dab5ca

@pm-dev
Copy link

pm-dev commented Jan 7, 2025

Although this does allow codegen to complete for me, I'm not sure if this is a good idea:
apollographql/apollo-ios#3496

@AnthonyMDev
Copy link
Contributor Author

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.

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