amrnn90 / laravel-cursor-paginator

Cursor pagination for Laravel
MIT License
19 stars 7 forks source link

Conflict with cursorPaginate() Laravel native #27

Open lucianobosco opened 3 years ago

lucianobosco commented 3 years ago

On the last Laravel release, they've implemented cursor-based pagination. As I see the returning data does not contain the first/last page as in this library, so I want to keep using this. Unfortunately, there seems to be a conflict with the pagination method since both use cursorPaginate(), therefore when calling this method it uses de Laravels default one instead of the one from this library. I'm not sure if I'm clear enough, but I'm just wondering if there is a way to rename the cursorPaginate() method from this library ASAP.

Any advice will be appreciated

Dwingloo commented 3 years ago

@lucianobosco This is true, we had to downgrade on a couple of projects down to "laravel/framework": "8.40" to avoid the conflict between the new release in 8.41 default cursorPaginate() method from laravel and the one in the package, hopefully @amrnn90 could find a solution for this, we can't keep projects downgraded like this.

lucianobosco commented 3 years ago

@lucianobosco This is true, we had to downgrade on a couple of projects down to "laravel/framework": "8.40" to avoid the conflict between the new release in 8.41 default cursorPaginate() method from Laravel and the one in the package, hopefully @amrnn90 could find a solution for this, we can't keep projects downgraded like this.

I would recommend you to fork this project and replace the cursorPaginate() method name directly in your forked repository instead of downgrading Laravel. You can take a look at my fork: https://github.com/lucianobosco/laravel-cursor-paginator where I've replaced the method by myCursorPaginate() Please note that you need to publish to packagist if you want to install your own library through composer. Also please do not use my fork since I will delete it as soon as this is solved by @amrnn90 , or at least be careful and remember that I will no maintain the fork.

Edit: this is my packagist link if you want it for testing purposes https://packagist.org/packages/lucianobosco/laravel-cursor-paginator

composer require lucianobosco/laravel-cursor-paginator

DeepDiver1975 commented 3 years ago

FYI: I added my own service provider and use my own name of the macro:

Maybe this is of help for anybody :shrug:


<?php

namespace App\Providers;

use Amrnn\CursorPaginator\Macro as PaginatorMacro;
use App\Exceptions\PaginationException;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Query\Builder as QueryBuilder;
use Illuminate\Http\Request;
use Illuminate\Support\ServiceProvider;

class PaginationServiceProvider extends ServiceProvider
{
    public function boot(): void
    {
        $macro = function ($perPage = null, $options = []) {
            $request = resolve(Request::class);

            return (new PaginatorMacro($request->all(), $perPage, $options))
                ->process($this);
        };

        QueryBuilder::macro('paginateWithCursor', $macro);
        EloquentBuilder::macro('paginateWithCursor', $macro);
    }
}
lucianobosco commented 3 years ago

@DeepDiver1975 could you please elaborate? The macro changes the method name of this package so instead of using cursorPaginate() you can use paginateWithCursor()? I have no knowledge about macros, so I need to ask: how this provider/macro is binded to this package? Thanks in advance

DeepDiver1975 commented 3 years ago

@lucianobosco hey ....

There is no bigger magic in this .... I just copied the provider to my own code base and register the macros under the name I desired ...... refs https://github.com/amrnn90/laravel-cursor-paginator/blob/master/src/PaginatorServiceProvider.php

DeepDiver1975 commented 3 years ago

FYI: I am maintaining my own fork by now for laravel 8 .... https://github.com/DeepDiver1975/laravel-cursor-paginator

Dwingloo commented 3 years ago

@DeepDiver1975 @lucianobosco We quit using the cursor paginate package for laravel, and went with the Laravel Native cursor pagination, its done better in the Native version for laravel, we also compared queries needed to hit the database and found out Laravel Native cursor paginate is better, saving us 1-3 queries on small/complex queries.

lucianobosco commented 3 years ago

@Dwingloo unless I missed something, the native cursorPaginate() doesn't return first/last link data, right? That's why you are saving 1-3 queries because of the missing count aggregates for those links generation.

lucianobosco commented 2 years ago

Hey guys, is anyone else facing some issues when using Subquery Ordering. Basically, the parent Model column is not recognized. I guess this package is not longer maintained and I will have to use native Laravel cursor pagination

Dwingloo commented 2 years ago

@lucianobosco yes, you are correct, switch to the native Laravel method instead.