kucrut / vite-for-wp

Vite integration for WordPress plugins and themes development.
GNU General Public License v2.0
260 stars 29 forks source link

Problem with split CSS chunks with multiple outputs #85

Closed netlas closed 5 months ago

netlas commented 6 months ago

I ran into some problems with multiple outputs that depended on same internal imports. Not sure if it's Vite itself or the @vitejs/plugin-vue that splits up the js/css of vue components that is being reused in multiple output files. The js file are getting imported asynchronically in runtime. But the split css for the component is never imported. This is only a problem in production, the vite dev server handles it just fine.

I manage to fix this with a wp filter. (Thanks for adding these filters in the source code btw). But this should perhaps be added in a future update? Unless this is vue specific, but I dont think it is.

function vfw_production_assets(array $assets, object $manifest, string $entry, array $options): ?array
{
    $url = \Kucrut\Vite\prepare_asset_url( $manifest->dir );
    $item = $manifest->data->{$entry};
    if ( ! empty( $item->imports ) ) {

        foreach ( $item->imports as $import ) {
            $import_item = $manifest->data->{$import};

            if ( ! empty( $import_item->css ) ) {
                foreach ( $import_item->css as $css_file_path ) {
                    $slug = strtolower(trim(preg_replace('/[^A-Za-z0-9-]+/', '-', $css_file_path), '-'));
                    // Including a slug based on the actual css file in the handle ensures it wont be registered more than once.
                    $style_handle = "{$options['handle']}-{$slug}";

                    if ( wp_register_style( $style_handle, "{$url}/{$css_file_path}", $options['css-dependencies'], null, $options['css-media'] ) ) {
                        $assets['styles'][] = $style_handle;
                    }
                }
            }

        }

    }
    return $assets;
}
add_filter( 'vite_for_wp__production_assets', fn ( ...$args ) => vfw_production_assets( ...$args ), 10, 4 );

To clarify a bit more. The two input files in vite.config.js is "admin.ts" and "admin_menu.ts". Both uses the Vue component Menu.vue.

The manifest.js file looks like this:

{
  "Menu.css": {
    "file": "assets/Menu-c4e73a0f.css",
    "src": "Menu.css"
  },
  "_Menu.vue_vue_type_script_setup_true_lang-91a54a7b.js": {
    "css": [
      "assets/Menu-c4e73a0f.css"
    ],
    "file": "assets/Menu.vue_vue_type_script_setup_true_lang-91a54a7b.js",
    "imports": [
      "_runtime-dom.esm-bundler-17b5f22b.js"
    ]
  },
  "_runtime-dom.esm-bundler-17b5f22b.js": {
    "file": "assets/runtime-dom.esm-bundler-17b5f22b.js"
  },
  "resources/js/admin.ts": {
    "file": "assets/admin-45447c50.js",
    "imports": [
      "_runtime-dom.esm-bundler-17b5f22b.js",
      "_Menu.vue_vue_type_script_setup_true_lang-91a54a7b.js"
    ],
    "isEntry": true,
    "src": "resources/js/admin.ts"
  },
  "resources/js/admin_menu.css": {
    "file": "assets/admin_menu-c4837d8c.css",
    "src": "resources/js/admin_menu.css"
  },
  "resources/js/admin_menu.ts": {
    "css": [
      "assets/admin_menu-c4837d8c.css"
    ],
    "file": "assets/admin_menu-5fa266d6.js",
    "imports": [
      "_runtime-dom.esm-bundler-17b5f22b.js",
      "_Menu.vue_vue_type_script_setup_true_lang-91a54a7b.js"
    ],
    "isEntry": true,
    "src": "resources/js/admin_menu.ts"
  }
}

_runtime-dom.esm-bundler-17b5f22b.js (Vue.js framework) and _Menu.vue_vue_type_script_setup_true_lang-91a54a7b.js (Menu.vue) are being imported async in admin_menu-5fa266d6.js or admin-45447c50.js which vite for wp is already registering. With my filter above assets/Menu-c4e73a0f.css is now also being registered.

netlas commented 6 months ago

Perhaps the manifest file should be checked recursively for css files. In the event the imported javascript files also has imports with css dependencies. For example if _Menu.vue_vue_type_script_setup_true_lang-91a54a7b.js would have imports with css files as well. In that case they would need to be registered too. I updated my filter function like this:

Edit: Imports are now registered in correct order, so you can properly override css dependencies.

public function vfw_production_assets(array $assets, object $manifest, string $entry, array $options): ?array
{
    $url = \Kucrut\Vite\prepare_asset_url( $manifest->dir );
    $item = $manifest->data->{$entry};
    $import_styles = array();
    if ( ! empty( $item->imports ) ) {
        // Recursive inline function to deeply check for .css files.
        $check_imports = function( array $imports ) use ( &$check_imports, $manifest, $url, $options, &$import_styles ) : void {

            foreach ( $imports as $import ) {
                $import_item = $manifest->data->{$import};

                if ( ! empty( $import_item->imports ) ) {
                    $check_imports( $import_item->imports );
                }

                if ( ! empty( $import_item->css ) ) {
                    foreach ( $import_item->css as $css_file_path ) {

                        $slug = strtolower(trim(preg_replace('/[^A-Za-z0-9-]+/', '-', $css_file_path), '-'));
                        // Including a slug based on the actual css file in the handle ensures it wont be registered more than once.
                        $style_handle = "{$options['handle']}-{$slug}";

                        if ( wp_register_style( $style_handle, "{$url}/{$css_file_path}", $options['css-dependencies'], null, $options['css-media'] ) ) {
                            $import_styles[] = $style_handle;
                        }
                    }
                }

            }

        };
        $check_imports( $item->imports ); // Initiate recrusion
    }
    // Put imported styles before the main ones.
    $assets['styles'] = array_merge($import_styles, $assets['styles']);

    return $assets;
}
add_filter( 'vite_for_wp__production_assets', fn ( ...$args ) => vfw_production_assets( ...$args ), 10, 4 );
BboyKeen commented 6 months ago

Hey @netlas What a coincidence! I ran into the same issue and was wondering if I should send a PR with the same kind of fix. Thanks for sharing your code, I think checking all the imports is the right way to go.

I noticed this issue only on entrypoints and didn't know if there is the same issue on imported components.

netlas commented 6 months ago

Indeed what a coincidence :) Yea I could probably have done a pull request, but I was just not confident enough that this solution would be correct for every use case, I'm quite green with Vite. Perhaps someone that knows this package better could take a look at it.

BboyKeen commented 6 months ago

Just tested it and your solution is working for my project 😉

netlas commented 6 months ago

Updated the WP filter above so the imported css will enqueue a head of the output css. That way you can properly override the css of your dependancies.

Also made a pull request for a permanent fix for this.

kucrut commented 5 months ago

Fixed via #86