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

relayStylePagination returns undefined for non-paginated query on field #6611

Closed
WillSquire opened this issue Jul 16, 2020 · 7 comments · Fixed by #6684
Closed

relayStylePagination returns undefined for non-paginated query on field #6611

WillSquire opened this issue Jul 16, 2020 · 7 comments · Fixed by #6684
Assignees
Milestone

Comments

@WillSquire
Copy link

Intended outcome:
Expecting results to come back when querying a relayStylePagination field for just totalCount. E.g:

query usersCount {
  users {
    totalCount
  }
}

Actual outcome:
Both data and error come back as undefined after migrating to relayStylePagination from updateQuery.

How to reproduce the issue:
relayStylePagination setup like so:

new InMemoryCache({
  typePolicies: {
    Query: {
      fields: {
        users: relayStylePagination(['groupId']),
      },
    },
  },
})

When querying for usersCount above, then data and error both come back as undefined. updateQuery was previously on each call, so it didn't apply to this query.

Versions
@apollo/client: 3.0.0

@benjamn benjamn added this to the Post 3.0 milestone Jul 16, 2020
@benjamn benjamn self-assigned this Jul 16, 2020
@benjamn
Copy link
Member

benjamn commented Jul 16, 2020

Ahh, I think I know what this is. The read function returns the result of makeEmptyData() by default, which is an object that only has edges and pageInfo. While the merge function does try to save any additional properties like totalCount that are received from the server, those extra properties are not present in the initial empty data, and anything undefined looks like a missing field when reading from the cache. I'll put together a PR to add totalCount: 0 to the makeEmptyData result, but if you have time to try making that change locally, that would help confirm this theory.

@fracmak
Copy link
Contributor

fracmak commented Jul 22, 2020

what is the purpose of makeEmptyData() on the read function? I'm finding it's causing problems in my react app where the makeEmptyData() is preventing queries from hitting the backend under certain circumstances (I believe it's priming the cache with empty results)

benjamn added a commit that referenced this issue Jul 23, 2020
Fixes #6611, thanks to this astute comment from @fracmak:
#6611 (comment)

Returning undefined from a field read function indicates to the
StoreReader that the field is missing, and usually means the query will be
fetched from the server. When we haven't yet written any data to a field
paginated using relayStylePagination, requesting data from the server is
almost always better than providing any kind of default information, so we
should avoid using makeEmptyData() as a default value.
@benjamn
Copy link
Member

benjamn commented Jul 23, 2020

@fracmak I think that's actually the most elegant solution—thanks! See #6684 for a fix.

benjamn added a commit that referenced this issue Jul 23, 2020
…#6684)

Fixes #6611, thanks to this astute comment from @fracmak:
#6611 (comment)

Returning undefined from a field read function indicates to the
StoreReader that the field is missing, and usually means the query will be
fetched from the server. When we haven't yet written any data to a field
paginated using relayStylePagination, requesting data from the server is
almost always better than providing any kind of default data, so we should
avoid using makeEmptyData() as a default value for the existing parameter.
@binchik
Copy link

binchik commented Aug 6, 2020

@benjamn relayStylePagination implementation assumes that query has edges and pageInfo defined, but the query may lack one or both of those. I have the same case as @WillSquire. A query like:

query usersCount {
  users {
    totalCount
  }
}

It'll crash in merge on incoming.edges.slice(0) with "incoming.edges is undefined"

@binchik
Copy link

binchik commented Aug 6, 2020

I patched it with:

diff --git a/node_modules/@apollo/client/utilities/utilities.cjs.js b/node_modules/@apollo/client/utilities/utilities.cjs.js
index 99cc06a..788e8d7 100644
--- a/node_modules/@apollo/client/utilities/utilities.cjs.js
+++ b/node_modules/@apollo/client/utilities/utilities.cjs.js
@@ -692,7 +692,7 @@ function relayStylePagination(keyArgs) {
             var args = _a.args;
             if (!args)
                 return existing;
-            var incomingEdges = incoming.edges.slice(0);
+            var incomingEdges = (incoming.edges || []).slice(0);
             if (incoming.pageInfo) {
                 updateCursor(incomingEdges, 0, incoming.pageInfo.startCursor);
                 updateCursor(incomingEdges, -1, incoming.pageInfo.endCursor);
@@ -716,7 +716,7 @@ function relayStylePagination(keyArgs) {
             var edges = tslib.__spreadArrays(prefix, incomingEdges, suffix);
             var pageInfo = tslib.__assign(tslib.__assign(tslib.__assign({}, incoming.pageInfo), existing.pageInfo), { startCursor: cursorFromEdge(edges, 0), endCursor: cursorFromEdge(edges, -1) });
             var updatePageInfo = function (name) {
-                var value = incoming.pageInfo[name];
+                var value = incoming.pageInfo ? incoming.pageInfo[name] : undefined;
                 if (value !== void 0) {
                     pageInfo[name] = value;
                 }

@anark
Copy link
Contributor

anark commented Aug 28, 2020

Also running into issues after querying for only totalCount. Tried this patch and version 3.2.0.beta-7 but still couldn't get reliable updates.

It seems if you first query without any edges/pageInfo

query usersCount {
  users {
    totalCount
  }
}

And then query for the edges

query usersCount {
  users {
    totalCount
    pageInfo{
      hasNextPage
      endCursor
    }
    edges {
      node{
        email
      }
    }
  }
}

The users will always be an empty and return no data.

@anark
Copy link
Contributor

anark commented Aug 29, 2020

Using some of @binchik solution and still functioning without args #6935

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants