voyager icon indicating copy to clipboard operation
voyager copied to clipboard

Laravel 10 support

Open jf-m opened this issue 2 years ago • 11 comments
trafficstars

This PR includes the last bits to make Voyager Laravel 10 compatible (work started there #5732 ).

Here's a breakdown of what I did:

  • Update the phpunit.xml and the test cases to PHPUnit 10 compatibility.
  • nullcheck on the MenuItem's getParameter, closing #5603
  • Remove all conditions/checks for the old seeds folder and use the seeders folder everywhere (Laravel 8+).
  • Solve issues I had with doctrine/DBAL's Table and Column classes when upgrading to L10.

This PR passes all the phpunit tests up until PHP 8.2 on my fork (https://github.com/jf-m/voyager/actions/runs/4600528910).

Any comments welcome, hope this helps !

jf-m avatar Apr 03 '23 19:04 jf-m

This looks dodgy...anyone tested successfuly?

mdenitti avatar Apr 06 '23 13:04 mdenitti

@mdenitti can you clarify what is that you find "dodgy" ? Like stated before, you can add comments if there's something wrong from your point of view.

I must say that my auto-indent might have removed some spaces in the composer.json and phpunit.xml. Maybe this is what confuses you ?

I can add some more explanations about how I solved the issue at the first place.

The initial problem from the current state of the branch 1.6-l10 can be seen here: https://github.com/the-control-group/voyager/actions/runs/4080000210/jobs/7498342456?pr=5732.

ErrorException: require_once(/home/runner/work/voyager/voyager/vendor/orchestra/testbench-core/laravel/vendor/autoload.php): Failed to open stream: No such file or directory

It came from the following line of code:

https://github.com/the-control-group/voyager/blob/a74198baf70f26ca6aa02723aea39a03cc45b043/src/Commands/InstallCommand.php#L146

This require_once, to my knowledge, is here for compatibility reasons, so Voyager can be compatible for both Laravel < 7 and Laravel 8+. Indeed, since Laravel 8 and above, the seeds folder is called seeders and the namespace Database\Seeders; is mandatory on the seeders.

Right now, Voyager Seeder system is based on Laravel <7 with seeds and no namespace. Voyager in the InstallCommand check the Laravel version, if 8+ then it will rename the seeds to seeders and it will manually add the namespace to all the seeders files here : https://github.com/the-control-group/voyager/blob/a74198baf70f26ca6aa02723aea39a03cc45b043/src/Commands/InstallCommand.php#L165-L181

In my PR, I removed all this, because Voyager is no more compatible for Laravel <7. I renamed seeds to seeders and I added all the needed namespaces. By doing so, the require_once base_path('vendor/autoload.php'); is no more necessary because the seeders file have not changed (the Namespace is already there). This solved the main issue.

jf-m avatar Apr 06 '23 14:04 jf-m

@mdenitti I added a comment on each major changes for more clarity, to help the reviewer. Hope this helps. 👍

jf-m avatar Apr 06 '23 14:04 jf-m

@mdenitti I added a comment on each major changes for more clarity, to help the reviewer. Hope this helps. 👍

Jean! Wonderfull this looks much better; Genius... Merging still blocked???? We need L10 support now :)))

mdenitti avatar May 10 '23 09:05 mdenitti

Is it worth downgrading somehow to L9 from L10? Or there will be support soon? 😊

dmatis2 avatar May 10 '23 22:05 dmatis2

try this, worked for me on laravel 10 composer require tcg/voyager dev-1.6-l10

thank later

hazbu avatar May 11 '23 03:05 hazbu

composer require tcg/voyager dev-1.6-l10

Found 1 security vulnerability advisory affecting 1 package. Run composer audit for a full list of advisories.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.

mdenitti avatar May 16 '23 07:05 mdenitti

How is this taking so long to get merged? It works!

octoxan avatar Jun 20 '23 12:06 octoxan

It looks like the-control-group do not intend to maintain the project any further. They have had no reaction since February, when I first mentioned the issue with Laravel 10. And also some links in the documentation no longer work. I think this repository will no longer be supported. Sad.

JonathanVorich avatar Jul 21 '23 06:07 JonathanVorich

@marktopper, @marktopper Hello, I hope both of you are alive. Could you please pay some attention here?

haydar avatar Aug 02 '23 14:08 haydar

Is this package no longer updated to support new versions of Laravel?

sajaddp avatar Aug 26 '23 11:08 sajaddp