ckfinder / ckfinder-laravel-package

CKFinder 3 package for Laravel
Other
153 stars 86 forks source link

Laravel 9 Upgrade #84

Closed roylaveto closed 2 years ago

roylaveto commented 2 years ago

I temporarily added my username to the composer file to be able to pull it from packagist since VCS is not working in git submodules. (So that can be removed.)

All changes had to be done to get the new AWS adapter working since we use this for our backups.

igorgaming commented 2 years ago

@roylaveto Hey, i try to use your fork and apparently, the main connector code does not support symphony/http-kernel 6.0

roylaveto commented 2 years ago

@roylaveto Hey, i try to use your fork and apparently, the main connector code does not support symphony/http-kernel 6.0

@igorgaming When does this happen for you? Do you have any more information so I can take a look. (Which file it happens in, what line of code etc..)

igorgaming commented 2 years ago

@roylaveto Yeah, of course.

[2022-05-02 18:21:22] local.ERROR: Declaration of CKSource\CKFinder\CKFinder::handle(Symfony\Component\HttpFoundation\Request $request, int $type = Symfony\Component\HttpKernel\HttpKernelInterface::MAIN_REQUEST, bool $catch = true)
must be compatible with Symfony\Component\HttpKernel\HttpKernelInterface::handle(Symfony\Component\HttpFoundation\Request $request, int $type = self::MAIN_REQUEST, bool $catch = true): Symfony\Component\HttpFoundation\Response

This happens on CKSource\CKFinder\CKFinder file on the 492 line, which is one of the files installed via php artisan ckfinder:download. The HttpKernelInterface required to specify Symfony\Component\HttpFoundation\Response return type for handle method.

Also there are error on 476 line. Type LocalFSAdapter is not defined(because the old import League\Flysystem\Adapter\Local should be changed to League\Flysystem\FilesystemAdapter. Maybe there are more errors in _connector files, but i think we cant fix files in _connector.

roylaveto commented 2 years ago

@igorgaming seems more like you are running PHP 8.1 with more strict type checking. If this is the case can you try running it with PHP 8.0?

If this is not the case please say so, then I'll look further into it! :)

igorgaming commented 2 years ago

@igorgaming seems more like you are running PHP 8.1 with more strict type checking. If this is the case can you try running it with PHP 8.0?

If this is not the case please say so, then I'll look further into it! :)

I am running on php 8.0. The reason of this is symphony/http-kernel 6.0(which laravel 9 requires). Check this line(in 6.0 and 5.4 branches): https://github.com/symfony/http-kernel/blob/42c79e352e569465f11b72088938b4f7f58f0d8e/HttpKernelInterface.php#L45

roylaveto commented 2 years ago

@igorgaming Seems like this isn't going to be an easy fix. There are a lot of functions that need to be updated because the cache adapter doesn't work anymore and simply replacing it with the cache adapter from Lustmored isn't going to work since it needs to be rewritten...

In the download command you can change the latest version of CKFinder to 3.5.3.1, this updates the code to be compatible with the returning typed. But after that there are way more files that need to be changed sadly.

I think we're going to need to wait for a rewrite from someone that wants to spend time figuring everything out as I simply don't have the time to do this.

igorgaming commented 2 years ago

@roylaveto Yeah, i think so. Thanks for your time.

roylaveto commented 2 years ago

@igorgaming I would recommend changing this package for https://github.com/UniSharp/laravel-filemanager

I just changed it and it works great! Seems like it has more maintenance as well.

igorgaming commented 2 years ago

@igorgaming I would recommend changing this package for https://github.com/UniSharp/laravel-filemanager

I just changed it and it works great! Seems like it has more maintenance as well.

Thanks, but I will try to wait a little longer with current package, because I have several plugins written for it (mostly they are related to additional checks for performing any actions that can be transferred to the package you recommended, but for now I will try to wait). As for maintain, yes, I think he is the most attractive from this side. Anyway, I planned to wait until php 8.1 becomes available on my hosting to switch to it immediately. If the current package is not fixed by this time, I think we will have to switch to the one you proposed.

igorgaming commented 2 years ago

Also i saw that this package is integrated only with CKEditor 4? I need integration for CKEditor 5.