Eptagone / Vite.AspNetCore

Small library to integrate Vite into ASP.NET projects
MIT License
264 stars 35 forks source link

Nested imports' CSS aren't processed when the dev server isn't running #98

Closed pete-intranel closed 5 months ago

pete-intranel commented 8 months ago

Is your feature request related to a problem? Please describe. Nested CSS assets aren't loaded, for instance a component imports a .scss which has @use '@mdi/font', icons are not displayed. This works fine when in dev mode, but fails with a prod build due to nesting in the manifest.

Describe the solution you'd like ProcessCssEntry.Process should process entry.Imports recursively.

Describe alternatives you've considered .

Additional context There is a todo regarding multiple css entries, so this would potentially address this too.

Here's a snippet of manifest; note main.ts has an imports reference, that in turn has a css asset:

...
  "_vue.esm-bundler-DOYej2wW.js": {
    "file": "assets/vue.esm-bundler-DOYej2wW.js",
    "css": [
      "assets/vue-B4WR3mra.css"
    ],
    "assets": [
    ]
  },
...
  "src/pages/privacy/main.ts": {
    "file": "assets/privacy-BrDXKx4g.js",
    "src": "src/pages/privacy/main.ts",
    "isEntry": true,
    "imports": [
      "_vue.esm-bundler-DOYej2wW.js"
    ],
    "css": [
      "assets/privacy-Bnwp9MNr.css"
    ]
  }

Maybe a bit like this? (the tag writing could be better)

...
if (tagName == "link" && (this.Rel == LINK_REL_STYLESHEET || this.As == LINK_AS_STYLE) && ScriptRegex.IsMatch(value))
{
    ProcessCssEntry(output, tagName, attribute, value, urlHelper, entry);
}
...

private void ProcessCssEntry(TagHelperOutput output, string tagName, string attribute, string value, Microsoft.AspNetCore.Mvc.IUrlHelper urlHelper, IViteChunk entry)
{
    // Get the styles from the entry
    var cssFiles = entry.Css;
    // Get the number of styles
    var count = cssFiles?.Count() ?? 0;
    // If the entrypoint doesn't have css files, destroy it.
    if (count == 0)
    {
        this.logger.LogWarning("The entry '{Entry}' doesn't have CSS chunks", value);
        output.SuppressOutput();
        return;
    }

    // Get the file path from the 'manifest.json' file
    foreach (string cssFile in cssFiles!)
    {
        string file = urlHelper.Content($"~/{(string.IsNullOrEmpty(this.basePath) ? string.Empty : $"{this.basePath}/")}{cssFile}");
        OutputCss(output, attribute, file);
    }

    if (entry.Imports != null)
    {
        foreach (string import in entry.Imports)
        {
            var importEntry = this.manifest[import];

            if (importEntry != null && importEntry.Css != null && importEntry.Css.Any())
            {
                ProcessCssEntry(output, tagName, attribute, value, urlHelper, importEntry);
            }
        }
    }
}

private void Output(TagHelperOutput output, string attribute, string file)
{
    // Forwards the rel attribute to the output.
    if (!string.IsNullOrEmpty(this.Rel))
    {
        output.Attributes.SetAttribute(new TagHelperAttribute(LINK_REL_ATTRIBUTE, this.Rel));
    }
    // Forwards the as attribute to the output.
    if (!string.IsNullOrEmpty(this.As))
    {
        output.Attributes.SetAttribute(new TagHelperAttribute(LINK_AS_ATTRIBUTE, this.As));
    }

    output.Attributes.SetAttribute(new TagHelperAttribute(
        attribute,
        file,
        HtmlAttributeValueStyle.DoubleQuotes)
    );
}

private void OutputCss(TagHelperOutput output, string attribute, string file)
{
    output.TagName = null;
    TagBuilder tagBuilder = new("link");

    // Forwards the rel attribute to the output.
    if (!string.IsNullOrEmpty(this.Rel))
    {
        tagBuilder.Attributes.Add(LINK_REL_ATTRIBUTE, this.Rel);
    }
    // Forwards the as attribute to the output.
    if (!string.IsNullOrEmpty(this.As))
    {
        tagBuilder.Attributes.Add(LINK_AS_ATTRIBUTE, this.As);
    }

    tagBuilder.Attributes.Add(attribute, file);
    tagBuilder.TagRenderMode = TagRenderMode.SelfClosing;

    output.Content.AppendHtml(tagBuilder);
}
firegodjr commented 7 months ago

