web icon indicating copy to clipboard operation
web copied to clipboard
verified publisher icon modernweb-dev

Erratic treatment of assets

Open morewry opened this issue 2 years ago • 4 comments

I'm having a number of issues trying to use this plugin. There are some unexpected and inconsistent results.

Versions

Node.js version 16.18.0

"@web/rollup-plugin-html": "^1.11.0",
"rollup": "^3.19.1",
"rollup-plugin-minify-html-literals": "^1.2.6",

Configuration

html({
  flattenOutput: true,
  input: resolve(cwd(), "./src/templates/dev/*.html"),
  minify: true,
  rootDir: resolve(cwd(), "./src/app"),
  publicPath: "/",
}),

Issues

FavIcons

The first inconsistency is treatment of favicons. When the rel has a space-separated value, it is not handled the same way.

This works as expected

Here the path is rewritten to match the fact that the plugin moves things to an assets folder and the file is moved as expected.

Input
<link rel="icon" href="/images/icons/favicon.ico" sizes="any" />
Output
<link rel="icon" href="/assets/favicon.ico" sizes="any" />
This does not work as expected

The file is also not moved to assets folder. No error or warning is shown, it is just silently skipped. IIRC this is an Android support thing for icons, but space-separated rel is valid, so this should be handled.

Input
<link
  rel="shortcut icon"
  sizes="196x196"
  href="/images/icons/favicon-196px.png"
/>
Output
<link
  rel="shortcut icon"
  sizes="196x196"
  href="/images/icons/favicon-196px.png"
/>
Hashes

One thing to note, however, is that none of the favicons get a filename hash. Historically, I did not hash favicons due to the influence of browsers' automatic request of root-level favicon.ico, but I don't see any reason not to do it these days and have done so for a number of years now. It's automatic with the build and the all icon URLs can be specifically defined.

JavaScript

I also saw unexpected behaviors with JavaScript modules. The behavior with paths and hashes is not consistent with other files, even other JavaScript files.

This works as expected

Here the path is rewritten to match the fact that the plugin moves things to an assets folder, the file is moved as expected, and it is also hashed.

Input
<script type="text/javascript" src="/scripts/global.js"></script>
Output
<script src="/assets/global-ee17209c.js"></script>
This does not work as expected

This is not placed in the assets folder and is also not hashed. The JavaScript files that would change between versions are one of the top kinds of files to hash.

Input
<script type="module" src="/scripts/component.js"></script>
Output
<script type="module" src="/component.js"></script>
Head vs Body

This unexpectedly and undesirably moved the component.js script tag with type=module to the bottom of body element when I had put into the head. I put in the head because that is where I wanted it to be. These days, if I need the behavior of a script in the body, I can do it sufficiently well with attributes on the script tag while having it in the head.

Misc Config Files

There are certain common configuration files that aren't handled appropriately. (In my context, I probably need to generate these a different way, but it's still noteworthy.)

This works as expected (mostly)

These are usually placed at the root and I'm not sure why it's placed in the assets folder when the JavaScript isn't. But overall it is handled as expected.

Input
<link rel="manifest" href="/manifest.webmanifest" />
Output
<link rel="manifest" href="/assets/manifest.webmanifest" />
This does not work as expected

I'm not entirely sure if Edge still uses this; it's needed if it does. Even if it's not, some people probably care about IE support. This just gets skipped. The path isn't rewritten, the file isn't moved.

Input
<meta name="msapplication-config" content="/browserconfig.xml" />
Output
<meta name="msapplication-config" content="/browserconfig.xml" />

Files in HTML

Files directly referenced in the HTML file are not included in the build output.

This does not work as expected
Input

The files that are referenced in the HTML file include these 17 files:

browserconfig.xml
comment.html
game.html
home.html
images/icons/favicon-120px.png
images/icons/favicon-152px.png
images/icons/favicon-180px.png
images/icons/favicon-196px.png
images/icons/favicon.ico
images/icons/favicon.svg
login.html
manifest.webmanifest
scripts/component.js
scripts/global.js
signup.html
store.html
styles/global.css
Output

The files that are output include these 15. Two are missing:

assets/favicon-120px.png
assets/favicon-152px.png
assets/favicon-180px.png
assets/favicon.ico
assets/favicon.svg
assets/global-15fca5cc.css
assets/global-ee17209c.js
assets/manifest.webmanifest
comment.html
component.js
game.html
home.html
login.html
signup.html
store.html

Files in CSS, JavaScript, etc.

Files indirectly referenced in the HTML, but directly referenced by the CSS, JavaScript, etc. do not have their paths rewritten and are not included in the build output. I was subconsciously expecting functionality like assetgraph. Obviously, the focus of this plugin is the HTML file, but if the rest of the indirectly referenced files aren't handled, things won't work. As far as I saw, this limitation and how to address it isn't mentioned in the documentation; it doesn't seem there's a way of using transformHtml or transformAsset?

This does not work as expected
Input

The additional files that are referenced via CSS, JS, etc. include these 15:

castle-alternate-unoptimized.svg
favicon-144px.png
favicon-16px.png
favicon-32px.png
favicon-64px.png
favicon-70px.png
favicon-96px.png
favicon-128px.png
favicon-144px.png
favicon-150px.png
favicon-192px.png
favicon-310px.png
favicon-512px.png
human-color-unoptimized.svg
land-unoptimized.svg
Output

All of these 15 files are missing and have the wrong paths, giving 404 errors when viewing the page in the event of their being requested.

morewry avatar Mar 15 '23 02:03 morewry

Just the beginning of an adventure here, but it does look like shortcut is not included as an asset rel for link.

Not sure if it needs to be added individually, or if it would be better to test for icon as a token list rather than an exact attribute, the way that is does here.

Would be happy to accept a PR clearing up this situation and any follow ups to get this erratic nature of asset inclusion cleared up for you!

Westbrook avatar Mar 17 '23 12:03 Westbrook

@Westbrook Thank you! With those leads, I may be able to contribute a PR. I really appreciate the breadcrumbs. If I have any questions, can I contact you directly? (i.e. on Twitter?)

morewry avatar Mar 20 '23 20:03 morewry

I'm not on Twitter right now. But, if you're into that sort of thing, you can catch me on Mastodon at https://mastodon.social/@westbrook Also happy to chat here while you work through any of the bigger pieces.

Westbrook avatar Mar 20 '23 22:03 Westbrook

I ran into this as well, planning to work on this.

Current list of things I want to improve:

  • add more link rels into account (e.g. "preload" and "prefetch")
  • hash everything by default (get rid of currently hardcoded types that are never hashed)
  • document better how to deal with exception if needed by using rollup's "assetFileNames"
  • make sure relative paths always work, will add more unit tests and different folder compositions
  • revisit all unit tests, write them in a more robust way
    • always verify full paths rather than just filenames
    • test different scenarious of "assetFileNames", e.g. when it has no nested folders or has nested folders
    • cover more edge cases when same files are referenced in multiple different places like HTML, CSS and JS

bashmish avatar Aug 06 '25 11:08 bashmish