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

reuse fragments by their selection set #437

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

knaeckeKami
Copy link
Collaborator

@knaeckeKami knaeckeKami commented Jan 7, 2024

This is just an idea for an alternative implementation to #417

  • Revert "[gql_code_builder] add dataClassConfig option to reuse class definitions for field selections that include only a single inline fragment spread"
  • simpler version of reuse fragments with less scope

This is a different approach to reusing fragments and reduce duplication in the code.

The idea is: Keep a Map of (TypeName ,BuiltSet<SelectionNode>) -> Reference for already generated fragments.

This will use the actual selections for figuring out whether a new class needs to be created or a fragment can be reused.

TODO: try if reusing generated subclasses for inline fragment spreads can also be optimized, without potential conflicts in types

TODO: potentially this can be extended to not only reuse fragments but all generated classes, if they have the same selections.

Drawbacks: the names of the generated classes are less stable and could change by adding new fragments which can be used

Advantage: IMO simpler to understand, easier to extend

@knaeckeKami knaeckeKami changed the title reuse classed by selection set reuse fragments by their selection set Jan 7, 2024
@LiLatee
Copy link
Contributor

LiLatee commented Jan 8, 2024

Hey again @knaeckeKami :D
I've tested it and for now, I am getting Out of memory error. I guess its because scope is much smaller than the previous solution and that's why.

And btw I had to manually modify fery_generator bcs the current version 0.8.2. expects dataClassConfig

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.

2 participants