zino-hofmann / graphql-flutter

A GraphQL client for Flutter, bringing all the features from a modern GraphQL client to one easy to use package.
https://zino-hofmann.github.io/graphql-flutter
MIT License
3.25k stars 618 forks source link

Mutation makes Query to re-run #389

Closed serendipity1004 closed 5 years ago

serendipity1004 commented 5 years ago

when I have Mutation and Query inside the same StatelessWidget, mutation call makes query to run again. Is this an intended behaviour? It is very problematic when you want to create infinite scroll and maybe perform actions on each item (press like or dislike). Making mutation causes query to re-run, making the list to start from the beginning again.

micimize commented 5 years ago

If you're using a normalized or optimistic cache, the query is probably being "rebroadcast" so that mutations are applied to the results. (If it's getting re-run on the network that's a big issue)

query is run again and the list is reset to index 0 because original query does not provide the cursor, only fetchMore does

Is the problem that the list re-renders as a new instance with and calls itemBuilder(context, index: 0), or is it an issue specifically with the variables?

I thought about adding a mechanism for merging variables in fetchMore, but didn't because Apollo doesn't. Will require some more consideration / investigation as to why they don't - part of my thinking was that it's mostly relevant in the fetchMore use-case, where you have the data to derive new variables from anyhow.

mainawycliffe commented 5 years ago

Does this behave the same when using InMemoryCache?

serendipity1004 commented 5 years ago

@micimize yes you are right. It was the re-broadcast. I checked the server request and there is no re-request after mutation. So, I am guessing that it is an issue with cache not being updated properly OR query not receiving correctly updated cache. I have a simple application that demonstrates this. I am running a mutation that just simply returns cache for updates which then should broadcast original length of items in cache but it is getting reset to first ever request made. I think this is some serious issue for those who are currently not using BloC architecture as you cannot use mutation and query inside same widget. If you can point me to right direction I could try to submit a pull request as I need this feature ASAP.

bug

Mobile

import 'package:flutter/material.dart';
import 'package:graphql_flutter/graphql_flutter.dart';
import 'package:cast_mobile/services/CacheKeyBuilder.dart';

const PAGINATE_TEST_OBJECT = """
  query paginateTestObject(\$next: String){
    paginateTestObject(next:\$next){
      __typename
      id
      nextKey
      nestedObj(next:\$next){
        __typename
        id
      }
    }
  }
""";

const PAGINATE_TEST_OBJECT2 = """
  query paginateTestObject2(\$next: String){
    paginateTestObject2(next:\$next){
      __typename
      id
    }
  }
""";

const MUTATION = """
  mutation testMutation(\$id:String!){
    testMutation(id:\$id){
      __typename
      id
    }
  }
""";

class TestScreen1 extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Query(
        options: QueryOptions(
          document: PAGINATE_TEST_OBJECT2,
        ),
        builder: (
          QueryResult queryResult, {
          VoidCallback refetch,
          FetchMore fetchMore,
        }) {
          print('------ QUERY HAS STARTED ------');

          if (queryResult.errors != null) {
            return Text('err');
          }

          if (queryResult == null || queryResult.data == null) {
            return Text('loading...');
          }

          final List data = queryResult.data['paginateTestObject2'];

          print('CURRENT DATA LENGTH IS ${data.length}');

          return ListView.builder(
            itemCount: data.length + 1,
            itemBuilder: (context, index) {
              if (index < data.length) {
                return Text(
                  data[index]['id'].toString(),
                );
              }

              return Column(
                children: <Widget>[
                  Mutation(
                    options: MutationOptions(
                      document: MUTATION,
                    ),
                    update: (Cache cache, QueryResult result){
                      print('------ UPDATE QUERY HAS STARTED ------');

                      print('------ UPDATE QUERY HAS ENDED ------');

                      return cache;
                    },
                    builder: (
                      RunMutation runMutation,
                      QueryResult result,
                    ) {
                      return RaisedButton(
                        child: Text('mutation'),
                        onPressed: () {
                          runMutation({
                            'id':'3'
                          });
                        },
                      );
                    },
                  ),
                  RaisedButton(
                    child: Text('LOAD MORE'),
                    onPressed: () {
                      FetchMoreOptions opts = FetchMoreOptions(
                        variables: {'next': data[data.length - 1]['id']},
                        updateQuery: (previousResultData, fetchMoreResultData) {

                          var oldData = previousResultData['paginateTestObject2'];
                          var newData =
                              fetchMoreResultData['paginateTestObject2'];

                          final List<dynamic> combined = [
                            ...oldData as List<dynamic>,
                            ...newData as List<dynamic>
                          ];

                          fetchMoreResultData['paginateTestObject2']= combined;

                          return fetchMoreResultData;
                        },
                      );

                      fetchMore(opts);
                    },
                  ),
                ],
              );
            },
          );
        },
      ),
    );
  }
}

SERVER

const TestObject = {
    nestedObj: (root: any, params: any, ctx: any) => {
        const arr = new Array(100).fill(0).map((item, index) => {
            return {id:index}
        });

        const {next} = params;

        const body = arr.splice(next ? parseInt(next) + 1 : 0, 20);

        return body;
    }
};

const Query = {
    paginateTestObject: (root:any, params:any, ctx:any) => {
        console.log(params.next);

        return {
            id:1,
            clientKey:params.next ? params.next : 0
        }
    },
    paginateTestObject2: (root:any, params: any, ctx:any) =>{
        console.log('paginate call');

        const arr = new Array(100).fill(0).map((item, index) => {
            return {id:index}
        });

        const {next} = params;

        const body = arr.splice(next ? parseInt(next) + 1 : 0, 20);

        return body;
    }
};

const Mutation = {
    testMutation: (root: any, params: any, ctx: any) => {
        return {
            id:params.id
        }
    }
};

export const resolver = {
    TestObject,
    Query,
    Mutation
};

type TestObject{
    id:ID!
    nestedObj(next:String): [NestedObj!]!
    clientKey: String!
}

type NestedObj{
    id:ID!
}

type Query{
    paginateTestObject(next:String):TestObject

    paginateTestObject2(next:String): [TestObject]
}

type Mutation{
    testMutation(id:String!) :NestedObj!
}
serendipity1004 commented 5 years ago

@mainawycliffe and yes. Even if I change cache to InMemoryCache, behaviour is the same as above.

mainawycliffe commented 5 years ago

I will debug and see if we can provide a quick solution for this.

serendipity1004 commented 5 years ago

@mainawycliffe how's the debugging going? would this be an issue even if I switch to BloC structure? Since BloC structure does not wrap the app with Notification widget, I am guessing mutation will not automatically trigger query?

mainawycliffe commented 5 years ago

I haven't had the time to troubleshoot yet, been caught up by work, hopefully today or tomorrow I will be able to. If you swap to bloc or any other state Management solutions for that matter, you won't face the problems with the two issues you are facing.

Regards Maina Wycliffe


From: Ji Ho Choi notifications@github.com Sent: Monday, August 26, 2019 4:06:19 AM To: zino-app/graphql-flutter graphql-flutter@noreply.github.com Cc: Maina Wycliffe mainawycliffe@outlook.com; Mention mention@noreply.github.com Subject: Re: [zino-app/graphql-flutter] Mutation makes Query to re-run (#389)

@mainawycliffehttps://github.com/mainawycliffe how's the debugging going? would this be an issue even if I switch to BloC structure? Since BloC structure does not wrap the app with Notification widget, I am guessing mutation will not automatically trigger query?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/zino-app/graphql-flutter/issues/389?email_source=notifications&email_token=AC5TXVTD4HYXMA2FBYWGLRLQGMUAXA5CNFSM4IODYUY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5DAANI#issuecomment-524681269, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AC5TXVVIQEOBTASLEQSNEUTQGMUAXANCNFSM4IODYUYQ.

micimize commented 5 years ago

@serendipity1004 I believe I encountered a related (possibly the same) issue. If I change a cached mutation result in updateQuery, the data isn't updated in the rendered widget. From my debugging it looks like it is the "query not receiving the correctly updated cache."

@mainawycliffe I'm going to take this as it seems like a rather big deal and not just a fetchMore issue.

serendipity1004 commented 5 years ago

@micimize yes. Thank you very much. I am currently updating my application so that it is using BloC and now it's behaving as expected. I hope this gets fixed ASAP as using this package as BloC implementation imposes so much more inefficiencies and much more code.

micimize commented 5 years ago

I was wrong, this is because we never write to cache in fetchMore 🤦‍♂

micimize commented 5 years ago

closed by #394

HofmannZ commented 5 years ago

:tada: This issue has been resolved in version 2.1.1-beta.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

HofmannZ commented 4 years ago

:tada: This issue has been resolved in version 3.0.0-beta.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

HofmannZ commented 4 years ago

:tada: This issue has been resolved in version 3.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: