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

Add pagination support to usePromise, useCachedPromise and useFetch. #25

Merged
merged 17 commits into from
Mar 7, 2024

Conversation

sxn
Copy link
Contributor

@sxn sxn commented Mar 4, 2024

Context

We're working on adding pagination support to Grid and List via a new pagination prop. To support users in more easily taking advantage of pagination, this PR updates usePromise, useCachedPromise and useFetch to have pagination support. This means a couple of things:

  1. The hooks' return signature is expanded to also include pagination: { hasMore: boolean, pageSize: number, onLoadMore: () => void }, which can be directly passed to the List or Grid component
  2. The hooks' signature is expanded to accept "paginated" variants of their respective arguments.

Changes

For usePromise and useCachedPromise

the hooks' first argument can be not just a function that returns a promise, but a function that returns a paginated promise. A paginated promise in this case is a function that receives pagination info and returns a promise. It means going from

const { data, isLoading } = usePromise(
  async (searchText: string) => { 
    const data =  // data can be anything
    return data;
  },
  [searchText]
)

to

const { data, isLoading, pagination } = usePromise(
  (searchText: string) => async (options: { page: number; lastItem?: string }) => { 
    const data =  // data _needs_ to be an array
    const hasMore = 
    return { data, hasMore };
  },
  [searchText]
);

Particular to useCachedPromise: when paginating, only the first page for each pagination instance is cached.

For useFetch

the hook's first argument can be not just a RequestInfo, but a paginated request info.
Similar to above, this means going from:

const { data, isLoading } = useFetch(url);

to

type FetchResponse = {  } // whatever the API call's expected return type is
type Item = {  }
const { data, isLoading, pagination } = useFetch(
  (options: { page: number, lastItem?: any }) => url,
  {
    mapResult: (result: FetchResponse) {
      const data =  // data _needs_ to be an array
      const hasMore = 
      return { data, hasMore };
    }
  }
);

The main difference between usePromise / useCachedPromise and useFetch is that the latter requires a mapResult option to be passed, in addition to the paginated request info, in order to tell the hook whether it should try to paginate further, and to have a chance to "process" the data returned by the API, in case e.g. the API returns something like { success: boolean, results: Item[] }.

Other changes

In order to support these changes, I've had to make a couple more changes:

  • onData now receives not just the data, but also a second argument: the pagination options.
    • the reason for this change is: pagination "data", i.e. page, latest hasMore, lastItem and the data accumulated from the current pagination instance, it's all stored in usePromise. However, because useCachedPromise only caches the first page, we needed a way to figure out what page we're currently at.
  • useCachedPromise and useFetch accept a cacheKeySuffix option, which, as the name implies will be added to the cacheKey that useCachedPromise already generates internally. Because useFetch builds on top of useCachedPromise, and because useCachedPromise uses the function & the args to generate the cache key – this wasn't enough to differentiate, when using e.g. useFetch((searchText: string) => async (options) => "https://example.com/?page= " + options.page + "&searchText=" + searchText, [searchText]), between searchText being empty and not, i.e.:
Screen.Recording.2024-03-04.at.15.00.05.mov

Screenshots

usePromise:

Screen.Recording.2024-03-04.at.14.59.01.mov

useCachedPromise:

Screen.Recording.2024-03-04.at.14.59.36.mov

useFetch:

Screen.Recording.2024-03-04.at.15.07.00.mov

@sxn sxn requested a review from mathieudutour March 4, 2024 14:13
src/useCachedPromise.ts Outdated Show resolved Hide resolved
src/useFetch.ts Outdated Show resolved Hide resolved
tests/src/fetch-paginated.tsx Outdated Show resolved Hide resolved
tests/src/cached-promise-paginated.tsx Outdated Show resolved Hide resolved
Copy link
Member

@mathieudutour mathieudutour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, the only thing missing is the peerDependency update in the package.json (which I assume you'll have to wait until the release?)

@mathieudutour
Copy link
Member

ah you're missing the doc updates as well

@sxn
Copy link
Contributor Author

sxn commented Mar 6, 2024

yeah we'll need to wait merging this until 1.69.0 is out. But I can update the dependencies already in both the utils and the tests, as well as the version.

The bigger leftover is the docs – working on that now. 😸

@sxn sxn requested a review from mathieudutour March 6, 2024 14:56
@sxn sxn merged commit 3859088 into main Mar 7, 2024
1 check passed
@sxn sxn deleted the pagination branch March 7, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants