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

[RFC] support pagination using cursors #1510

Closed
yonahforst opened this issue Feb 9, 2018 · 20 comments
Closed

[RFC] support pagination using cursors #1510

yonahforst opened this issue Feb 9, 2018 · 20 comments

Comments

@yonahforst
Copy link
Contributor

I'm in the middle of writing a client for AWS DynamoDB. One of the problems is that the DB uses cursor-based pagination (is that what it's called?)

I'm using a custom reducer to keep track of the previous + next keys, but I need a way to pass those from the <Pagination/> component to the client.

At first I thought it would be a good idea to allow the page param to be an object, but that got messy quickly. there are several places where it's assumed to be an integer.

However, filters can be objects and maybe that's a better fit anyway. What do you guys thinking about exposing setFilters to the <Pagination/> component?

It's working great for me and I have a PR lined up to submit.

@fzaninotto
Copy link
Member

Good idea. And while we are at it, we could expose a setPerPage method, allowing the pagination to change the number of rows per page.

@yonahforst
Copy link
Contributor Author

Well, perhaps I spoke too soon when I said 'working great'.

I don't think my current implementation is suitable as generic DynamoDB client that anyone can use (which I would very much like it to be).

My main problem is that I don't know the prev/next cursor keys until the server responds with the contents of the current page. Somehow I need to pass those key to <Pagination/>. Now I'm doing it with a custom reducer and making it a ConnectedComponent but it's really ugly and requires a bunch of config.

I'm actually considering moving back to page and perPage and just maintaining a list of ids locally to the client. (DynamoDB cursors are just ids, so it's easy to figure out prev/next)

What would you guys recommend?

another option would be just to have a custom saga that updates the filters with the next/prev keys upon CRUD_GET_LIST_SUCCESS and then read them in <Pagination/> without connecting it

@yonahforst
Copy link
Contributor Author

using page and perPage is working well for me.
https://github.com/abiglobalhealth/aor-dynamodb-client

still working on supporting CREATE, UPDATE, DELETE

@mrcasablr
Copy link

@yonahforst repo link is a 404

@yonahforst
Copy link
Contributor Author

Whoops. Had to make it public. Thanks for catching that

@djhi
Copy link
Contributor

djhi commented Feb 11, 2018

Can you make a PR to add it to the ecosystem section in the documentation ?

@yonahforst
Copy link
Contributor Author

Done. Pr #1518

@garethsb
Copy link

We'd love to see cursor-based pagination support in React Admin.

As @yonahforst says above, what's needed is for a way for the dataProvider to return the prev/next (and optionally first/last) cursors/URLs with the returned data for GET_LIST and GET_MANY_REFERENCE, e.g. { data: [...], cursors: { prev: ..., next: ..., } } rather than { data, [...], total: 42 }.

How this is best hooked up to the Pagination component is debatable. Would it would be best for the Pagination component to to have access to the whole data (I don't think it does now?) and do e.g. setPage(data.cursors.next)?

Just noticed #1787, which proposes something similar. @fzaninotto, am I on the right track?

@fzaninotto
Copy link
Member

Pagination does not receive the data key of the dataProvider response, you're right. But it receives the resource data from the store (an array of objects already fetched) and the list of ids that is deduced from the dataProvider data, so it's feasible to recompose the initial data variable.

Changing the GET_LIST return value format is a breaking change that we won't address in the near future, so I recommend that you write a custom Pagination component using the cursor/URL form the data.

@garethsb
Copy link

Thank you very much for the response.

But it receives the resource data from the store (an array of objects already fetched) and the list of ids that is deduced from the dataProvider data, so it's feasible to recompose the initial data variable.

I think you're suggesting adding the cursors to one or more of the individual returned resources' data? In many APIs with cursor-based pagination, the response does not return a cursor per resource, but only cursors that identify the positions at the start/end of the current page. There may be cursors for the previous or next page, even when the page itself is empty. So e.g. associating the cursors with one of the resources may be possible sometimes but not in the general case.

I recommend that you write a custom Pagination component using the cursor/URL form the data.

Do you suggest this should use the dataProvider directly, rather than try to (ab)use the setPage mechanism?

Thanks!

@isumix
Copy link

isumix commented Jan 20, 2021

The offset & limit or page & perPage based pagination is broken by design, unless we never change our data. So I'd suggest streamline this for react-admin.

Meanwhile, I think adding filter like {after: 'some_id'} could help.

@fzaninotto, what do you think about it?

PS: there is a comprehensive pagination spec I like.

@jtomaszewski
Copy link
Contributor

Problem with filters is that they are memoized if the user goes out and back to the list view. I managed to put the page and cursor param in the filters, and it works; but now if the user goes back to the list view, they end up on the "last page they've been browsing", instead of starting with page 1 again.

@jtomaszewski
Copy link
Contributor

For now, we got around the issue differently; we store the cursor string value in page param. However, we needed to copy & paste useListController and useListParams files just to remove getNumberOrDefault from https://github.com/marmelab/react-admin/blob/master/packages/ra-core/src/controller/useListParams.ts#L373 , so that it doesn't try to convert that param to a number.

I wonder how could we change the implementation of those hooks, so that it allows for different format of pagination params, without having to entirely fork those hooks?

@fzaninotto
Copy link
Member

the API for getList is radically different when we use cursors:

  • getList request: getList('posts', { last: id, after: 10 }) instead of getList('posts', { page: 1, perPage: 10 })
  • getList response: { data: [], pageInfo: { hasNextPage: true, endCursor: id1, hasPrevPage: true, startCursor: id2 }) instead of { data: [], total: 123 }
  • listController response: { hasPrevPage, goToPrevPage, hasNextPage, goToNextPage } instead of { page, setPage }

However, using typeScript unions, this can be added in a backwards-compatible fashion.

We will not address this in 4.0, but maybe in a future minor release.

@fzaninotto
Copy link
Member

We've added Partial Pagination in react-admin 4.0: #7120

@garethsb
Copy link

@fzaninotto in your comment above you suggested an API with endCursor and startCursor in the pageInfo. How should this be done with the new partial pagination support?

@jvliwanag
Copy link

Since partial pagination doesn't support cursors yet as what's needed here, should we reopen this one? Or better to create a new issue to track?

@fzaninotto
Copy link
Member

Sure, cursor-based pagination would be nice. But we don't keep feature requests open if nobody in the community is willing to contribute an implementation. This is an open-source project, not a free development workshop.

If the core team needs it for on of our projects, we'll implement it, and possibly open-source it. We'll welcome any community implementation.

If you need this feature and you can't implement it yourself, please sponsor its development.

@owenbendavies
Copy link

The company I work at are using a GraphQL api that uses cursors rather than per page pagination.
It also uses infinite scroll so back / forward / selecting a specific page isn't needed.

Currently the contractors who set up our frontend created their own custom list component to support this while re-using most the other react admin components from the list component.

They did this quickly, and a little bit of a hack to get it out quickly as we had a deadline.

We are also using react-admin Enterprise Edition but I don't think any of those components are used by the list.

We may be interested in getting this solved properly in react-admin as we want as little custom code in our project as possible, one of the main reasons for using react-admin.

So we don't waste time and effort:

  1. Is this already being worked on for a future release?
  2. If not would we able to commission Marmelab to implement this as you will know the code base better than our contractors, @fzaninotto
  3. If not we may dedicate resource to implementing this ourselves, if so is there any existing partial work done on this to help us save time

Thank you,
Owen

@fzaninotto
Copy link
Member

Hi Owen,

No, there is no ongoing implementation for cursor-based pagination

We'd be happy to work on it for you. Can you drop an email to our sales jeremie@marmelab.com to continue the conversation?

Cheers,

François

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants