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

[2.x] Laravel 10 support #2155

Merged
merged 18 commits into from Mar 30, 2023
Merged

Conversation

antonioribeiro
Copy link
Member

Just installed it locally, tests are passing.

@what-the-diff
Copy link

what-the-diff bot commented Feb 15, 2023

@ifox
Copy link
Member

ifox commented Feb 15, 2023

Thanks @antonioribeiro!

Some of our dependencies are not supporting Laravel 10 yet, as noted on #2122 (comment).

lazychaser/laravel-nestedset is an optional dependency, so not a blocker, but cartalyst/tags and Astronomic/laravel-translatable would prevent the install at the moment, no?

.github/workflows/main.yml Outdated Show resolved Hide resolved
@antonioribeiro
Copy link
Member Author

Yeah, you are right, @ifox , closing this for now.

@ifox
Copy link
Member

ifox commented Feb 15, 2023

I think we can keep it open? Definitely want to release support for Laravel 10 on Twill 2.

@antonioribeiro
Copy link
Member Author

Sure I'll reopen it as soon as we are able to close all subjects. I'm waiting on the nestedset guys to merge one of those PRs :D

@antonioribeiro
Copy link
Member Author

antonioribeiro commented Feb 21, 2023

@ifox , all PHPUnit tests are now passing, but we have some flags on roave-backwards-compatibility-check that must be thought before we move forward. This is one example:

protected $dates = [
    'deleted_at',
];

It was deprecated in favor of $casts on Laravel 8, and now removed from Laravel 10, so we are supposed to change the implementation but should we keep supporting Laravel versions that doesn't have $casts, are we still supporting any?

@ifox
Copy link
Member

ifox commented Feb 21, 2023

Thanks @antonioribeiro.

We do indeed have to take a closer look at this change, because we also rely on getDates from Eloquent, which implementation has changed in Laravel 10: https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L1441. Before, it was using the $dates model property: https://github.com/laravel/framework/blob/9.x/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L1450. And this is where we rely on it:

foreach ($this->model->getDates() as $f) {

We are not supporting any version that didn't have $casts, as it has been available since 5.0.

@antonioribeiro
Copy link
Member Author

@ifox, I just reimplemented getDates() to support Laravel 10 getting from $dates and created some tests to check if it's all working fine.

Most of the other errors can probably be ignored, but I will still look at them all tomorrow

@antonioribeiro
Copy link
Member Author

@ifox , here's the analysis of the breaking change errors we get:

BROKEN - Needs a fix -- Please review the fix

Error: Property Illuminate\Database\Eloquent\Model#$dates was removed

Can probably be ignored

Error: Default parameter value for parameter $inverse of Illuminate\Database\Eloquent\Concerns\HasRelationships#morphToMany() changed from false to NULL

Error: Parameter 7 of Illuminate\Database\Eloquent\Concerns\HasRelationships#morphToMany() changed name from inverse to relation

Not being used directly (internal calls)

Error: Method Illuminate\Foundation\Bus\Dispatchable::dispatchNow() was removed

Error: Method Illuminate\Foundation\Bus\DispatchesJobs#dispatchNow() was removed

Error: Parameter 4 of Illuminate\Foundation\Validation\ValidatesRequests#validateWithBag() changed name from customAttributes to attributes

Error: The parameter $parameters of Illuminate\Support\Traits\Macroable#__call() changed from no type to a non-contravariant array

Error: The parameter $parameters of Illuminate\Support\Traits\Macroable#__call() changed from no type to array

Error: Parameter 3 of Illuminate\Foundation\Validation\ValidatesRequests#validate() changed name from customAttributes to attributes

@what-the-diff
Copy link

what-the-diff bot commented Mar 29, 2023

PR Summary

  • Add Laravel 10 Support
    Updated the codebase to support Laravel 10
  • Fix Date Casting in Models
    Improved the handling of date formats in models
  • Update Composer Dependencies
    Upgraded dependencies to the latest version, including PHP 8, and added new ones: chillerlan/php-qrcode, friendsofphp/php-cs-fixer, kalnoy/nestedset, nunomaduro/collision, and orchestra/testbench v8.*
  • Remove Unused Code from Tests Folder
    Removed unnecessary code in the tests folder to resolve errors with PHPUnit 9+

@ifox ifox changed the title Allow installing on Laravel 10 [2.x] l Laravel 10 Mar 29, 2023
@ifox ifox changed the title [2.x] l Laravel 10 [2.x] lLaravel 10 Mar 29, 2023
@ifox ifox changed the title [2.x] lLaravel 10 [2.x] Laravel 10 support Mar 29, 2023
@what-the-diff
Copy link

what-the-diff bot commented Mar 30, 2023

PR Summary

  • Add Support for Laravel 10
    This update adds compatibility with Laravel 10, a popular PHP framework.

  • Fix Date Casting in Models
    The fix improves the handling of date fields in the application's data models, providing accurate results.

  • Update Composer Dependencies
    Dependencies, including PHP 8, are updated to their latest versions, enhancing the application's performance and security.

  • Remove Unnecessary CI Checker
    The backwards compatibility checker is removed from the Continuous Integration (CI) process, as it's no longer required due to an updated laravel/framework dependency version.

@haringsrob haringsrob merged commit d34d84d into area17:2.x Mar 30, 2023
15 checks passed
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