idleberg / php-vite-manifest

A parser for Vite manifest files
https://packagist.org/packages/idleberg/vite-manifest
MIT License
12 stars 2 forks source link

Css entry not detected as css (css used as input entry) #3

Closed idflood closed 11 months ago

idflood commented 1 year ago

I'm trying to implement vitejs on my base wordpress theme, and I've found that the css file is injected with a script tag:

$viteAssets->inject(["src/main.js", "src/main.css"], [
  'integrity' => FALSE
]);

The generated manifest.json contains this entry:

"src/main.css": {
    "file": "assets/main-54215d86.css",
    "isEntry": true,
    "src": "src/main.css"
  },

One reason this happen is because the current Manifest.php check for the existence of a css property in the entry which is not the case there. The reason this doesn't have a "css" property is probably because i set the css as an entry input instead of importing it from the javascript.

export const viteConfig = {
  ...
  build: {
  rollupOptions: {
      input: [
          resolve(__dirname, './src/main.js'),
          resolve(__dirname, './src/main.css'),
          resolve(__dirname, './src/editor.css'),
          resolve(__dirname, './src/admin.css')
        ],
      },
  }
  ...
}

So maybe in Manifest.php it should also check if file extension ends with .css to consider the entry as style in addition to the current check.

idleberg commented 1 year ago

Hi David. I had hoped to write back to you earlier, but life got in the way. It took me a while to (hopefully) understand the problem and assumed that it boils down to the automatic injection by wordpress-vite-assets. I started working on a fix and was confident to release it in the next couple of weeks.

Today, you were so kind to open a PR. However, this added some confusion and I'm not so sure I understand the problem you are describing. So, I'd like to ask some questions:

Why do you think it is necessary to make any changes in this repository? The getEntrypoint() method already returns any file type.

Shouldn't the change made in the other package be sufficient to address you problem?

PS:Sorry if those questions might sound naive to you. I don't have a proper environment right now to test my changes and I haven't touched PHP in a while.

idflood commented 1 year ago

Hi @idleberg , no problem. Your fix looks cleaner but I think one part need to be fixed on this package.

The remaining issue is that getStyles check for the entry["css"] which is not defined when the entrypoint is directly a css file (here is a sample manifest). So with the change you mention the css file is not anymore inserted as script tag but it is still not injected as <style>.

Here is the part I had to change so that the getStyles method returns this css entry, otherwise the getStyles returns nothing: https://github.com/idleberg/php-vite-manifest/pull/4/files#diff-b2bc2549b260268f1c9acda1f36675701f32b0db0679fa58ddcaca0772274f41R136-R142 (note that the condition could be simplified and only check if the $entry ends with css)

Don't hesitate to let me know if this is still not clear for you.