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

Use an actual cursor for relay style pagination #311

Open
spawnia opened this issue Sep 10, 2018 · 14 comments
Open

Use an actual cursor for relay style pagination #311

spawnia opened this issue Sep 10, 2018 · 14 comments
Labels
enhancement A feature or improvement

Comments

@spawnia
Copy link
Collaborator

spawnia commented Sep 10, 2018

While the implementation for Relay/Connection pagination is compliant with the Relay spec, it does not actually implement cursor based pagination but rather offset-based pagination in disguise. Read about the difference here https://www.sitepoint.com/paginating-real-time-data-cursor-based-pagination/

This is a hard problem to solve, since there is no easy way to reliable get the next page after a cursor, unless we are basing the sort order on an ordered index.

https://stackoverflow.com/questions/26188519/handling-paging-with-changing-sort-orders

Since i do not think we can find a generalized solution that fits all use cases, we might
limit the scope of Lighthouse to providing just a basic API for the user to implement their own, cursor-based pagination resolver. We can take care of providing helpers for encoding/decoding the cursor, but apart from that, the use cases can be so vastly different we can not possibly cover them all.

Maybe something like https://github.com/lampager/lampager-laravel might provide us with a nice framework to build cursor pagination.

@spawnia spawnia added the bug An error within Lighthouse label Sep 10, 2018
@spawnia spawnia added the enhancement A feature or improvement label Oct 9, 2018
@yaquawa
Copy link
Contributor

yaquawa commented Nov 23, 2018

Hi @spawnia

I found the same issue when I was working on #368.
Generally speaking, offset pagination is evil in most of cases…
Actually I submitted a idea at here laravel/ideas#1347 so we can easily implement the cursor based pagination.

I came up with numbers of ideas a while ago.

for the relation directives, it's rather easy to give a generic solution something like this:

  1. find the cursorKeys property in the related model
  2. if no one exist fallback to the the defaultCursorKeys key in config

for @paginate directive we can:

  1. find the cursorKeys argument of @paginate
  2. if no one exist fallback to the the defaultCursorKeys key in config

(or instead of the cursorKeys maybe orderByKeys is more 'generic')

anyway, I think both of above cases can be done with just a little tweaking of the existing code.

@nuwave nuwave deleted a comment from EdwardNelson Apr 30, 2020
@spawnia spawnia changed the title Relay/Connection pagination does not actually query via Cursor Use an actual cursor for relay style pagination Apr 30, 2020
@spawnia spawnia removed the bug An error within Lighthouse label Apr 30, 2020
@simonenj
Copy link

What is the best way to integrate lampager and lighthouse?

@spawnia
Copy link
Collaborator Author

spawnia commented Jun 21, 2020

What is the best way to integrate lampager and lighthouse?

Create an open source package or add a PR to Lighthouse. You will have to put in some effort, but doing it in the open means you get valuable community feedback and contributions.

@benkingcode
Copy link

What if I have data that is not ordered in a predictable way? Like a table of rows sorted by a column that changes over time. It sounds like neither offset-based or cursor-based options would work here.

@benkingcode
Copy link

Thinking about this some more, what if the paginator's cursor was a base64-encoded array of the IDs of results already returned (e.g. 1, 2, 3), then when the user requests the next page with the cursor, the paginator can essentially make the query WHERE id NOT IN (1, 2, 3)

Any thoughts on potential pitfalls with this?

@spawnia
Copy link
Collaborator Author

spawnia commented Jul 21, 2020

@dbbk that would make the client side state handling much harder, it would have to keep track of all pages that were seen. If the goal is to have the clients see every possibly entry over a somewhat extended period of time, that might be a correct approach. I doubt it is a common need and probably way to complicated for most use cases.

@benkingcode
Copy link

Wouldn't the cursor be able to generate itself completely server side? For example, first query of 10 results it is just the array of those 10 IDs. Then, on the next query for the second page, it takes the existing cursor and appends the new set of 10 IDs so now it has 20.

So it shouldn't theoretically require any client side computation?

@spawnia
Copy link
Collaborator Author

spawnia commented Jul 21, 2020

  • The client would not able to navigate back to a previous page easily.
  • Passing a mutable list back and forth introduces state.
  • Efficiency for large lists might be horrible.

That said, there are some merits to your idea. The issues might be solvable. I think the best way forward would be for you to try your hands on an implementation. If it is simple enough and works for the general case, we can further discuss inclusion or integration with Lighthouse.

@benkingcode
Copy link

Sounds good! I'll give it a go, thanks for the tips. I don't think there is currently a plug-in way to provide new paginator types to Lighthouse, so I'll have to fork, right?

@tlaverdure
Copy link
Collaborator

Cursor pagination has landed in Laravel

laravel/framework#37216

@spawnia
Copy link
Collaborator Author

spawnia commented May 11, 2021

We have to keep in mind that cursor pagination in Laravel is subject to some limitations which our current pseudo-cursor implementation does not have. This means that we cannot simply switch out the implementation without making breaking changes.

I think we should try and make the upgrade path as smooth as possible, preferrably not coupling the change to a major release of Lighthouse. We could offer the Laravel native actual cursor pagination as an opt-in configuration option, and make it the default in the next major version of Lighthouse.

@benkingcode
Copy link

@spawnia I'm interested in taking up this work as I have a new project with a requirement for real cursors. Could you expand a bit on what limitations I'd need to look out for?

@spawnia
Copy link
Collaborator Author

spawnia commented Feb 7, 2022

@jaulz
Copy link
Contributor

jaulz commented Mar 28, 2023

For anyone stumbling across the same I just wrote a directive which I think works as far as I can see:
https://gist.github.com/jaulz/1b0a8c8f0841668357f274a746943dfe

It can even navigate using "before" and "after". It's mostly a copy of the paginate directive (thanks for that!) and doesn't have that much additional magic:

      $first = $args['first'] ?? $this->defaultCount();
      $last = $args['last'] ?? $this->defaultCount();
      $after = rescue(
        fn() => Cursor::fromEncoded($args['after'] ?? ''),
        null,
        false
      );
      $before = rescue(
        fn() => Cursor::fromEncoded($args['before'] ?? ''),
        null,
        false
      );

      ...

      // Retrieve result
      if ($before) {
        $paginator = $builder->cursorPaginate(
          $last,
          ['*'],
          'cursor',
          new Cursor(
            collect($before->toArray())
              ->forget('__pointsToNextItems')
              ->toArray(),
            false
          )
        );
      } else {
        $paginator = $builder->cursorPaginate($first, ['*'], 'cursor', $after);
      }

Also, it returns the builder itself:

      return [
        'builder' => $builder,
        'paginator' => $paginator,
      ];

which lets me later lazily retrieve the total count:

   'total' => fn () => $builder->count(),

Unfortunately, I am limited in time right now to work on a proper PR but it would be really great to see it incorporated in the library itself. @dbbk maybe you could give it a try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or improvement
Projects
None yet
Development

No branches or pull requests

6 participants