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

[11.x] Adding the withQueryString method to the paginate and simplePaginate interfaces. #54460

Conversation

dvlpr91
Copy link
Contributor

@dvlpr91 dvlpr91 commented Feb 4, 2025

Adding the withQueryString method to the paginate and simplePaginate interfaces.

@dvlpr91 dvlpr91 changed the title adding-the-withQueryString-method-to-the-paginate-and-simplePaginate-… [11.x] Adding the withQueryString method to the paginate and simplePaginate interfaces. Feb 4, 2025
…interfaces

- According to the documentation, `paginate` has a `withQueryString` method.
  - See, https://laravel.com/docs/11.x/pagination#appending-query-string-values
- Not only `cursorPaginate`, but also `paginate` and `simplePaginate` should implement `withQueryString` and work with it.
@dvlpr91 dvlpr91 force-pushed the adding-the-withQueryString-method-to-the-paginate-and-simplePaginate-interfaces branch from 92e792b to f2365d7 Compare February 4, 2025 06:32
@jmarcher
Copy link
Contributor

jmarcher commented Feb 4, 2025

This is a breaking change

@crynobone
Copy link
Member

Please send this to master branch.

@crynobone crynobone closed this Feb 4, 2025
@dvlpr91
Copy link
Contributor Author

dvlpr91 commented Feb 4, 2025

@jmarcher, @crynobone

Is this really a compatibility breaking change?

The withQueryString method is already implemented in abstract classes.

They are just added to the interface.

@crynobone
Copy link
Member

Any custom implementation of Paginator interface that doesn't implement wihtQueryString() will be broken if this is accepted. Adding new method to interface should only be added for major release.

dvlpr91 added a commit to dvlpr91/framework that referenced this pull request Feb 4, 2025
- According to the documentation, `paginate` has a `withQueryString` method.
  - See, https://laravel.com/docs/pagination#appending-query-string-values
- Not only `cursorPaginate`, but also `paginate` and `simplePaginate` should implement `withQueryString` and work with it.

ref: laravel#54460
dvlpr91 added a commit to dvlpr91/framework that referenced this pull request Feb 4, 2025
- According to the documentation, `paginate` has a `withQueryString` method.
  - See, https://laravel.com/docs/pagination#appending-query-string-values
- Not only `cursorPaginate`, but also `paginate` and `simplePaginate` should implement `withQueryString` and work with it.

ref: laravel#54460
taylorotwell pushed a commit that referenced this pull request Feb 5, 2025
- According to the documentation, `paginate` has a `withQueryString` method.
  - See, https://laravel.com/docs/pagination#appending-query-string-values
- Not only `cursorPaginate`, but also `paginate` and `simplePaginate` should implement `withQueryString` and work with it.

ref: #54460
taylorotwell pushed a commit to illuminate/contracts that referenced this pull request Feb 6, 2025
- According to the documentation, `paginate` has a `withQueryString` method.
  - See, https://laravel.com/docs/pagination#appending-query-string-values
- Not only `cursorPaginate`, but also `paginate` and `simplePaginate` should implement `withQueryString` and work with it.

ref: laravel/framework#54460
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.

3 participants