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

No documentation on how to add pagination type #2274

Open
robertdeboer opened this issue Jan 14, 2023 · 5 comments
Open

No documentation on how to add pagination type #2274

robertdeboer opened this issue Jan 14, 2023 · 5 comments
Labels
discussion Requires input from multiple people

Comments

@robertdeboer
Copy link

Describe the bug

There is no way to integration a custom pagination type. You can add a custom resolver to add custom builder instance but cannot add a new pagination type. This is useful features as there are 1) multiple ways to do pagination 2) external factors that my required a custom or modified pagination type such as DB design, system architecture, etc.

Expected behavior/Solution

A way to integration a custom pagination type. I have added one by altering the source pagination files but that is not ideal. I believe requiring it to be an extension of a LengthAwarePagination type to be fair since it would only require adding the new type as an available type type and being able to call it as a method available on the builder.

Steps to reproduce

  1. Look at the documentation
  2. There is documentation on how to add a resolver but that does not return the pagination, only a pre modified Builder instance

Output/Logs

N/A

Lighthouse Version

v5.70.0

I have done this manually by modifying the following src/Pagniation files:

  • PaginationType
  • PaginationServiceProvider
  • PaginationArgs
@spawnia
Copy link
Collaborator

spawnia commented Jan 17, 2023

It is unclear to me what exactly you would like to see changed and how to achieve it. Perhaps you can just propose a pull request?

@spawnia spawnia added the discussion Requires input from multiple people label Jan 17, 2023
@robertdeboer
Copy link
Author

@spawnia

I'm sorry, I hope this helps.

Problem:

There is no way currently (that I can see at least in the code or documentation) to add a new pagination type. There is documentation on how to point to a custom class that will return a Query Builder but it must return a Builder and so the builder cannot implement any custom pagination.

Why:

In my case I wanted to implement a different form of limit/offset pagination than the standard Laravel one. There are also Laravel packages implementing different pagination methods. However, there is no way to add this new pagination type to the list of available types when declaring the schema. I was able to do it by altering the source files in my vendor directory for testing purpose but this not optimal.

How:

Since my pagination method exposed itself as a method of the builder, it fit in nicely with how other pagination types are already included. I simply needed to update the default schema and add some additional logic to know when and what method to call based on a new type value

I would be more than happy to help work on this. However, I wanted to bring this up first to see if there was a valid reason not to do so or if this was already in the works. I think with some updates to the Paginator implementation and the introduction of a PaginatorType this could be done.

@spawnia
Copy link
Collaborator

spawnia commented Jan 31, 2023

Pagination is already a quite complex part of Lighthouse, so I am somewhat apprehensive about adding even more to it. However, I also see the value in making it easier for end users and have Lighthouse do the heavy lifting.

I personally do not need anything like this so I do not plan to work on this, but am open to doing it as paid work or to reviewing a pull request that adds it.

@robertdeboer
Copy link
Author

I will start working on something but here is my high level idea.

  • Remove the actual implementation of pagination from the pagination type - this simply becomes a wrapper for calling the appropriate functionality
  • Add a new type: PaginationType. This represents a type of pagination and provides the pagination implementation. In essence the type enum of Paignation transitions from a simple enum list to a dynamical built list of all available pagination types available to GraphQL at that time
  • I think to start and to make it the easiest to implement, the actual pagination should be available as a method on the builder as it is done now

@spawnia
Copy link
Collaborator

spawnia commented Feb 4, 2023

I could see there being a config option that maps the names of pagination types to classes that implement their behaviour. The classes would have to implement a common interface and define runtime behaviour, schema manipulation steps, etc.

[
    'paginator' => PaginatorPagination::class,
    'simple' => SimplePagination::class,
    'custom' => YourOwnPagination::class,
]

Then, this configuration will need to be tied in with existing directives that use pagination - so @paginate and the relation directives such as @hasMany.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Requires input from multiple people
Projects
None yet
Development

No branches or pull requests

2 participants