teamtnt / laravel-scout-tntsearch-driver

Driver for Laravel Scout search package based on https://github.com/teamtnt/tntsearch
MIT License
1.09k stars 144 forks source link

Laravel 9.x Compatibility #334

Closed laravel-shift closed 2 years ago

laravel-shift commented 2 years ago

This is an automated pull request from Shift to update your package code and dependencies to be compatible with Laravel 9.x.

Before merging, you need to:

If you do find an issue, please report it by commenting on this PR to help improve future automation.

laravel-shift commented 2 years ago

:alembic: Using this package? If you would like to help test these changes or believe them to be compatible, you may update your project to reference this branch.

To do so, temporarily add Shift's fork to the repositories property of your composer.json:

{
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/laravel-shift/laravel-scout-tntsearch-driver.git"
        }
    ]
}

Then update your dependency constraint to reference this branch:

{
    "require": {
        "teamtnt/laravel-scout-tntsearch-driver": "dev-l9-compatibility",
    }
}

Finally, run: composer update

njoguamos commented 2 years ago

I tested this on a Laravel 9 and PHP 8.1 and got the following error:

[2022-02-12 17:50:54] local.ERROR: During inheritance of Countable: Uncaught _PhpScoperc223a629f245\Psy\Exception\ErrorException: PHP Deprecated: Return type of TeamTNT\TNTSearch\Support\Collection::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in ***/vendor/teamtnt/tntsearch/src/Support/Collection.php on line 123 in ***/vendor/teamtnt/tntsearch/src/Support/Collection.php:123

The line 123 caused the error.

This is what Laravel 9 upgrade statement: PHP is beginning to transition to requiring return type definitions on PHP methods

I will diagnose further and submit a PR when I get a solution.

johndalangin commented 2 years ago

Hi @njoguamos let me know if you need any help on this since we are also keen on upgrading to Laravel 9 and we're using this package in production also. 👍

praxxys commented 2 years ago

I pushed a PR here to fix the issue https://github.com/teamtnt/tntsearch/pull/266

@njoguamos

njoguamos commented 2 years ago

@praxxys thank you. I will check it out when I get some time.

amirmms commented 2 years ago

hi please merge this we need this package on laravel 9