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

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

Closed
pete-intranel opened this issue Mar 14, 2024 · 10 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@pete-intranel
Copy link

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);
}
@pete-intranel pete-intranel added the enhancement New feature or request label Mar 14, 2024
@Eptagone Eptagone self-assigned this Mar 15, 2024
@firegodjr
Copy link

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
Copy link
Owner

Hi, do you have a reproduction repo?

@hdimon
Copy link

hdimon commented May 26, 2024

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
Copy link

Kalshu commented May 31, 2024

I'm also interrested by this feature

@Eptagone
Copy link
Owner

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
Copy link

hdimon commented May 31, 2024

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
Copy link
Contributor

someguy20336 commented Jun 8, 2024

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
Copy link

jsaele commented Jun 13, 2024

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
Copy link
Owner

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants