Pagination

[jackdclark]

Closes: #41, closes #42, and closes #29

Provides a fetchMore function that will send the same query with the provided options merged into the original options, meaning the consumer doesn't have to manually maintain the state. Removed this as the same can be achieved with refetch now that it accepts options.

useQuery accepts an updateData function as part of the options object that enables control of how the previous & new results are merged, this function can be overridden if necessary when calling refetch as well.

TODO

• Agree on implementation
• Test coverage
• Update documentation

Example usage

// use options.updateResult to append the new page of posts to our current list of posts
const updateData = (prevData, prevData) => ({
  ...data,
  allPosts: [...prevData.allPosts, ...data.allPosts]
});

export default function PostList() {
  // set a default offset of 0 to load the first page
  const [skipCount, setSkipCount] = useState(0);

  const { loading, error, data } = useQuery(
    allPostsQuery,
    { variables: { skip: skipCount, first: 10 } },
    updateData
  );

  if (error) return <div>There was an error!</div>;
  if (loading && !data) return <div>Loading</div>;

  const { allPosts, _allPostsMeta } = data;
  const areMorePosts = allPosts.length < _allPostsMeta.count;

  return (
    <section>
      <ul>
        {allPosts.map(post => (
          <li key={post.id}>
            <a href={post.url}>{post.title}</a>
          </li>
        ))}
      </ul>
      {areMorePosts && (
        <button
          // set the offset to the current number of posts to fetch the next page
          onClick={() => setSkipCount(allPosts.length)}
        >
          Show more
        </button>
      )}
    </section>
  );
}

[bmullan91]

This looks like the right approach 👍I've suggested we move the update fn into the options, since we're already passing + extending them via useQuery and useClientRequest.

Should the fn be called update which would match Apollo's name for the same function, or updateResult?

[bmullan91]

LGTM 👍

[bmullan91]

😍

[bmullan91]

👏 excellent tests

[jackdclark]

@bmullan91 as discussed earlier I've removed fetchMore in favour of the declarative implementation. If a more imperative style is desired then the same can be achieved by using refetch and passing in new options.

I've also added a pagination guide to the README that includes distinct page & infinite loading examples.

If you could take another look when you get a chance please, hopefully we can get this in next week 😀

[bmullan91]

Since the name is updateResult, maybe we should pass the entire result? It might be useful for a developer to change / override the errors in some circumstances.

useClientRequest would then look something like:

let result = await client.request(revisedOperation, revisedOpts);

if (revisedOpts.updateResult) {
  if (typeof revisedOpts.updateResult !== 'function') {
    throw new Error('options.updateResult must be a function');
  }
  result = revisedOpts.updateResult(state, result)
}

The pagination example updateResult:

const updateResult = (prevResult, newResult) => ({
  ...newResult,
  data: {
    ...newResult.data,
    allPosts: [...prevResult.data.allPosts, ...result.data.allPosts]
  }
});

useQuery(MY_QUERY, {
  updateResult
})

It makes updating the data a bit more verbose, but the developer could write a wrapper if it was a really common usecase:

const dataUpdater = (fn) => (prevResult, newResult) => ({
  ...newResult,
  data: fn(prevResult.data, newResult.data)
})

[jackdclark]

As discussed, passing the entire result back allows internal properties like loading & error to be changed, or new ones to be added which could be abused or cause tricky bugs. We're going to rename this to updateData as a better description of what it's doing & if we see a use case for changing / overriding errors in the future, we could always introduce an updateErrors option.

[bmullan91]

LGTM 👍 - some minor comments just

[bmullan91]

We should simplify the example - this is could be:

ALL_POSTS_QUERY = `
  query($limit: Int, $skip: Int) {
    allPosts(limit: $limit, skip: $skip) {
      total,
      items {
        id
        title
      }
    }
  }
`

[jackdclark]

I think the separation between data & meta is good here? There are redundant parts to this example that could be removed & make it more readable though, how about:

export const ALL_POSTS_QUERY = `
  query allPosts($first: Int!, $skip: Int!) {
    allPosts(first: $first, skip: $skip) {
      id
      title
      url
    }
    _allPostsMeta {
      count
    }
  }
`;

[bmullan91]

Minor: We should add a comment here saying why we're doing this

[jackdclark]

Agree, I've added an explanation & added a note to the README for refetch

[bmullan91]

Great work @jackdclark 🙌