Skip to content

Conversation

@rodrigopedra
Copy link
Contributor

When creating a Paginator from both Illuminate\Database\Query\Builder and Illuminate\Database\Eloquent\Builder, the $total property uses the value() helper, as it can be a \Closure.

But the $perPage property replicates the same logic as the value() helper, as below:

function value($value, ...$args)
{
return $value instanceof Closure ? $value(...$args) : $value;
}

This PR:

  • Changes the $perPage calculation to use the value() helper

Note:

On Illuminate\Database\Eloquent\Builder, the previous code used the ternary shorthand operator (?:) instead of the null coalescing operator (??) to defer checking the model for the $perPage value, in case it wasn't defined.

It would allow a call like this:

Model::paginate(0);

To make the Model@getPerPage() method to be called, which would technically be a breaking change.

I opted to use the null coalescing operator (??) as $total uses it, and as, IMO, it is very unlikely someone to be calling Model::paginate(...) with a zero $perPage value.

But I can revert to the ternary shorthand operator (?:) if we want to avoid any breaking change.

@taylorotwell taylorotwell merged commit 2155a63 into laravel:11.x Feb 17, 2025
46 checks passed
crynobone added a commit that referenced this pull request Feb 18, 2025
fixes #54679 Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
taylorotwell pushed a commit that referenced this pull request Feb 19, 2025
fixes #54679 Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants