Merge pull request #67 from commitd/sh/usepage

stuarthendren · web-flow · commit a8163a32b940 · 2023-03-14T13:22:29.000Z
fix(usepagination): fixing prop update issue with pagination
diff --git a/src/usePagination/usePagination.stories.tsx b/src/usePagination/usePagination.stories.tsx
@@ -10,7 +10,7 @@ import {
   Slider,
 } from '@committed/components'
 import { Meta, Story } from '@storybook/react'
-import React, { useEffect, useMemo, useState } from 'react'
+import React, { useMemo, useState } from 'react'
 import useSwr from 'swr'
 import { PaginationData, usePagination } from '.'
 
@@ -84,7 +84,7 @@ const Template: Story<UsePaginationDocsProps> = (args) => {
 }
 
 export const Default = Template.bind({})
-Default.args = { totalItems: 100, startPage: 1, pageSize: 20 }
+Default.args = { totalItems: 100, pageSize: 20 }
 
 export const ClientSide: Story = () => {
   const items = Array.from({ length: 100 }, (v, i) => i)
@@ -97,7 +97,7 @@ export const ClientSide: Story = () => {
     <>
       <Row css={{ gap: '$2', mb: '$3' }}>
         {items.slice(startIndex, endIndex).map((item) => (
-          <div>{item}</div>
+          <div key={`${item}`}>{item}</div>
         ))}
       </Row>
       <Pagination page={page} onPageChange={setPage} count={totalPages} />
@@ -160,7 +160,7 @@ export const ServerSide: Story = () => {
       />
       <div>
         {(data?.users ?? []).map((item) => (
-          <div>{item.name}</div>
+          <div key={`${item.name}`}>{item.name}</div>
         ))}
       </div>
     </Column>
@@ -217,3 +217,55 @@ MakeYourOwn.parameters = {
     },
   },
 }
+
+const ChildWithPagination = ({ items, pageSize }) => {
+  const { page, totalPages, startIndex, endIndex, setPage } = usePagination({
+    totalItems: items.length,
+    pageSize,
+  })
+
+  return (
+    <>
+      <Row css={{ gap: '$2', mb: '$3' }}>
+        {items.slice(startIndex, endIndex).map((item) => (
+          <div key={`${item}`}>{item}</div>
+        ))}
+      </Row>
+      <Pagination page={page} onPageChange={setPage} count={totalPages} />
+    </>
+  )
+}
+
+export const PropChangeTest: Story = () => {
+  const [totalItems, setTotalItems] = useState(100)
+  const [pageSize, setPageSize] = useState(10)
+
+  const items = useMemo(
+    () => Array.from({ length: totalItems }, (v, i) => i),
+    [totalItems]
+  )
+
+  return (
+    <>
+      <Slider
+        labelFunction={(value) => `Items ${value}`}
+        value={[totalItems]}
+        onValueChange={(value) => setTotalItems(value[0])}
+      />
+      <Slider
+        labelFunction={(value) => `Page size ${value}`}
+        value={[pageSize]}
+        onValueChange={(value) => setPageSize(value[0])}
+      />
+      <ChildWithPagination items={items} pageSize={pageSize} />
+    </>
+  )
+}
+PropChangeTest.parameters = {
+  docs: {
+    description: {
+      story:
+        'A test story to check the values correctly update when the component props change.',
+    },
+  },
+}
diff --git a/src/usePagination/usePagination.test.ts b/src/usePagination/usePagination.test.ts
@@ -23,24 +23,26 @@ test('should start in given state', () => {
 
 test('should start using the provided start state', () => {
   const { result } = renderHook(() =>
-    usePagination({ page: 5, totalItems: 100, pageSize: 10 })
+    usePagination({ totalItems: 100, pageSize: 10 })
   )
   const {
     page,
     totalPages,
+    totalItems,
     startIndex,
     endIndex,
     pageSize,
     isPreviousDisabled,
     isNextDisabled,
   } = result.current
-  expect(page).toBe(5)
-  expect(totalPages).toBe(10)
-  expect(startIndex).toBe(40)
-  expect(endIndex).toBe(50)
+  expect(page).toBe(1)
   expect(pageSize).toBe(10)
+  expect(totalItems).toBe(100)
+  expect(totalPages).toBe(10)
+  expect(startIndex).toBe(0)
+  expect(endIndex).toBe(10)
   expect(isNextDisabled).toBe(false)
-  expect(isPreviousDisabled).toBe(false)
+  expect(isPreviousDisabled).toBe(true)
 })
 
 test('can set page', () => {
@@ -75,8 +77,14 @@ test('can set page', () => {
 
 test('can not set page too high', () => {
   const { result } = renderHook(() =>
-    usePagination({ page: 10, totalItems: 100, pageSize: 10 })
+    usePagination({ totalItems: 100, pageSize: 10 })
   )
+  expect(result.current.page).toBe(1)
+
+  act(() => {
+    result.current.setPage(10)
+  })
+
   expect(result.current.page).toBe(10)
 
   act(() => {
@@ -92,7 +100,7 @@ test('can not set page too high', () => {
 
 test('can not set page too low', () => {
   const { result } = renderHook(() =>
-    usePagination({ page: 1, totalItems: 100, pageSize: 10 })
+    usePagination({ totalItems: 100, pageSize: 10 })
   )
   expect(result.current.page).toBe(1)
 
@@ -109,27 +117,27 @@ test('can not set page too low', () => {
 
 test('can set page size', () => {
   const { result } = renderHook(() =>
-    usePagination({ page: 10, totalItems: 100, pageSize: 10 })
+    usePagination({ totalItems: 100, pageSize: 10 })
   )
 
   act(() => {
     result.current.setPageSize(20)
   })
 
-  expect(result.current.page).toBe(5)
+  expect(result.current.page).toBe(1)
   expect(result.current.pageSize).toBe(20)
 })
 
 test('can set total items', () => {
   const { result } = renderHook(() =>
-    usePagination({ page: 10, totalItems: 100, pageSize: 10 })
+    usePagination({ totalItems: 100, pageSize: 10 })
   )
 
   act(() => {
     result.current.setTotalItems(200)
   })
 
-  expect(result.current.page).toBe(10)
+  expect(result.current.page).toBe(1)
   expect(result.current.totalPages).toBe(20)
 })
 
@@ -164,3 +172,24 @@ test('can update query', () => {
 
   expect(result.current.query).toBe(previous)
 })
+
+test('can update total items', async () => {
+  const { result, rerender } = renderHook(usePagination, {
+    initialProps: { totalItems: 100, pageSize: 10 },
+  })
+  act(() => {
+    result.current.setPage(2)
+  })
+
+  expect(result.current.page).toBe(2)
+  expect(result.current.totalPages).toBe(10)
+  expect(result.current.startIndex).toBe(10)
+  expect(result.current.endIndex).toBe(20)
+
+  act(() => rerender({ totalItems: 200, pageSize: 15 }))
+
+  expect(result.current.page).toBe(2)
+  expect(result.current.totalPages).toBe(14)
+  expect(result.current.startIndex).toBe(15)
+  expect(result.current.endIndex).toBe(30)
+})
diff --git a/src/usePagination/usePagination.ts b/src/usePagination/usePagination.ts
@@ -15,6 +15,13 @@ export interface PaginationData {
   isPreviousDisabled: boolean
   /** The page size */
   pageSize: number
+  /** The total items */
+  totalItems: number
+}
+
+const defaultOptions = {
+  totalItems: 0,
+  pageSize: 20,
 }
 
 /**
@@ -24,14 +31,12 @@ export interface PaginationData {
  *
  */
 export function usePagination<T = void>({
-  totalItems: startTotalItems = 0,
-  pageSize: startPageSize = 20,
-  page: startPage = 1,
-  queryCallback = () => undefined as T,
+  queryCallback,
+  totalItems: propTotalItems = defaultOptions.totalItems,
+  pageSize: propPageSize = defaultOptions.pageSize,
 }: Partial<{
   totalItems: number
   pageSize: number
-  page: number
   queryCallback: (data: PaginationData) => T
 }> = {}): PaginationData & {
   query: T
@@ -46,9 +51,55 @@ export function usePagination<T = void>({
   /** set the number of items */
   setTotalItems: (totalItemsSize: number) => void
 } {
-  const [totalItems, setTotalItemsInternal] = useState(startTotalItems)
-  const [pageSize, setPageSizeInternal] = useState(Math.max(startPageSize, 1))
-  const [page, setPageInternal] = useState(Math.max(startPage, 1))
+  const [page, setPageInternal] = useState(1)
+
+  const [data, setDataInternal] = useState({
+    totalItems: Math.max(propTotalItems, 0),
+    pageSize: Math.max(propPageSize, 1),
+  })
+
+  const setData = useCallback(
+    (
+      newData: Partial<{
+        totalItems: number
+        pageSize: number
+      }>
+    ) => {
+      setDataInternal((current) => ({
+        pageSize: Math.max(1, newData.pageSize ?? current.pageSize),
+        totalItems: Math.max(0, newData.totalItems ?? current.totalItems),
+      }))
+    },
+    [setDataInternal]
+  )
+  const setTotalItems = useCallback(
+    (totalItems: number) => {
+      setData({ totalItems })
+    },
+    [setData]
+  )
+
+  const setPageSize = useCallback(
+    (pageSize: number) => {
+      setData({ pageSize })
+    },
+    [setData]
+  )
+
+  const savedCallback = useRef<(data: PaginationData) => T>(
+    queryCallback ?? (() => undefined as T)
+  )
+
+  useEffect(() => {
+    setData({
+      totalItems: propTotalItems,
+      pageSize: propPageSize,
+    })
+  }, [propTotalItems, propPageSize, setData])
+
+  useEffect(() => {
+    savedCallback.current = queryCallback ?? (() => undefined as T)
+  }, [queryCallback])
 
   const {
     totalPages,
@@ -57,18 +108,10 @@ export function usePagination<T = void>({
     isNextDisabled,
     isPreviousDisabled,
   } = useMemo(
-    () => getDerivedData(totalItems, pageSize, page),
-    [page, pageSize, totalItems]
+    () => getDerivedData(data.totalItems, data.pageSize, page),
+    [page, data]
   )
 
-  const setTotalItems = useCallback((newTotalItems: number) => {
-    setTotalItemsInternal(Math.max(0, newTotalItems))
-  }, [])
-
-  const setPageSize = useCallback((newPageSize: number) => {
-    setPageSizeInternal(Math.max(1, newPageSize))
-  }, [])
-
   const setPage = useCallback(
     (newPage: number) => {
       setPageInternal(Math.max(1, Math.min(newPage, totalPages)))
@@ -84,34 +127,15 @@ export function usePagination<T = void>({
     setPage(page - 1)
   }, [page, setPage])
 
-  useEffect(() => {
-    return () => {
-      setTotalItems(startTotalItems)
-    }
-  }, [startTotalItems, setTotalItems])
-
-  useEffect(() => {
-    return () => {
-      setPageSize(startPageSize)
-    }
-  }, [startPageSize, setPageSize])
-
   useEffect(() => {
     setPageInternal((page) => Math.max(1, Math.min(page, totalPages)))
   }, [totalPages])
 
-  const savedCallback = useRef<(data: PaginationData) => T>(queryCallback)
-
-  // Remember the latest callback.
-  useEffect(() => {
-    savedCallback.current = queryCallback
-  }, [queryCallback])
-
   const query = useMemo(() => {
     const callback = savedCallback.current
     return callback({
       page,
-      pageSize,
+      ...data,
       totalPages,
       startIndex,
       endIndex,
@@ -120,7 +144,7 @@ export function usePagination<T = void>({
     })
   }, [
     page,
-    pageSize,
+    data,
     totalPages,
     startIndex,
     endIndex,
@@ -130,7 +154,8 @@ export function usePagination<T = void>({
 
   return {
     page,
-    pageSize,
+    pageSize: data.pageSize,
+    totalItems: data.totalItems,
     totalPages,
     startIndex,
     endIndex,
