Open dehypnosis opened 1 year ago
Thanks, I'll have a proper look later.
One thing that i noticed on the first glance, you seem to have implemented another way of calculating the possibleTypes map.
Would it be feasible to leverage https://github.com/gql-dart/gql/blob/master/codegen/gql_code_builder/lib/src/utils/possible_types.dart instead ?
sorry for the late response! please pull the latest changes from master to ci succeeds
@knaeckeKami how are you doing well? just merged upstream/master branch.
Thanks! It seems there are now difference in the generated code in end_to_end_test, despite the reuse_fragments flag being disabled. I'll look into why this is.
@knaeckeKami oh, sorry for unexpected trouble. Is that related to WhenExtension ?
Not sure yet, didn't have a proper look, but you can check the diff in the CI output: https://github.com/gql-dart/gql/actions/runs/6485343593/job/17611211975?pr=417
Thanks!
A user reported some issues with unions with a lot of possible values, see here:
https://github.com/LiLatee/flutter_ferry_unions_issue
I noticed that the DFS searches in all permutations of the given union types, which leads to exponential code generation time .
This is a possible fix:
https://github.com/gql-dart/gql/tree/possible_reuse_fragment_fix
Please add a test that checks if this bug is fixed by this PR
Hi @knaeckeKami @dehypnosis! 😄
I've prepared a repo with an example that generates a code with errors. In short, overridden fields have a wrong return type. I think that the problem exists because I have two different fragments defined on a single type defined in schema. In my case I have two fragments::
ActivityB
and ActivityBWithoutSharedItem
defined on type ActivityB
.
Is there a quick solution for that? 🤔
I've spent a few more hours and created an even much simpler example to reproduce the issue and I really cannot understand what's the problem here.
I've found three solutions to fix the issue by modifying fragments:
Remove import. Just import (even without using it) causes the error.
Remove one fragment and use its content directly. Removing ItemEdge
here and pasting that fragment's content fixes the issue
Or removing one union field activityData
Each of these modifications fixes the issue and I need to use just one of them.
Is there a chance that you could take a look at that in the near future? 🙏
Hi!
I also looked a bit into it. I think the issue even occurs in the end_to_end_test in this query:
hero_with_interface_subtyped_fragments.graphql
.
query HeroWithInterfaceSubTypedFragments($episode: Episode!) {
hero(episode: $episode) {
...heroFieldsFragment
}
}
fragment heroFieldsFragment on Character {
id
name
...on Human {
...humanFieldsFragment
}
...on Droid {
...droidFieldsFragment
}
}
fragment humanFieldsFragment on Human {
homePlanet
friends {
...on Droid {
id
name
...droidFieldsFragment
}
...on Human {
id
name
homePlanet
}
}
}
fragment droidFieldsFragment on Droid {
primaryFunction
}
And it seems to be triggered by Inline fragment spreads with typeconditon where the selection is just one fragment spread.
But I don't know much more yet.
I now understand what the issue is.
I found a way to fix it in gql_code_builder, but not sure if this breaks smth else.
@LiLatee can you try
dependency_overrides:
gql_code_builder:
git:
url: https://github.com/gql-dart/gql/tree/repro_reuse_fragments
ref: repro_reuse_fragments
path: codegen/gql_code_builder
and give me feedback?
@knaeckeKami Unfortunately still the same issue on the example repository 😞
btw I guess that's a proper dependency overrides
gql_code_builder:
git:
ref: repro_reuse_fragments
url: https://github.com/gql-dart/gql.git
path: codegen/gql_code_builder
yes, thanks for fixing the broken link.
Sorry. I did not push all the code. please try again.
I am getting other errors right now
I can add Data
to the name, but not sure if it's fine.
I don't have time right now, but I can check it more deeply tomorrow
Ok, I had a suspicion that it would be not that easy ;)
Would appreciate if you could create failing reproduction to the end_to_end_test repo.
But I know now what the issue is, and I think I can find a fix in the near future.
Would appreciate if you could create failing reproduction to the end_to_end_test repo.
Sure, I can try to take care of it tomorrow 👌
But I know now what the issue is, it's only and I think I can find a fix in the near future.
Would be awesome 🤞
Would appreciate if you could create failing reproduction to the end_to_end_test repo.
@knaeckeKami should I create a failing case for the code before you commented that condition above?
Because your comment removes some part of fragment reusing. E.g. before that change my user.data.gql.dart
file had 212 lines and now it has 712, because new fragments (that already exist) are created again.
Failing in the sense that it's more code or that the code doesn't compile?
Anyway, yes please share.
I know that this might increase the code size but this optimization is not correct in the way it is now. But maybe we can find I way to make it work again.
Sorry guys, I'm busy doing my day works nowadays. I will dive into to take a look in this weekend if the issue is not resolved until then.
@knaeckeKami oh I've finally found one issue. It looks like the order of the imports in .graphql files is important. e.g. Case 1:
Case 2:
This example uses repro_reuse_fragments version (commit 6ae9dfaddc4e417afaf3eea59bc8709ee403745e)
thanks!
@LiLatee implemented another approach to reuse fragment classes in #437 . this is just an experiment at this point, but feel free to try it out.
Hi @knaeckeKami This PR is a succession of https://github.com/gql-dart/gql/pull/413 After the above PR, our team have developed our ongoing project with dev release version which includes above PR. And we made a small improvement and a bugfix to make the reuse-fragment feature stable. So here is a PR to finalize this feature.
Changes
ferry_generator:graphql_builder.data_class_config.reuse_fragments
option enabled.