bug: fix pagination double fetch

packages/peregrine/lib/hooks/usePagination.js

@@ -1,14 +1,95 @@
-import { useMemo, useState } from 'react';
+import { useCallback, useMemo, useState } from 'react';
+import { getSearchParam } from './useSearchParam';
 
-export const usePagination = () => {
-    const [currentPage, setCurrentPage] = useState(0);
-    const [totalPages, setTotalPages] = useState(null);
+/**
+ * Sets a query parameter in history. Attempt to use React Router if provided
+ * otherwise fallback to builtins.
+ */
+const setQueryParam = ({ location, history, parameter, value }) => {
+    const { search } = location;
+    const queryParams = new URLSearchParams(search);
+    queryParams.set(parameter, value);
 
+    if (history.push) {
+        history.push({ search: queryParams.toString() });
+    } else {
+        // Use the native pushState. See https://developer.mozilla.org/en-US/docs/Web/API/History_API#The_pushState()_method
+        history.pushState({ search: queryParams.toString() }, '');
+    }
+};
+
+const defaultInitialPage = 1;
+
+/**
+ * `usePagination` provides a pagination state with `currentPage` and
+ * `totalPages` as well as an API for interacting with the state.
+ *
+ * @param {Object} location the location object, like window.location, or from react router
+ * @param {Object} history the history object, like window.history, or from react router
+ * @param {String} namespace the namespace to apply to the pagination query
+ * @param {String} parameter the name of the query parameter to use for page
+ * @param {Number} initialPage the initial current page value
+ * @param {Number} intialTotalPages the total pages expected to be usable by this hook
+ *
+ * TODO update with defaults
+ *
+ * @returns {[PaginationState, PaginationApi]}
+ */
+export const usePagination = ({
+    location = window.location,
+    history = window.history,
+    namespace = '',
+    parameter = 'page',
+    initialPage,
+    initialTotalPages = 1
+} = {}) => {
+    const searchParam = namespace ? `${namespace}_${parameter}` : parameter;
+    if (!initialPage) {
+        // We need to synchronously fetch the initial page value from the query
+        // param otherwise we would initialize this value twice.
+        initialPage = parseInt(
+            getSearchParam(searchParam, location) || defaultInitialPage
+        );
+    }
+
+    const [currentPage, setCurrentPage] = useState(initialPage);
+    const [totalPages, setTotalPages] = useState(initialTotalPages);
+
+    const setPage = useCallback(
+        page => {
+            // Update the query parameter.
+            setQueryParam({
+                location,
+                history,
+                parameter: searchParam,
+                value: page
+            });
+
+            // Update the state object.
+            setCurrentPage(page);
+        },
+        [history, location, searchParam]
+    );
+
+    /**
+     * @typedef PaginationState
+     * @property {Number} currentPage the current page
+     * @property {Number} totalPages the total pages
+     */
     const paginationState = { currentPage, totalPages };
-    const api = useMemo(() => ({ setCurrentPage, setTotalPages }), [
-        setCurrentPage,
-        setTotalPages
-    ]);
+
+    /**
+     * @typedef PaginationApi
+     * @property {Function} setCurrentPage
+     * @property {Function} setTotalPages
+     */
+    const api = useMemo(
+        () => ({
+            setCurrentPage: setPage,
+            setTotalPages
+        }),
+        [setPage, setTotalPages]
+    );
 
     return [paginationState, api];
 };

packages/peregrine/lib/hooks/useQuery.js

@@ -28,7 +28,14 @@ export const useQuery = query => {
      */
     const runQuery = useCallback(
         async ({ variables }) => {
-            const payload = await apolloClient.query({ query, variables });
+            let payload;
+            try {
+                payload = await apolloClient.query({ query, variables });
+            } catch (e) {
+                payload = {
+                    error: e
+                };
+            }
             receiveResponse(payload);
         },
         [apolloClient, query, receiveResponse]

packages/peregrine/lib/hooks/useSearchParam.js

@@ -1,6 +1,6 @@
 import { useEffect } from 'react';
 
-const getSearchParam = (parameter = '', location = window.location) => {
+export const getSearchParam = (parameter = '', location = window.location) => {
     const params = new URLSearchParams(location.search);
 
     return params.get(parameter) || '';

packages/venia-concept/src/RootComponents/Category/__tests__/__snapshots__/categoryContent.spec.js.snap

@@ -24,7 +24,7 @@ exports[`renders the correct tree 1`] = `
   <div
     className="d"
   >
-    <withRouter(Classify(Pagination))
+    <Classify(Pagination)
       pageControl={Object {}}
     />
   </div>

packages/venia-concept/src/RootComponents/Category/category.js

@@ -5,8 +5,10 @@ import { usePagination, useQuery } from '@magento/peregrine';
 import { toggleDrawer } from 'src/actions/app';
 import catalogActions from 'src/actions/catalog';
 import { mergeClasses } from 'src/classify';
+
 import { fullPageLoadingIndicator } from 'src/components/LoadingIndicator';
-import { connect } from 'src/drivers';
+import { connect, withRouter } from 'src/drivers';
+import { compose } from 'redux';
 import categoryQuery from 'src/queries/getCategory.graphql';
 import isObjectEmpty from 'src/util/isObjectEmpty';
 import { getFilterParams } from 'src/util/getFilterParamsFromUrl';
@@ -15,22 +17,25 @@ import defaultClasses from './category.css';
 
 const Category = props => {
     const { filterClear, id, openDrawer, pageSize } = props;
+    const classes = mergeClasses(defaultClasses, props.classes);
+
+    const [paginationValues, paginationApi] = usePagination({
+        history: props.history,
+        location: props.location
+    });
 
-    const [paginationValues, paginationApi] = usePagination();
     const { currentPage, totalPages } = paginationValues;
     const { setCurrentPage, setTotalPages } = paginationApi;
 
     const pageControl = {
         currentPage,
         setPage: setCurrentPage,
-        updateTotalPages: setTotalPages,
         totalPages
     };
 
     const [queryResult, queryApi] = useQuery(categoryQuery);
     const { data, error, loading } = queryResult;
     const { runQuery, setLoading } = queryApi;
-    const classes = mergeClasses(defaultClasses, props.classes);
 
     // clear any stale filters
     useEffect(() => {
@@ -65,13 +70,27 @@ const Category = props => {
 
     useEffect(() => {
         setTotalPages(totalPagesFromData);
+        return () => {
+            setTotalPages(null);
+        };
     }, [setTotalPages, totalPagesFromData]);
 
-    if (error) return <div>Data Fetch Error</div>;
+    // If we get an error after loading we should try to reset to page 1.
+    // If we continue to have errors after that, render an error message.
+    useEffect(() => {
+        if (error && !loading && currentPage !== 1) {
+            setCurrentPage(1);
+        }
+    }, [currentPage, error, loading, setCurrentPage]);
+
+    if (error && currentPage === 1 && !loading) {
+        return <div>Data Fetch Error</div>;
+    }
 
-    // show loading indicator until data has been fetched
-    // and pagination state has been updated
-    if (!totalPages) return fullPageLoadingIndicator;
+    // Show the loading indicator until data has been fetched.
+    if (!totalPagesFromData) {
+        return fullPageLoadingIndicator;
+    }
 
     return (
         <CategoryContent
@@ -96,6 +115,8 @@ Category.propTypes = {
 
 Category.defaultProps = {
     id: 3,
+    // TODO: This can be replaced by the value from `storeConfig when the PR,
+    // https://github.com/magento/graphql-ce/pull/650, is released.
     pageSize: 6
 };
 
@@ -104,7 +125,10 @@ const mapDispatchToProps = dispatch => ({
     openDrawer: () => dispatch(toggleDrawer('filter'))
 });
 
-export default connect(
-    null,
-    mapDispatchToProps
+export default compose(
+    withRouter,
+    connect(
+        null,
+        mapDispatchToProps
+    )
 )(Category);

packages/venia-concept/src/components/Gallery/items.js

@@ -2,7 +2,9 @@ import React, { Component } from 'react';
 import { arrayOf, number, shape } from 'prop-types';
 import GalleryItem from './item';
 
-const pageSize = 12;
+// TODO: This can be replaced by the value from `storeConfig when the PR,
+// https://github.com/magento/graphql-ce/pull/650, is released.
+const pageSize = 6;
 const emptyData = Array.from({ length: pageSize }).fill(null);
 
 // inline the placeholder elements, since they're constant

packages/venia-concept/src/components/Pagination/__tests__/__snapshots__/pagination.spec.js.snap

@@ -0,0 +1,26 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`renders nothing when there is only 1 page 1`] = `null`;
+
+exports[`renders when there is more than 1 page 1`] = `
+<div
+  className="root"
+>
+  <button
+    onClick={[Function]}
+  >
+    <div />
+    1
+  </button>
+  <button
+    onClick={[Function]}
+  >
+    2
+  </button>
+  <button
+    onClick={[Function]}
+  >
+    3
+  </button>
+</div>
+`;