getStyles() not working in dist library rewrite

Created on 2 October 2023, 9 months ago
Updated 31 May 2024, 28 days ago

Problem/Motivation

When processing JS files in Vite::rewriteLibraryForDist(), it will try to include bundled css by calling Manifest::getStyles($path).

However, this lookup fails when the generated file does have a matching entry in manifest.json.

Here is an example:

// vite.config.js
rollupOptions: {
  input: 'src/main.js'
}
// main.js
import 'virtual:uno.css';
import './styles.css';
// manifest.json
{
  "src/main.css": {
    "file": "main-58455558.css",
    "src": "src/main.css"
  },
  "src/main.js": {
    "css": [
      "main-58455558.css"
    ],
    "file": "main-b48015fe.js",
    "isEntry": true,
    "src": "src/main.js"
  }

getStyles() returns getChunkPropertyPaths('css', $chunk, $prependBaseUri).

getChunkPropertyPaths() checks if there's a value for the css property, and then does:

    return array_filter(array_map(
      fn($import) => $this->getChunk($import, $prependBaseUri),
      $this->manifest[$chunk][$property],
    ));

getChunk() does a lookup on main-58455558.css, but it's not a top level chunk in manifest.json, so getStyles() returns nothing.

Steps to reproduce

Here is how to configure vite in a way that works with the current code.

input: 'src/main.js',
output: {
  entryFileNames: `src/[name].js`,
  assetFileNames: `src/[name].[ext]`,
},

With the src/ path added to the entry file and generated asset name, the manifest.json has:

{
  "src/main.css": {
    "file": "src/main.css",
    "src": "src/main.css"
  },
  "src/main.js": {
    "css": [
      "src/main.css"
    ],
    "file": "src/main.js",
    "isEntry": true,
    "src": "src/main.js"
  }
}

Proposed resolution

getStyles should use the file names in the chunk's css property instead of looking them up as another chunk in the manifest.

I think this has been overlooked since having the css in the libraries.yml file will load it when it's not found as a bundle for the js library.

Remaining tasks

Patch it.

API changes

Might disrupt sites, so should be in a 2.x branch.

πŸ› Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States mortona2k Seattle

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mortona2k
  • πŸ‡ΊπŸ‡ΈUnited States mortona2k Seattle

    Create the path from the bundled file name instead of looking it up as another chunk.

    @@ -116,7 +116,7 @@ class Manifest {
         }
     
         return array_filter(array_map(
    -      fn($import) => $this->getChunk($import, $prependBaseUri),
    +      fn($import) => $this->getPath($import),
           $this->manifest[$chunk][$property],
         ));
       }
    
  • First commit to issue fork.
  • Merge request !6fix: do not treat css and assets as chunk β†’ (Open) created by JaZo
  • Status changed to Needs review 6 months ago
  • πŸ‡³πŸ‡±Netherlands JaZo
  • Status changed to Needs work 5 months ago
  • πŸ‡΅πŸ‡±Poland wotnak

    This is caused by a change in Vite 5. https://vitejs.dev/guide/migration.html#corresponding-css-files-are-not-...
    In earlier versions css files were added as separate chunks, in Vite 5 for js entrypoints they are only listed in the js chunk under css key.

    Manifest generated for a js file that imports css file:

    • on Vite 3 and 4
        {
          "src/main.css": {
            "file": "assets/main-98fde70f.css",
            "src": "src/main.css"
          },
          "src/main.js": {
            "css": [
              "assets/main-98fde70f.css"
            ],
            "file": "assets/main-690fc99d.js",
            "isEntry": true,
            "src": "src/main.js"
          }
        }
        
    • on Vite 5
        {
          "src/main.js": {
            "css": [
              "assets/main-gINCZU8i.css"
            ],
            "file": "assets/main-6jBz11gc.js",
            "isEntry": true,
            "src": "src/main.js"
          }
        }
        

    Since we already support both Vite 3-4 and Vite 5 in other places (manifest location, dev server response codes) we should also ensure support for both versions in this case.

  • πŸ‡«πŸ‡·France myriam_b

    Hello,

    Any news for this issue ?
    It's ok for me when i update manifest.php like the merge request. Without this patch, Sass file import in my Typescript are not loaded.

    Thanks,

Production build 0.69.0 2024