WMR creates references to CSS assets too early in build pipeline

[rschristian]

Describe the bug WMR creates references to CSS stylesheets too early, before the CSS minification plugin runs, resulting in references to CSS files that no longer exist. In practice, this creates an avoidable 404 error but no real harm; since the stylesheet has been merged into another, the styles still end up on the page just fine.

Imagine we have foo.css and style.css. style is referenced from the HTML, foo is imported into some JS.

WMR will process foo.css and will generate a <link> tag for the HTML, using the current asset filename (foo.<hash>.css) as the href value:

https://github.com/preactjs/wmr/blob/64efc9817c28feb5d573e9138fcbb55ea963f100/packages/wmr/src/plugins/wmr/client-prod.js#L8-L13

Later on in the build however, we reach a step to minify and merge CSS content. foo.<hash>.css gets merged into style.<hash>.css and ceases to exist. When we build our app, that <link> that tries to request foo.<hash>.css will 404 and fail.

To Reproduce

1. clone https://github.com/rschristian/wmr-leftover-css-asset-bug
2. Build app
3. Serve the built output
4. See 404 error

Expected behavior No 404

Desktop (please complete the following information):

• WMR Version: 2.1.0

Additional context Need to look at this more tomorrow, but I'm guessing it should just be an easy case of moving that HTML <link> generation to after the CSS minify step, to ensure we're not referencing assets that will no longer exist

[developit]

Ahh good find. Basically this bit of code: https://github.com/preactjs/wmr/blob/64efc9817c28feb5d573e9138fcbb55ea963f100/packages/wmr/src/plugins/optimize-graph-plugin.js#L273-L276

... needs to be moved to here: https://github.com/preactjs/wmr/blob/64efc9817c28feb5d573e9138fcbb55ea963f100/packages/wmr/src/plugins/optimize-graph-plugin.js#L35

(or, it might even be possible to simply reverse these two lines?)

[rschristian]

Sorry for the delay, definitely didn't "look at this more tomorrow", lol

I'm not sure that block is even an issue? It shouldn't the source of the issue in this example and I'm not sure it would ever be. The !cssImport would resolve to false, as the HTML does have a direct CSS import, so that block is never entered.

https://github.com/preactjs/wmr/blob/c18ad8978d3cc68c424e4a3cd910cdb464cfb27b/packages/wmr/src/plugins/optimize-graph-plugin.js#L261-L265

Seems fine to me, unless I'm misunderstanding?

~~The error I believe is here:~~

https://github.com/preactjs/wmr/blob/64efc9817c28feb5d573e9138fcbb55ea963f100/packages/wmr/src/plugins/optimize-graph-plugin.js#L365-L376

~~This is in hoistCascadedCss, which is called before hoistEntryCss~~

https://github.com/preactjs/wmr/blob/64efc9817c28feb5d573e9138fcbb55ea963f100/packages/wmr/src/plugins/optimize-graph-plugin.js#L29-L35

~~The problem being that it generates that \n${meta.styleLoadFn}(${url}); before the assets are finalized.~~

This is the problem child in the (prettified) bundle output:

function(e) {
    if (console.log(e), "undefined" == typeof document) wmr.ssr.head.elements.add({
      type: "link",
      props: {
        rel: "stylesheet",
        href: e
      }
    });
    else {
      if (document.querySelector('link[rel=stylesheet][href="' + e + '"]')) return;
      const n = document.createElement("link");
      n.rel = "stylesheet", n.href = e, document.head.appendChild(n)
    }
  }("/assets/foo.042a7a6a.css"),

And the debug message to go along with that is:

Hoisting CSS "assets/foo.042a7a6a.css" imported by index.53be4520.js into parent HTML import "assets/style.a59d1211.css".

~~Switching the hoistEntry & hoistCascade didn't immediately work and that's all I had time for tonight, but I imagine it needs minimal finagling which I can get tomorrow, assuming of course I'm not being super daft and misunderstanding an easy fix you suggested 😅~~

Edit: hm nevermind, that wasn't the issue area.