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

fix: compatability to Laravel 11.17 #90

Merged
merged 7 commits into from
Jul 31, 2024
Merged

Conversation

innocenzi
Copy link
Contributor

Fixes #89

Since Laravel v11.17 (laravel/framework#52147), whereLike and orWhereLike exist and are no longer compatible with this package's version of these methods.

This pull request adapts their definitions (and unfortunately, behavior) to work with the latest version of Laravel. The caseInsensitive parameter has been renamed to caseSensitive and its logic updated to be the same as Laravel.

This is a draft PR because I couldn't run tests on my computer, and documentation regarding the changes is missing.

THIS IS A BREAKING CHANGE.

@tpetry
Copy link
Owner

tpetry commented Jul 24, 2024

Wow. Thats a tricky change. Because Laravel‘s behaviour is differently to the one I declared before. So breaking existing apps would be very easy.

But using a new method name and throwing an exception for the old one would also break apps switching to this driver in the future.

I may have to really go the way and ”backporting“ this behavioural change of Laravel. 🫤 But I could add a PHPStan to trigger an error when the case sensitive parameter is not used.

@innocenzi
Copy link
Contributor Author

Unfortunately I think the only option is to make a major (fortunately this package is still 0.x) and properly document the changes.

This is unfortunate, if the laravel/framework#52147 has been caught before being merged, its behavior could have been aligned to this package :(

@tpetry tpetry marked this pull request as ready for review July 31, 2024 09:15
@tpetry tpetry changed the title fix: use actual namespace instead of alias fix: compatability to Laravel 11.17 Jul 31, 2024
@tpetry tpetry merged commit 5f1c063 into tpetry:master Jul 31, 2024
28 checks passed
@tpetry
Copy link
Owner

tpetry commented Jul 31, 2024

Thanks for your work @innocenzi.

I've now finally declared a stable 1.0.0 version. So this time (and in the future) I increased the major version number to indicate some behaviour changed because I had to adapt the implementation to Laravel's behaviour.

This wasn't an issue in the past years. But its now the second time they've added an method with different implementation than I've added.

I am now working on a daily CI run to be notified of merged breaking changes hopefully early enough to change Laravel's implementation.

@innocenzi innocenzi deleted the fix/def branch August 2, 2024 13:15
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.

Broken since laravel v11.17.0
2 participants