TitasGailius / laravel-moonlight

Batteries included Laravel preset based on InertiaJS, VueJS and TailwindCSS stack.
145 stars 10 forks source link

Normalize Directory Paths #6

Closed phillipsharring closed 4 years ago

phillipsharring commented 4 years ago

Fixes issue when attempting to use w/ Windows. Tested w/ fresh install of Laravel 7.x & laravel-moonlight

TitasGailius commented 4 years ago

Hello,

thanks for the PR but I prefer to use / instead of a DIRECTORY_SEPARATOR constant in this particular case.

phillipsharring commented 4 years ago

Moonlight in its current release doesn’t run on Windows. It fails when trying to create “webpack.mix.js” as a directory. Using the constant fixes that. It’s your decision, but I was just letting you know about it.

TitasGailius commented 4 years ago

then there must be something else because copy('some/directory/file.txt, other/directory/file.txt should also work on windows just fine.

phillipsharring commented 4 years ago

Agreed: That should work.

However, it runs into trouble in the ensureFileDirectoryExists function. In the case of the webpack.mix.js file, that function searches specifically for / in the $file string and doesn't find it, because that string (on my machine is) "C:\laragon\www\test-moonlight\webpack.mix.js," so it assumes it's a directory, doesn't return, then tries to make the directory, and fails, because a file with that name already exists.

Here's the console output from creating a fresh Laravel project, requiring moonlight, and running it (standard, non-error output removed).

C:\laragon\www
λ laravel new test-moonlight

C:\laragon\www
λ cd test-moonlight\

C:\laragon\www\test-moonlight
λ composer require titasgailius/laravel-moonlight

C:\laragon\www\test-moonlight
λ php artisan ui moonlight

   ErrorException

  mkdir(): File exists

  at C:\laragon\www\test-moonlight\vendor\titasgailius\laravel-moonlight\src\Moonlight.php:194
    190|             return;
    191|         }
    192|
    193|         if (! is_dir($diretory)) {
  > 194|             mkdir($diretory, 0755, true);
    195|         }
    196|     }
    197|
    198|     /**

  1   C:\laragon\www\test-moonlight\vendor\titasgailius\laravel-moonlight\src\Moonlight.php:194
      mkdir("C:\laragon\www\test-moonlight\webpack.mix.js")

  2   C:\laragon\www\test-moonlight\vendor\titasgailius\laravel-moonlight\src\Moonlight.php:175
      TitasGailius\Moonlight\Moonlight::ensureFileDirectoryExists("C:\laragon\www\test-moonlight\webpack.mix.js")

And if on line 89 of src/Moonlight.php, the first line of ensureFileDirectoryExists, I add:

        dd($file, Str::beforeLast($file, '/'));

It outputs:

C:\laragon\www\test-moonlight
λ artisan ui moonlight
"C:\laragon\www\test-moonlight\webpack.mix.js"
"C:\laragon\www\test-moonlight\webpack.mix.js"

The first line output being the $file, the second being the results of Str::beforeLast($file, '/').

So that check to see if $file is not a directory fails, then it tries to create webpack.mix.js as a folder, which it can't because that already exists as a file. I've verified this by moving the webpack.mix.js out of the way, and a folder is indeed created, but then the script goes on to fail on the next file it attempts to copy, because it thinks it should be a directory.

So, yes, as you say - copying with / should work, but searching in the system paths provided by the OS does not, which is why I suggested normalizing the paths to work with the OS's directory separator.

TitasGailius commented 4 years ago

Thank you for this detailed description of the problem.

The root cause of the problem was that webpack.mix.js and tailwind.config.js files are placed in the root application directory.

When base_path generates a path for these files, they have no / separator. As a result, the Str::beforeLast($file, '/') call was returning the entire file path and not the actual directory of a file.

I merged a fix for this https://github.com/TitasGailius/laravel-moonlight/pull/7. I would be really thankful if you double-checked it if it works for you too. It's not tagged yet but you can require with composer require titasgailius/laravel-moonlight:dev-master.