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

feat: add support for generic paginators #1372

Merged
merged 10 commits into from Nov 4, 2022

Conversation

erikgaal
Copy link
Contributor

@erikgaal erikgaal commented Sep 14, 2022

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Changes

This PR adds support for generic paginators.

Before:

$users = User::query()->paginate(); // LengthAwarePaginator
$users->items(); // array

After:

$users = User::query()->paginate(); // LengthAwarePaginator<User>
$users->items(); // array<User>

Breaking changes

@erikgaal erikgaal changed the title feat: add support for generic ga feat: add support for generic paginators Sep 14, 2022
stubs/Contracts/Pagination.stub Outdated Show resolved Hide resolved
Copy link
Collaborator

@canvural canvural left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing. Other than that it looks good!

@@ -28,8 +28,8 @@ abstract class AbstractPaginator implements \Illuminate\Contracts\Support\Htmlab
/**
* @template TValue
*
* @implements \ArrayAccess<mixed, TValue>
* @implements \IteratorAggregate<mixed, TValue>
* @implements \ArrayAccess<array-key, TValue>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this class implements ArrayAccess we should add the offset* methods here with the correct return/parameter types. This will let PHPStan know User::paginate(10)[3] returns User An example can be seen here.

Also for the other classes below + tests for this 👍🏽

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! ✅

@canvural
Copy link
Collaborator

That's great. Can you also resolve the conflict? Then if the CI is green, we can merge this!

@canvural canvural merged commit 42eb41b into larastan:master Nov 4, 2022
@canvural
Copy link
Collaborator

canvural commented Nov 4, 2022

Errors from monicahq/chandler looks legit.

Thank you!

@erikgaal erikgaal deleted the pagination-generics branch November 4, 2022 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants