arnoson / kirby-vite

Use Kirby CMS together with Vite
MIT License
81 stars 7 forks source link

[v4.0.5] Problem returning the outDir #27

Closed bogdancondorachi closed 1 year ago

bogdancondorachi commented 1 year ago

I've tried to update the plugin and my setup to the v4 but keep getting:

Cannot assign bool to property arnoson\KirbyVite\Vite::$outDir of type ?string

That happens on the outDir function:

protected function outDir() {
    return $this->outDir ??= kirby()->root('base')
      ? getRelativePath(
        kirby()->root('index'),
        kirby()->root('base') . '/' . $this->config()['outDir']
      )
      : $this->config()['outDir'];
  }

I've also tried both starter kit's and got the same results without luck on getting to the bottom of it.

EDIT: I've got through the above error by adding $this->outDir ??= kirby()->root('base') inside parentheses to check whether $this->outDir is null and then decide whether to calculate the output directory path based on the condition

return ($this->outDir ??= kirby()->root('base'))

But forward it seems like the manifest path is wrong, returning public//manifest.json meaning that the $outDir returns empty in the following case

$manifestPath = "$index/$outDir/manifest.json";

*The tests are made on the vite starter kit with just the modifications presented above.

arnoson commented 1 year ago

Are you using a public folder setup? I just checked and for me both the basic and the multi page kit are working. I'm running it on WSL 2 (Ubuntu). Do I anderstand that plain kits are not working for you? Or have you modified the vite.config.js somehow?

Why did you set the outDir to base return ($this->outDir ??= kirby()->root('base'))? In boths kits the outDir is dist inside public.

bogdancondorachi commented 1 year ago

Yes, I'm using the public folder setup. Also I've tried both basic and multi-page kit without any modification and getting the same error, running it on Windows with php 8.2.1 and apache.

Cannot assign bool to property arnoson\KirbyVite\Vite::$outDir of type ?string

I've tried this to get past the error and see what outDir is returning in this case

protected function outDir() {
    return ($this->outDir ??= kirby()->root('base'))
      ? getRelativePath(
        kirby()->root('index'),
        kirby()->root('base') . '/' . $this->config()['outDir']
      )
      : $this->config()['outDir'];
  }

I've added this to get the 'manifest.json not found' path

public function manifest(): array {
    if (!F::exists($manifestPath)) {
      if (option('debug')) {
        throw new Exception('`manifest.json` not found at: ' . $manifestPath);
      }
      return [];
    }
}

And it returns me with: manifest.json not found at: C:\Users\bogda\Downloads\kirby-vite-basic-kit-main\kirby-vite-basic-kit-main\public//manifest.json where it does not return dist

If you got any ideas I'm happy to test around to get to the bottom of this issue

arnoson commented 1 year ago

Okay, I'm not coding in Windows so I don't have composer/php ready to test it. Some thoughts:

bogdancondorachi commented 1 year ago

I've looked more into the getRelativePath function and indeed it's related to file path differences between the operating systems. I've managed to fix the issue like follows:

function getRelativePath($rootPath, $fullPath) {
  $rootPath = realpath(rtrim($rootPath, '/'));
  $fullPath = realpath(rtrim($fullPath, '/'));

  if ($rootPath !== false && $fullPath !== false && strpos($fullPath, $rootPath) === 0) {
    $relativePath = ltrim(substr($fullPath, strlen($rootPath)), DIRECTORY_SEPARATOR);
    return $relativePath;
  }

  return false;
}

Let me know your thoughts on this and if you can test it on linux as well.

arnoson commented 1 year ago

I could reproduce your issue in windows now and your getRelativePath works great in both windows and linux. Thanks for pointing out this issue and coming up with a fix! Do you want to create a PR with these changes? I could merge it later today

bogdancondorachi commented 1 year ago

Great! Thanks as well for pointing me in the right direction. I've added the PR #28

arnoson commented 1 year ago

Released in v4.0.6 :)