Use embedded graphql strings instead of seperate .graphql files

[yosef-abraham]

Hey guys,
as I discussed in the Feedback issue (#1) I suggested a better (IMO) way of structuring the widgets and data requirements based on the design of react-relay.

I will try to lay here the way I think it should work.

GQLClientProvider

In relay, there is one object that manages the dispatching and processing of queries, subscriptions, mutations etc. It quite naturally translates to a GQLClientProvider here that all widgets in the tree will know to reference when wanting to perform some graphql operation.

  @override
  Widget build(BuildContext context) {
    return GQLClientProvider(
      client: _buildClient();
      child: MaterialApp(...)
    );
  }

Queries widgets, fragment widgets

The strength of relay, as stated in numerous lectures online, is its ability to localize and couple components and their data requirements. That way, it is less likely to add/remove component behaviour while forgetting to adjust the data requirements accordingly.

@query(r"""
query FetchArticle($articleId: ID!) {
  article($articleId) {
    title
    publicationDate

    ...ArticleAuthor_author
    ...ArticleComments_comments
  }
}
""")
class Article with $FetchArticleQuery extends StatelessWidget {

  final String articleId;
  const Article({Key key, this.articleId}) : super(key: key);

  // The actual build method is generated and sits in $FetchArticleQuery

  buildVars() => $FetchArticleQuery$Vars(articleId: this.articleId);

  Widget buildLoading(BuildContext context) {
    return CircularProgressIndicator();
  }

  Widget buildError(BuildContext, Error error) {
    return Text("$error");
  }

  @override
  Widget buildResult(BuildContext context, $FetchArticleQuery$Response data) {
    // Note how I don't have access here to author and comments
    // since they go directly through context to ArticleAuthor and ArticleComments.
    // This is "data-masking"

    return Column(
      children: [
        Text(data.article.title),
        Text(data.article.publicationDate.toString()),
        
        ArticleAuthor(),

        ArticleComments(),

      ]
    );
  }
}

@fragment(r"""
  fragment ArticleComments_comments {
    title,
    author {
      name
    }
  }
""")
class ArticleComments with $ArticleComments_commentsFragment extends StatelessWidget {
  
  const ArticleComments({Key key}) : super(key: key);
  // Here we don't need loading and error since it's only rendered after parent query was executed
  @override
  Widget buildFragment(BuildContext context, ArticleComments_commentsFragment$Data data) {
    return Column(
      children: data.map((comment) => Text(comment.title + comment.author.name))
    );
  }
}

@fragment("""
fragment ArticleAuthor_author {
  name
}
""")
class ArticleAuthor with $ArticleAuthor_authorFragment extends StatelessWidget {
  const ArticleAuthor({Key key}) : super(key: key);

  @override
  Widget buildFragment(BuildContext context, ArticleComments_authorFragment$Data data) {
    return Text(data.name);
  }
}

It's just a quick thought process, I'm not sure how to handle pagination, caching, offline mutations and so on. but it's a start. LMK what you think.

[smkhalsa]

Please see my comment here regarding dependency injection.

As mentioned in the comment referenced above, I think the annotated queries within dart files could be helpful (I think we could use a single gql annotation for this).

Can you expand on the value of the query / fragment mixins over simply using the Query widget?

[yosef-abraham]

A few downsides to the Query widget approach:

1. Query widgets requires me to imperatively check for loading/error/success states in the result.
2. The query widgets can't have an annotation.
3. The query widget doesn't support masked fragments and therefore forces me to have the query far away from down-the-tree widgets that suppose to be able to update their fragment requirements at ease.

[klavs]

@iscriptology, I think you've got something interesting here.

Would you be able to go the extra mile and "generate" the mixins by hand using the constructs of this GraphQL client? That would be very useful really understand the implications beyond what's visible in the use case.

The "generated" mixins do not need to be the best and cleanest implementation possible, but they need to do the job.

Skip the pagination, caching etc. for now

[smkhalsa]

I think this approach is too prescriptive with respect to how devs structure their widget trees.

First of all, it requires that the query be at the root of the tree. Consider the following example:

class AllPokemonScreen extends StatelessWidget {
  final client = GetIt.I<Client>();

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text('All Pokemon'),
      ),
      body: Query(
        client: client,
        queryRequest: AllPokemon(
          buildVars: (vars) => vars..first = 500,
        ),
        builder: (BuildContext context, QueryResponse<$AllPokemon> response) {
          if (response.loading)
            return Center(child: CircularProgressIndicator());

          final pokemons = response.data?.pokemons ?? [];

          return ListView.builder(
            itemCount: pokemons.length,
            itemBuilder: (context, index) => PokemonCard(
              pokemon: pokemons[index],
            ),
          );
        },
      ),
    );
  }
}

The mixin approach would require the dev to either create a separate widget for the query or put the query at the root. In the latter case, whenever there's an update to the data, it would re-render the entire widget tree. If there is an expensive build operation in the AppBar or any other widget outside of the Query above which doesn't depend on the query data, it would need to be re-rendered unnecessarily.

A second issue with this approach is that it assumes that devs want completely separate widget trees when there is an error or loading state. I often don't check the loading or error states until further down the widget tree, allowing the core structural widgets to load before data is available, resulting in a better user experience.

Consider this example:

class PokemonDetailScreen extends StatelessWidget {
  final client = GetIt.I<Client>();

  final String id;

  PokemonDetailScreen({this.id});

  @override
  Widget build(BuildContext context) {
    return Query(
      client: client,
      queryRequest: PokemonDetail(
        buildVars: (vars) => vars..id = id,
      ),
      builder: (BuildContext context, QueryResponse<$PokemonDetail> response) {
        final pokemon = response.data?.pokemon;

        return Scaffold(
          appBar: AppBar(
              title: Text(
            pokemon?.name,
          )),
          body: response.loading
              ? Center(
                  child: CircularProgressIndicator(),
                )
              : Column(
                  crossAxisAlignment: CrossAxisAlignment.stretch,
                  children: <Widget>[
                    if (pokemon != null)
                      PokemonCard(
                        pokemon: pokemon,
                      ),
                    Padding(
                      padding: const EdgeInsets.all(8.0),
                    ),
                    Text(
                      'Height',
                      style: Theme.of(context).textTheme.title,
                    ),
                    if (pokemon != null) Text('min: ${pokemon.height.minimum}'),
                    if (pokemon != null) Text('max: ${pokemon.height.maximum}'),
                    Padding(
                      padding: const EdgeInsets.all(8.0),
                    ),
                    Text(
                      'Weight',
                      style: Theme.of(context).textTheme.title,
                    ),
                    if (pokemon != null) Text('min: ${pokemon.weight.minimum}'),
                    if (pokemon != null) Text('max: ${pokemon.weight.maximum}'),
                  ],
                ),
        );
      },
    );
  }
}

In the mixin approach, I'd have to create a separate Scaffold, AppBar, and any other common components in buildLoading, buildError, and buildResult. Of course, I could create an additional build method that each of the above would call that builds the common components where I pass in data if it's available, but then what's the point of having separate build methods at all?

[yosef-abraham]

Good points @smkhalsa
Will try to fake a generated mixin that will also address those issues.
@klavs Hope to come up with one soon.

[yosef-abraham]

@smkhalsa @klavs
I played with some ideas, and it seems like no good option for mixin utilization. Yet I have some small tweaks that could make the usage of the library much more enjoyable.

1. Generate queries and fragments from annotations in addition to standalone graphql files.
2. Change (or compose) Query (in)to QueryBuilder with this added functionality:

• Get client from context if no client explicitly provided. (Provider.of)
• Provide all fragments through context and Provider.value widget. (I'm not sure how to do it, code generation I think is the only option cause it knows the query structure)

3. FragmentBuilder<T> widget that simply consumes the fragments provided through context and builds them.

Those are I think minor changes that will help a lot to flutter developers.

I "implemented" FragmentBuilder as it is straightforward:

class FragmentBuilder<T> extends StatelessWidget {
  final Function(BuildContext, T) builder;
  
  const FragmentBuilder({Key key, this.builder}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return builder(context, Provider.of<T>(context));
  }
}

QueryBuilder I think will require some code generation to provide all the fragments.

Let's see a usage example:

@gql(r"""
  query MyHomePage_query {
    ...AllFilmsList_allFilms
  }
""")
class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return QueryBuilder(
      query: MyHomePage_query(),
      builder: (context, QueryResponse<$MyHomePage_query> response) { 
        // The QueryBuilder implicitly exposese through context the AllFilmsList_allFilms fragment
        return AllFilmsList();
      }
    );
  }
}

@gql(r"""
  fragment AllFilmsList_allFilms {
    allFilms {
      title
    }
  }
""")
class AllFilmsList extends StatelessWidget {
  const AllFilmsList({Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return FragmentBuilder<$AllFilmsList_allFilms>(
      builder: (context, allFilms) {

      }
    );
  }
}

the mere fact that I can write my queries and fragments attached to my widgets is a huge benefit I think. The added use of provider to support those simple workflow will be much appreciated I think.

What do you guys think?

[smkhalsa]

@iscriptology

Generate queries and fragments from annotations in addition to standalone graphql files.

I like this idea. I suggest creating an issue in https://github.com/gql-dart/gql as this would need to be implemented in package:gql_build.

Get client from context if no client explicitly provided. (Provider.of)

Feel free to submit a PR for this.

Provide all fragments through context and Provider.value widget. (I'm not sure how to do it, code generation I think is the only option cause it knows the query structure)

I'm not quite sure what you mean. Can you provide an example?

FragmentBuilder widget that simply consumes the fragments provided through context and builds them.

I'm not sure why you need a FragmentBuilder widget. gql_build generates data.gql.dart files for each fragment automatically, so couldn't you simply pass a List<$AllFilmsList_allFilms> to AllFilmsList?

[klavs]

@iscriptology I like your ideas and your work in figuring out PoCs is especially useful.

But we have to stay true to the current limitations. When we start doing more in the tool, we... have to do more in the tool. Doing inline queries removes IDE support and ability to use external tools to help with GraphQL. More concretely, package:gql lacks the ability to validate documents, therefor writing GraphQL in annotations would not necessarily make the usage more enjoyable, before a supplemental tooling is implemented.

That said, it's a great goal to have and any help in achieving it is much appreciated!

[smkhalsa]

Get client from context if no client explicitly provided. (Provider.of)

@iscriptology #9 may make this moot

[yosef-abraham]

I tweaked the apollo-vscode extension on my fork (iscriptology/apollo-tooling) and it looks like it could support the @gql annotation quite nicely: (code completion, jump-to-definition etc.)

I think we can regardless of all the discussion to implement @gql annotation in gql_build and see how to take it from there in the flutter side of things.

[yosef-abraham]

@klavs Hey,
is there any update regarding @gql code generation? It would be so effective for me...
Also if I need to open a feature-request elsewhere please LMK where

[klavs]

@iscriptology There's no ongoing work on this.

I recommend you build your queries in a separate file as if they were used via annotations. That will guarantee easy adoption and also provide a good test set for when such functionality is implemented.

At this point I am not 100 percent sure where such functionality belongs to. There will probably be some Ferry-specific code needed as well as some re-usable code in gql_code_builder.

Prototypes and POCs are always welcome. If you were to provide a usage example with as much edge cases covered as possible, it would greatly increase the chance of us implementing something like that.

[smkhalsa]

Another interesting approach would be to do something like https://gqless.dev/

[JasCodes]

@smkhalsa This would be so awesome!!!

[smkhalsa]

I'd be interested in moving this forward once dart-lang/build#1689 (comment) is resolved.

Since Dart's build system currently can only output generated files to the source directory, the @gql annotation approach would result in a mess of generated files cluttering the source directory.

[itsparth]

@smkhalsa I love the approach of https://gqless.dev/, why can't we proceed without dart-lang/build#1689 (comment)? I mean we just need to generate at the root level where schema.graphql resides right?

[smkhalsa]

@itsparth to be clear, my comment above relates to using @gql annotations within source files, not to gqless.

Dart's build tool currently can only generate files as siblings of the source file. To prevent our source directories from getting cluttered with lots of generated files, we currently recommend creating a /graphql directory to contain your .graphql files (and the generated files). However, this obviously wouldn't work if we wanted to use @gql annotations from within our Dart source files.

With respect to gqless, I haven't put much thought into how to implement something equivalent in Dart. If you want to take a stab at a proof of concept, I'd love to explore it further though.

[itsparth]

@smkhalsa Thanks for the clarification, I am interested in taking the gqless approach forward, will think about it and try to come up with a POC. Will create a new issue for discussion around that.