Adding 'vars_create_factories' option to create factory initializers with required parameters

[caffeineflo]

Hi 👋

We've been using Ferry for a while now and found it useful for our needs, so we thought we'll contribute back the improvements we made for it internally.

One of the things we found bothersome were the built_value Builders that generalize everything as optional e.g.:

Looking at the Query HeroForEpisode from the end_to_end_tests library here

query HeroForEpisode($ep: Episode!) {
  hero(episode: $ep) {
    name
    friends {
      name
    }
    ... on Droid {
      ...DroidFragment
    }
  }
}

The parameter ep is required, but the generated builder treats it as optional, allowing optional chaining like this:

GHeroForEpisodeVars((b) => b..ep = null?.GEpisode.EMPIRE); (The null is only for demonstration)

I know this is by design (coming from built_value) but unfortunately there doesn't seem to be an easy way around this ... until now!

We thought, why not generate create factories that correctly treat any parameters accordingly and just use the same builder function? Like this:

factory GHeroForEpisodeVars.create({required _i1.GEpisode ep}) =>
      GHeroForEpisodeVars((b) => b..ep = ep);

That way we can have the best of both worlds and the resulting object can be called with .toBuilder() in the builder assignment of the request (b..vars = Vars.create().toBuilder())

I've added this enhancement as an opt-in to the code builder as a config option - adding vars_create_factories: true to the VarsBuilder and tried to not touch anything else as to not break anything.

The generated code has been tested locally - if Unit Tests or end_to_end_tests are preferred, please let me know, I saw the previous TriState PR but didn't want to duplicate the whole e2e tests for that.

That said, I'll leave some comments in the code but would love to get some feedback and see wether or not this could be merged into gql and ultimately ferry (happy to surface the option on ferry_generator as well).

🥂

Note: I've seen that this issue came up before in these issues: https://github.com/gql-dart/ferry/issues/446 https://github.com/gql-dart/ferry/issues/386

And that others had the same issue with built_value https://github.com/google/built_value.dart/issues/1038 https://github.com/google/built_value.dart/issues/1050

Hopefully this is a way to get the best of both worlds

[LiLatee]

Nice! I missed that too! It would be nice to make required params as not-optional 😃

[knaeckeKami]

Hi! the proposal look good to me.

I didn't get to review the code yet, but please make sure that the CI succeeds. It seems this PR breaks the end_to_end_test_tristate repo, which is a copy of the end_to_end_test repo with the the TriStateValueConfig option set.

The generated code has been tested locally - if Unit Tests or end_to_end_tests are preferred, please let me know, I saw the previous TriState PR but didn't want to duplicate the whole e2e tests for that.

Yeah, the growing number of config options makes proper end to end tests harder, in the future this potentially needs to be refactored.

But since your PR only adds generated code, and does not change anything else, IMO it's fine to just add the flag to the two end_to_end_test repos and re-generate the code.

[caffeineflo]

@knaeckeKami Thanks for the reply!

I think I know why CI breaks, I’ll take a look!

I’m out until Jan 6th, so I’ll get back to this afterwards!

Happy Holidays

[knaeckeKami]

great! if you have any questions or issues, don't hesistate to ask here or in discord. I know this repo is not the easiest wo work with ;)

[caffeineflo]

@knaeckeKami Thanks!

I just pushed an update to the branch that should fix the failing CI test. Looks like it was only failing because the build_runners were generating files in the _tristate e2e tests in a different order which should now be fixed!

Let me know if you find anything else or if we can merge this and I can hover up the config up to the ferry_codegen package afterwards

[knaeckeKami]

Thanks!

I suggest to add this flag to the build.yaml of the end_to_end_test repo, to make sure that this feature works with the test suite and so we don't break it in the future.

Please also re-run code generation in the end_to_end_test repo afterwards .

[caffeineflo]

@knaeckeKami Found one more bug with the TriState e2e tests that should be good now! Other than that, this should be good to go

[knaeckeKami]

thanks, LGTM!