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

previousResult is undefined when using updateQuery in fetchMore #5897

Open
saschametz opened this issue Feb 1, 2020 · 17 comments
Open

previousResult is undefined when using updateQuery in fetchMore #5897

saschametz opened this issue Feb 1, 2020 · 17 comments
Assignees
Milestone

Comments

@saschametz
Copy link
Contributor

saschametz commented Feb 1, 2020

Intended outcome:
The previousResult variable should contain the previous data.

Actual outcome:
The previousResult is undefined

How to reproduce the issue:
I followed the tutorial on this page using the Relay-style cursor pagination. After switching everything to the latest stable apollo version the previousResult variable contains the data.

fetchMore({
          variables: {
            cursor: comments.pageInfo.endCursor
          },
          updateQuery: (previousResult, { fetchMoreResult }) => {
            const newEdges = fetchMoreResult.comments.edges;
            const pageInfo = fetchMoreResult.comments.pageInfo;

            // Apollo v3 Beta
           // previousResult is undefined

           // Apollo latest stable version
           // previousResult contains the data

            return newEdges.length
              ? {
                  // Put the new comments at the end of the list and update `pageInfo`
                  // so we have the new `endCursor` and `hasNextPage` values
                  comments: {
                    __typename: previousResult.comments.__typename,
                    edges: [...previousResult.comments.edges, ...newEdges],
                    pageInfo
                  }
                }
              : previousResult;
          }
        })

Versions
v3.0.0-beta.20

@benjamn
Copy link
Member

benjamn commented Feb 3, 2020

Can you put together a reproduction? https://github.com/apollographql/react-apollo-error-template

That documentation will be rewritten before AC3 is shipped, but one thing I can tell you is that the @connection directive is no longer necessary (and should no longer be used) with AC3.

@benjamn benjamn added this to the Release 3.0 milestone Feb 3, 2020
@toddtarsi
Copy link

I am also encountering this issue, but I am seeing it whenever I'm using the fetchMore API. I need to do some investigating, but it seems like everything is honky-dorey whenever I run this on Chrome. However, if I run this in Cordova on Android, there's going to be a CORS preflight step and the fetchMore API always comes back with a first parameter of undefined in that environment. I will investigate further, but this is certainly an issue for my team as we're coming online in AC3.

@toddtarsi
Copy link

toddtarsi commented Feb 13, 2020

Okay, as I investigate the difference between my two platforms (Chrome web vs Cordova Android), I'm seeing one primary difference.

This line: https://github.com/apollographql/apollo-client/blob/master/src/core/ObservableQuery.ts#L596

When I log result here on web, stale is false.
On Cordova Android, stale is true.

...interesting

EDIT: Going a little further here, I'm seeing all data returned, but the complete flag is false on Cordova and true on web. This trips isMissing = true, which bubbles up to stale = true, which ends with us at undefined I think. Will post more findings as I see why complete = false.

@toddtarsi
Copy link

toddtarsi commented Feb 13, 2020

Just to note, I tried the fix from #5938 to no avail unfortunately.

@toddtarsi
Copy link

Investigating further, I think its because I have two mutations running quite sequentially here. I'm sorry to be vague. If I find a fix or a source, I'll report here.

@toddtarsi
Copy link

toddtarsi commented Feb 14, 2020

Okay, I am through my issue here.
@saschametz - The issue for me came down to that I had a nested field on my user object:

user { field { subfield1, subfield2 }}

And then I had two concurrent mutations, one of which writes to subfield1 and one of which writes to subfield2.

However, they changed the default cache normalization for objects without ids to just fully overwrite instead of a full merge function like lodash. Good for performance, bad for stability.

My fix came down to making this change to my InMemoryCache:

const cacheOptions = {
  typePolicies: {
    User: {
      fields: {
        field: {
          merge(existing, incoming, { mergeObjects }) {
            // Correct, thanks to invoking nested merge functions.
            return mergeObjects(existing, incoming);
          },
        },
      },
    },
  },
};

This can be further read about here: https://github.com/apollographql/apollo-client/blob/70247206dfa59d16251ce8cae654731c0986b9fe/docs/source/caching/cache-field-behavior.md

There is some new stuff full of gotchas.

@benjamn - I am not sure how, but I would recommend surfacing some of the invariants that get caught here somehow as well. I don't have returnPartialData defined anywhere in my codebase, but this throw never got back to me or was never invoked. I couldn't tell:

https://github.com/apollographql/apollo-client/blob/master/src/cache/inmemory/readFromStore.ts#L172

I only found this because calls to fetchMore later would have a clientData of null.

@toddtarsi
Copy link

@benjamn - I don't mean to spam you guys too much, but if you add a logger right here letting people know the mine cart just left the tracks a bit, you may have a lot less vagueness and confusion in which issues get reported:

throw execResult.missing[0];

Generally, any time I am having issues on 3.x, my first step is to add a logger to this, and then I know which fires to fight before I get to undefined client data on an updateQuery call.

I'm not trying to spend too much of your time. I just wanted to recommend this logger to help keep others from being confused with your time.

@benjamn benjamn self-assigned this Feb 21, 2020
@tcodes0
Copy link

tcodes0 commented Mar 3, 2020

Hhaving a similar issue, no easy fix in sight tho. @toddtarsi my previous result seems to come undefined only for subscriptions

@toddtarsi
Copy link

@Thomazella - Did you add a console logger in the area I recommended above? It sucks to add a logger there, but it helped me track down some issues that weren't escaping in an obvious way. If using npm to install it, you'll have to add it to the transpiled code, but it isn't so bad. Also, feel free to ignore this if not using the InMemoryCache.

@hwillson
Copy link
Member

Just a heads up, updateQuery has been deprecated in AC3 (see #6464). We're happy to answer questions about switching to use field policies instead, and will have new documentation ready shortly.

@hwillson hwillson modified the milestones: Release 3.0, Post 3.0 Jul 10, 2020
@toddtarsi
Copy link

toddtarsi commented Jul 10, 2020

Yeah, I have a question:

I have a query shaped something like this (pseudocode):

user {
  accounts [{
    accountItems [Item]
  }]
  topItems [Item]
}

And I have a number of fetchMore queries, which all return [Item]. If updateQuery is gone, how do I tell some of those queries to append to topItems, and some of those queries to append to accountItems for account xyz?

@benjamn
Copy link
Member

benjamn commented Jul 10, 2020

@toddtarsi Something like this?

new InMemoryCache({
  typePolicies: {
    User: {
      fields: {
        topItems: {
          merge(existing = [], incoming) {
            return [...existing, ...incoming];
          },
        },
      },
    },
    Account: {
      fields: {
        accountItems: {
          merge(existing = [], incoming) {
            return [...existing, ...incoming];
          },
        },
      },
    },
  },
})

Of course you can make this a little less repetitive by using a helper function, like the concatPagination function we provide:

import { concatPagination } from "@apollo/client/utilities"

new InMemoryCache({
  typePolicies: {
    User: {
      fields: {
        topItems: concatPagination(),
      },
    },
    Account: {
      fields: {
        accountItems: concatPagination(),
      },
    },
  },
})

If you need to do something more complicated, fetchMore returns a Promise with the new results, which you can write manually into the cache however you see fit.

@toddtarsi
Copy link

toddtarsi commented Jul 10, 2020

@benjamn - Sorry, I didn't explain quite well enough. Let me try harder.

This is a simplification of my schema.

type Item {
  ID: Int
}

type Account {
  accountItems: [Item]
}

type User {
  accounts: [Account]
  topItems: [Item]
}

type Schema {
  mainUserQuery: User
  fetchSomeTopItems(inputs): [Item]
  fetchSomeAccountItems(inputs): [Item] 
}

The schema here is type Schema. Type User is meant to be the shape of the main user query.

Now, in my app I call fetchMore with fetchSomeTopItems at some point, and fetchMore with fetchSomeAccountItems at other times. With updateQuery I would append to topItems, or accountItems as needed using some factory reducer. With cache policies, I don't see how I could do this.

Ah, I see your update. Okay, so I should just use the promise result from fetchMore and use the updateQuery API manually. That makes sense, thank you for your answer.

Just to check, updateQuery as a property on the fetchMore API is deprecated, but updateQuery as a direct API call is still a valid property on a query result, correct?

@benjamn
Copy link
Member

benjamn commented Jul 10, 2020

Just to check, updateQuery as a property on the fetchMore API is deprecated, but updateQuery as a direct API call is still a valid property on a query result, correct?

Correct!

@toddtarsi
Copy link

@benjamn - Makes sense; a little bit of hoisting and I should be right as rain then. Thank you for the help sir!

@0soltys
Copy link

0soltys commented Aug 13, 2020

const { data, fetchMore } = useQuery(SOME_QUERY);

instead of this

fetchMore({
        variables: { ... },
        updateQuery: (prev: any, { fetchMoreResult }) => {
          if (!fetchMoreResult) return prev;
          return {
            fetchedData: {
              ...prev?.fetchedData,
              data: [...prev?.fetchedData?.data, ...fetchMoreResult?.fetchedData?.data],
            },
          };
        },
      });

I do something like that, and it's solve the pb

fetchMore({
        variables: { ... },
        updateQuery: (prev: any, { fetchMoreResult }) => {
          if (!fetchMoreResult) return prev;
          return {
            fetchedData: {
              ...data?.fetchedData,
              data: [...data?.fetchedData?.data, ...fetchMoreResult?.fetchedData.?data],
            },
          };
        },
      });

@nikhedonia
Copy link

nikhedonia commented Oct 21, 2020

I'm experiencing the same issue in @apollo/[email protected] when using useQuery.
Any updates on this issue?
I've noticed also the error is not deterministic and seems to be related to network latency.

@soltysaleksandr workaround works but it would be nice if it would work out of the box

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

9 participants