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

subscribeToMore causes the initial request to be refetched #7817

Open
geosigno opened this issue Mar 10, 2021 · 13 comments
Open

subscribeToMore causes the initial request to be refetched #7817

geosigno opened this issue Mar 10, 2021 · 13 comments

Comments

@geosigno
Copy link

Hello,

I'm setting up a quite simple message feed on my app. There is an infinite scroll that uses the fetchMore function with a cursor pagination logic and a subscribeToMore to listen for new messages.

I can succeed to make the apollo working for fetching more OR subscription but not for both...

This is my initial query:

    messageFeed(id: $id, cursor: $cursor) {
      messages {
        id
        value
      }
      cursor
    }

This is my merging function:

typePolicies: {
  Query: {
    fields: {
      messageFeed: {
        keyArgs: false,
        merge(existing, incoming) {
          let messages
          if (existing) {
            messages = [...existing.messages, ...incoming.messages];
          } else {
            messages = incoming.messages;
          }
          return {
            cursor: incoming.cursor,
            messages
          }
        }
      }
    }
  }
}

So if I stop here, it is working fine. The merging is well executed and I got an infinite flow of messages while I'm scrolling down.

Here come the subscription and the issues...

This is my subscribeToMore function:

    subscribeToMore({
      document: NEW_MESSAGE_SUBSCRIPTION,
      variables: {
          discussionId: discussionID
      },
      updateQuery: (prev, { subscriptionData }) => {
        if (!subscriptionData.data) return prev;
        const newMessage = subscriptionData.data.message;
        return  {
          messageFeed: {
            messages: [newMessage]
          }
        };
      }
    })

When a new message is posted, here what happening on the DOM:

initial 20 first messages
new message from the subscription
again initial 20 first messages

When debugging, I noticed the merge function is called twice.

First call (when the subscription is fired)
existing => the first 20 messages already displayed in the DOM
incoming => the new message from the subscription
merge => the two are merged and the 21 messages are well returned.

It should stop here.

BUT then, a new graphql post (with the same variables as the initial request at page load) is coming from nowhere which cause the merge function to be called again:

existing => the 21 messages already displayed in the DOM
incoming => the same 20 messages as the initial request
merge => the two are merge and the 41 and returning (including 20 duplicates)

Any idea from where is coming that second call?

@geosigno geosigno changed the title Subscription merging create dupplicata subscribeToMore causes the initial request to be refetched Mar 10, 2021
@benjamn benjamn self-assigned this Mar 10, 2021
@benjamn
Copy link
Member

benjamn commented Mar 10, 2021

@geosigno I agree the extra write should not be happening. What fetchPolicy (if any) are you using for the initial query? Any chance you could put the relevant code into a runnable reproduction?

@geosigno
Copy link
Author

I don't use any fetchPolicy. It will not be easy to extract that in a repo...

I was thinking about adding a "filter" function in the merge that will remove any duplicate, but this would not be very optimal, especially when hundreds/thousands of questions will be refetched....

Any other idea?

@geosigno
Copy link
Author

I think the issue is only with the subscribeToMore. Even if i remove the merge part and keep only the subscribeToMore i got the same issue than #6340.

@LovingGarlic
Copy link

I'm having this same exact issue, for the same exact use case 😅

Currently looking for the best way to merge and sort the results as a workaround

@geosigno
Copy link
Author

I managed to make it work with the below merge config:

    messageFeed: {
          merge(existing, incoming, { readField }) {
            const messages = existing ? { ...existing.messages } : {};
            incoming.messages.forEach((message) => {
              messages[readField("id", message)] = message;
            });
            return {
              cursor: incoming.cursor,
              messages,
            };
          },
          read(existing) {
            if (existing) {
              return {
                cursor: existing.cursor,
                messages: Object.values(existing.messages),
              };
            }
          },
        },

my subscribeToMore updateQuery is as below:

updateQuery: (prev, { subscriptionData }) => {
        if (!subscriptionData.data) return prev;
        const newMessage = subscriptionData.data.message.data;
        return Object.assign({}, prev, {
          messageFeed: {
            messages: [newMessage],
          },
        });
      },

The only issue left I had was that the new message was not on the top of the list. To fix this, I added a sort() function at the place where the questions were returned in the DOM.

I hope it will help you, let me know if anything is unclear. And hopefully, this phantom request will be fixed very soon.

@LovingGarlic
Copy link

Thanks for that @geosigno. I was able to determine if the request was triggered by a fetchMore or a subscription based on the presence or lack of a cursor in the variables and then implement a different merge strategy from there and luckily in my case neither of them require looping through all the messages. Here's to hoping this does get fixed soon

@LovingGarlic
Copy link

I've found a way to (for me at least) avoid the phantom request here, although it does still run the merge function after the subscription fires.
First I went through and made sure that both my updateQuery and my merge functions recreate their respective objects exactly including __typename fields.
Then, I set up my call to useQuery as such:

const { data, error, networkStatus, subscribeToMore, fetchMore } = useQuery(QUERY, {
    notifyOnNetworkStatusChange: true,
    fetchPolicy: 'cache-and-network',
    nextFetchPolicy: 'cache-first',
    variables: {
      id: id,
      first: 15,
    },
  });

By using cache-and-network then cache-first it gets the inital data from whereever it can (cache and/or network) then each time after that, it will check the cache first, avoiding the extra network request. I do still need to check in my merge function for a cursor as I mentioned in my previous comment, but at least I got rid of that phantom request 😅

@Akryum
Copy link

Akryum commented Apr 7, 2021

Maybe related to #6916

@FedeBev
Copy link

FedeBev commented Apr 19, 2021

In a situation without a pagination to deal with, setting nextFetchPolicy: 'cache-first', as suggested by @LovingGarlic, works perfectly.

@geosigno
Copy link
Author

geosigno commented Apr 28, 2021

First I went through and made sure that both my updateQuery and my merge functions recreate their respective objects exactly including __typename fields.

@LovingGarlic Would you mind share the code here?

@LovingGarlic
Copy link

First I went through and made sure that both my updateQuery and my merge functions recreate their respective objects exactly including __typename fields.

@LovingGarlic Would you mind share the code here?

Essentially it's something like this:

merge(existing, incoming, { args }) {
    if (!incoming) {
      return [];
    }

    if (!existing || !args.paginationArgs.before) {
      return incoming;
    }

    return {
      __typename: existing.__typename,
      pageInfo: {
        ...incoming.pageInfo,
      },
      edges: args.paginationArgs.before
        ? [...incoming.edges, ...existing.edges]
        : [...existing.edges, ...incoming.edges],
    };
},

Note in the second conditional, that I check for the existence of a before cursor (my pagination only goes one direction). This is to determine if the merge was triggered by a fetchMore or a subscription as mentioned above. The notice in the last return, making sure to add the __typename field to the returned object (this seems necessary for the cache to be updated properly), then building out the pageInfo and edges. A similar strategy would be needed for the updateQuery.

@samkit5495
Copy link

I am too getting the phantom request after subscribeToMore is called. Is anyone working on fixing this ?

@alessbell
Copy link
Member

Hi folks 👋 Is anyone able to provide a runnable reproduction here?

We have a CodeSandbox that mirrors a forkable GitHub repo where you'll find a subscriptions.tsx already set up using GraphQLWsLink. This would greatly help any debugging efforts. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants