babenkoivan / elastic-migrations

Elasticsearch migrations for Laravel
MIT License
188 stars 32 forks source link

TypeError on composer require #18

Closed patie closed 3 years ago

patie commented 3 years ago
Software Version
PHP 7.4.14
Elasticsearch 7.9
Laravel 8.12

Describe the bug when i do composer require babenkoivan/elastic-migrations i get TypeError:

   TypeError

  rtrim() expects parameter 1 to be string, null given

  at vendor/babenkoivan/elastic-migrations/src/Filesystem/MigrationStorage.php:23
     19▕
     20▕     public function __construct(Filesystem $filesystem)
     21▕     {
     22▕         $this->filesystem = $filesystem;
  ➜  23▕         $this->directory = rtrim(config('elastic.migrations.storage_directory'), '/');
     24▕     }
     25▕
     26▕     public function create(string $fileName, string $content): MigrationFile
     27▕     {

      +1 vendor frames
  2   [internal]:0
      ElasticMigrations\Filesystem\MigrationStorage::__construct(Object(Illuminate\Filesystem\Filesystem))

      +19 vendor frames
  22  artisan:37
      Illuminate\Foundation\Console\Kernel::handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
Script @php artisan package:discover --ansi handling the post-autoload-dump event returned with error code 1
patie commented 3 years ago

i see its duplicate of https://github.com/babenkoivan/elastic-migrations/issues/4, but i have cache driver on local 'null', tried also cache:clear, and i still got same TypeError

babenkoivan commented 3 years ago

Hey @patie, thanks for reporting! I'll try to reproduce it and come back with the results.

Have you tried to run these commands?

php artisan vendor:publish --provider="ElasticClient\ServiceProvider"
php artisan vendor:publish --provider="ElasticMigrations\ServiceProvider"

If this doesn't work due to the error, you can manually copy config files from the vendor directory.

patie commented 3 years ago

yes i tried this commands, but whole artisan was blocked by the same error, only work solution was described in #4, when you must update file from vendors (because this file depends on config which not exists at this time), after this run commands and update composer.. its working now, but its not good feeling, when you have problem on start with installing library only 😬

babenkoivan commented 3 years ago

Hey @patie,

I've tried to reproduce the issue again using a fresh Laravel installation with PHP 7.4, but it works. I'm not sure why this error happens. config('elastic.migrations.storage_directory') should always return a string, cause I merge configs on ServiceProvider::register() and register the commands only in ServiceProvider::boot() method.

Nevertheless, I've modified the code the way this error never happens, but it might be still necessary to publish the configs.

The fix is introduced in v1.3.2.