I've been able to mitigate this on my multi-entrypoint project using vite-plugin-css-injected-by-js with the relativeCSSInjection flag set to true. Injects the css into the dom on a per-chunk basis, and as a bonus I don't have to import each entrypoint as a stylesheet.

Eptagone commented 7 months ago

Hi, do you have a reproduction repo?

hdimon commented 5 months ago

Hi, do you have a reproduction repo?

Hello, If you are still interested then I can give you link to my repo with test project where I encountered the same issue.

Kalshu commented 5 months ago

I'm also interrested by this feature

Eptagone commented 5 months ago

Hi, do you have a reproduction repo?

Hello,

If you are still interested then I can give you link to my repo with test project where I encountered the same issue.

Yes, please

hdimon commented 5 months ago

Hi, do you have a reproduction repo?

Hello, If you are still interested then I can give you link to my repo with test project where I encountered the same issue.

Yes, please

Here it is https://github.com/hdimon/ViteAspNetCoreTest/tree/multiple_css_files . multiple_css_files branch has been prepared to demostrate issue. Just do "npm run build" in ClientApp folder and then run application. If you want to see how it would look if it worked as expected then uncomment line 132 //cssInjectedByJsPlugin({ relativeCSSInjection: true }) in vite.config.ts, then do "npm run build" in ClientApp folder and run application. Let me know if you face any issues and thank you very much in advance!

someguy20336 commented 5 months ago

Ran into this yesterday. Seems more like a pretty important enhancement to me, given that it effectively isn't implementing the specification. Any reasonably sized project will probably result in several css files and thus they won't end up getting pulled in (probably?).

I might be interested in fixing it, but I need your input. I wrote my own TagHelper that correctly traverses the manifest (could share it here if interested), but the usage is slightly different than what you currently have designed. What do you think about this? I chose not to use a link considering it could result in any number of link tags (even though yea, you could probably do that with a link tag anyway).

<vite-styles entry-point="path/to/my/file.ts" />

Edit: on the other hand, I guess it could be a pretty big breaking change to change the tag helper, so my Idea is probably dumb. Let me know what you think

jsaele commented 5 months ago

We also parse the manifest ourselves, and build a dictionary of chunk names and chunks. We also have a tag helper that accepts the name of an entry point, then calls a method on the class that holds the parsed manifest to retrieve all css files for that entry point. That seems to work well. What was intuitive for us was to load "dependent" css first, i.e. shared/common css which holds variables and does some reset etc before loading the entry point css. We added it as an attribute on the head tag, and the link tags are added to the bottom of the head tag atm.

An issue we ran into though, was when loading async chunks after the entry point was loaded (while navigating client side for instance). Where the chunk also depended on the shared css, it seems the bundled js added a new link tag with the shared css (now 2 of the same). That messed up some styling for us, but I see in the spec link someguy20336 added that they add the entry point css first then the shared css.

I guess it ends up working in most cases anyway, but in case someone else have a similar issue we fixed it by using css layers.

Eptagone commented 5 months ago

Hi, do you have a reproduction repo?

Hello, If you are still interested then I can give you link to my repo with test project where I encountered the same issue.

Yes, please

Here it is https://github.com/hdimon/ViteAspNetCoreTest/tree/multiple_css_files . multiple_css_files branch has been prepared to demostrate issue. Just do "npm run build" in ClientApp folder and then run application. If you want to see how it would look if it worked as expected then uncomment line 132 //cssInjectedByJsPlugin({ relativeCSSInjection: true }) in vite.config.ts, then do "npm run build" in ClientApp folder and run application. Let me know if you face any issues and thank you very much in advance!

Hi @hdimon. Thanks for the reproduction repo. Try again with the new v2.1 release. It should work now, so I'm closing the issue. If you find any details, feel free to open a new issue.

Eptagone commented 5 months ago

Ran into this yesterday. Seems more like a pretty important enhancement to me, given that it effectively isn't implementing the specification. Any reasonably sized project will probably result in several css files and thus they won't end up getting pulled in (probably?).

I might be interested in fixing it, but I need your input. I wrote my own TagHelper that correctly traverses the manifest (could share it here if interested), but the usage is slightly different than what you currently have designed. What do you think about this? I chose not to use a link considering it could result in any number of link tags (even though yea, you could probably do that with a link tag anyway).

<vite-styles entry-point="path/to/my/file.ts" />

Edit: on the other hand, I guess it could be a pretty big breaking change to change the tag helper, so my Idea is probably dumb. Let me know what you think

Hi @someguy20336. As you said, changing the tag helper syntax is a pretty big breaking change, so that's not an option. But if you have a better implementation, you can try to improve the current implementation and open a pull request.