nuwave / lighthouse

A framework for serving GraphQL from Laravel
https://lighthouse-php.com
MIT License
3.36k stars 438 forks source link

Use an actual cursor for relay style pagination #311

Open spawnia opened 6 years ago

spawnia commented 6 years ago

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.

yaquawa commented 5 years ago

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 https://github.com/laravel/ideas/issues/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.

simonenj commented 4 years ago

What is the best way to integrate lampager and lighthouse?

spawnia commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

Cursor pagination has landed in Laravel

https://github.com/laravel/framework/pull/37216

spawnia commented 3 years ago

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 commented 2 years ago

@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 commented 2 years ago

@dbbk see https://laravel.com/docs/pagination#cursor-pagination

jaulz commented 1 year ago

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?