Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Fix ordering of stylesheets when using @vite #43962

Merged
merged 1 commit into from
Sep 5, 2022
Merged

[9.x] Fix ordering of stylesheets when using @vite #43962

merged 1 commit into from
Sep 5, 2022

Conversation

jul13579
Copy link
Contributor

@jul13579 jul13579 commented Sep 1, 2022

First up: Thank you all so much for this awesome framework (and the great documentation). I'm using it for many years now and always enjoy working with it!

When I started a new project recently I however came across a problem: My stylesheets end up in the wrong order in the rendered HTML when using @vite. This prevented any CSS overrides from within my application's stylesheet to actually work.

This is the current order of the stylesheets:

<link rel="stylesheet" href="https://.../build/assets/app.0981a13d.css">
<link rel="stylesheet" href="https://.../build/assets/vendor.a8fc34b5.css">

when it should IMHO actually be the other way around.

This PR fixes this issue, although please forgive the very "quick-fix" nature of the PR. I'm not aware if my changes could break things elsewhere or if this actually touches intended behaviour.

For anyone coming across this

For the time being, I solved this issue in my project by introducing a custom blade directive and extending from \Illuminate\Foundation\Vite. This allowed me to prevent any changes to vendor code while still being able to have my stylesheets rendered in the right order.

// In app/Providers/AppServiceProvider.php
public function boot()
{
    ...
    Blade::directive('custom_vite', function ($arguments) {
        $arguments ??= '';
        $class = Vite::class;
        return "<?php echo app('$class')({$arguments}); ?>";
    });
}

// Created app\Facades\Vite.php
<?php

namespace App\Facades;

use Exception;
use Illuminate\Foundation\Vite as OriginalVite;
use Illuminate\Support\Collection;
use Illuminate\Support\HtmlString;

class Vite extends OriginalVite
{
    /**
     * Generate Vite tags for an entrypoint.
     *
     * @param  string|string[]  $entrypoints
     * @param  string|null  $buildDirectory
     * @return \Illuminate\Support\HtmlString
     *
     * @throws \Exception
     */
    public function __invoke($entrypoints, $buildDirectory = null)
    {
        $entrypoints = collect($entrypoints);
        $buildDirectory ??= $this->buildDirectory;

        if ($this->isRunningHot()) {
            return new HtmlString(
                $entrypoints
                    ->prepend('@vite/client')
                    ->map(fn ($entrypoint) => $this->makeTagForChunk($entrypoint, $this->hotAsset($entrypoint), null, null))
                    ->join('')
            );
        }

        $manifest = $this->manifest($buildDirectory);

        $tags = collect();

        foreach ($entrypoints as $entrypoint) {
            $chunk = $this->chunk($manifest, $entrypoint);

            foreach ($chunk['imports'] ?? [] as $import) {
                foreach ($manifest[$import]['css'] ?? [] as $css) {
                    $partialManifest = Collection::make($manifest)->where('file', $css);

                    $tags->push($this->makeTagForChunk(
                        $partialManifest->keys()->first(),
                        asset("{$buildDirectory}/{$css}"),
                        $partialManifest->first(),
                        $manifest
                    ));
                }
            }

            $tags->push($this->makeTagForChunk(
                $entrypoint,
                asset("{$buildDirectory}/{$chunk['file']}"),
                $chunk,
                $manifest
            ));

            foreach ($chunk['css'] ?? [] as $css) {
                $partialManifest = Collection::make($manifest)->where('file', $css);

                $tags->push($this->makeTagForChunk(
                    $partialManifest->keys()->first(),
                    asset("{$buildDirectory}/{$css}"),
                    $partialManifest->first(),
                    $manifest
                ));
            }
        }

        [$stylesheets, $scripts] = $tags->partition(fn ($tag) => str_starts_with($tag, '<link'));

        return new HtmlString($stylesheets->join('').$scripts->join(''));
    }
}

Afterwards just use @vite_custom(...) instead of @vite(...) in your blade templates 😉

In order to allow CSS overrides from within the application's
stylesheet, the application's stylesheet has to be included *after* any
vendor stylesheet.
@jul13579 jul13579 changed the title Fix ordering of stylesheets Fix ordering of stylesheets when using @vite Sep 1, 2022
@taylorotwell taylorotwell marked this pull request as draft September 1, 2022 14:23
@nunomaduro nunomaduro changed the title Fix ordering of stylesheets when using @vite [9.x] Fix ordering of stylesheets when using @vite Sep 1, 2022
@timacdonald
Copy link
Member

@jul13579 thank you for the PR!

Are you able please provide a minimal reproduction case for this?

laravel new bug-report --github="--public"

I was unable to replicate this to verify the issue. Thank you!

@jul13579
Copy link
Contributor Author

jul13579 commented Sep 2, 2022

@jul13579 thank you for the PR!

Are you able please provide a minimal reproduction case for this?

laravel new bug-report --github="--public"

I was unable to replicate this to verify the issue. Thank you!

Hi @timacdonald! Thank you for your quick response.

I have now set-up the bug-report repo here, where I consolidated the example into this single commit. I used the same setup that I used in my other project, hence there might even be a more minimal example.

When using the code from above referenced commit, do the following to reproduce the issue:

  1. composer i
  2. npm i
  3. npm run build
  4. php artisan serve

Now if you navigate to http://localhost:8000 you should see a tiny card (Vuetify's VCard) containing "Some content". The text is not underlined, although I want it to be since I have overwritten the .v-card class with text-decoration: underline. This is due to the ordering of the stylesheets in the DOM. When taking a look at it, they appear like this:

<link rel="stylesheet" href="http://127.0.0.1:8000/build/assets/app.8e438c19.css">
<link rel="stylesheet" href="http://127.0.0.1:8000/build/assets/vendor.dc6e3714.css">

When you manually reorder those elements by dragging and dropping the vendor...css above the app...css, my stylesheet modifications work.

I hope this helps and actually is what you wanted me to do 😅

Copy link
Member

@timacdonald timacdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've verified this issue and the fix. Additionally, this created parity between the dev server and the build.

Unfortunately this bug fix is also a potentially breaking change, but I do feel it is worthwhile making the change now as a bugfix.

As the CSS tags were in the wrong order, vendor CSS may be overriding userland CSS.

Not a breaking change if this was noticed and the userland CSS precedence was forced, e.g. ! important.

Breaking change if this was not noticed and the vendor CSS is still taking precedence. So although it is breaking, it is also really what is expected i.e. my CSS has higher precedence than vendor CSS.

Thank you very much @jul13579 for the PR and reproduction!

@timacdonald timacdonald marked this pull request as ready for review September 5, 2022 01:36
@taylorotwell taylorotwell merged commit 16e2c71 into laravel:9.x Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants