Incorrect paths after asset compile

issues
1

Hiya :wave:

From the guides, it’s a feature that e.g. /app/assets/fonts/hack/hack-regular.woff2 is copied to /public/assets/hack/hack-regular.woff2 during asset compile obviously dropping the first hierarchy fonts/.

However, this is a problem when you reference fonts, background images etc from another stylesheet. I’ve created a simple test app to illustrate:

After bootstrapping, I’ve just added a fonts directory, added the stylesheet for it and added it via the app.js entrypoint:

Running hanami assets compile works fine and produces the following assets in public/:

As expected the hack.woff2 is created without fonts/ in its path:

That’s not the case in the compiled app.css which contains the fonts/ path segment.

Also, possibly unrelated, the path in the compiled app.css descends below the public/ webroot for static assets with ../../ which doesn’t seem right.

Am I doing something wrong here or could this be a bug?

/cc @jodosha @timriley

2

Thanks for this great summary, @svoop!

I’ve just replicated this here locally too, and IMO this is definitely a bug.

Per esbuild’s CSS docs, we do configure a loader for font files, but this clearly isn’t doing the job we need it to do.

To test this locally, I put a file in app/assets/fonts/ and referred to it from a CSS file just like your example above.

Then I edited my config/assets.js to increase the level of logging from esbuild:

await assets.run({
  esbuildOptionsFn: (args, esbuildOptions) => {
    esbuildOptions.logLevel = "verbose";

    return esbuildOptions;
  },
});

And then when I ran hanami assets compile, I saw this relevant bit of output:

[two_one] ⬥ [VERBOSE] Resolving import "../fonts/comic-mono.ttf" in directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css" of type "url-token"
[two_one]
[two_one]   Checking "../fonts/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/images/*"
[two_one]   Checking "../fonts/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/*"
[two_one]   Read 1 entry for directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css"
[two_one]   Checking "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/images/*"
[two_one]   Checking "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/*"
[two_one]   The path "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/comic-mono.ttf" was marked as external by the user
[two_one]   Read 5 entries for directory "/Users/tim/Source/hanami/scratch/two_one/app/assets"
[two_one]   Read 1 entry for directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts"
[two_one]   Primary path is "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/comic-mono.ttf" in namespace "file"

See here how it’s mentioning that the font was “marked as external?” This is an esbuild feature that can indicate files to exclude from the build. And in assets-js, we actually mark all directories aside from css/ js/ as external. This is what allows us to then handle the files in those directories separately (such as images), and then later copy them across into the compiled assets directory as part of our hanami-assets esbuild plugin.

However, what we’re discovering is that this also means that files in those external directories cannot be referenced from within JS or CSS files and have their file references appropriately updated for the resulting bundle.

To test this for sure, I moved the font directly inside the app/assets/css/ directory, and referred to it there:

@font-face {
  font-family: "comic-mono";
  src: url("./comic-mono.ttf");
}

And then when I ran hanami assets compile again, we see more encouraging output:

[two_one] ⬥ [VERBOSE] Resolving import "./comic-mono.ttf" in directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css" of type "url-token"
[two_one]
[two_one]   Checking "./comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/images/*"
[two_one]   Checking "./comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/*"
[two_one]   Read 2 entries for directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css"
[two_one]   Checking "/Users/tim/Source/hanami/scratch/two_one/app/assets/css/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/images/*"
[two_one]   Checking "/Users/tim/Source/hanami/scratch/two_one/app/assets/css/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/*"
[two_one]   Read 2 entries for directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css"
[two_one]   No "browser" map found in directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css"
[two_one]   Attempting to load "/Users/tim/Source/hanami/scratch/two_one/app/assets/css/comic-mono.ttf" as a file
[two_one]     Checking for file "comic-mono.ttf"
[two_one]     Found file "comic-mono.ttf"
[two_one]   Checking "/Users/tim/Source/hanami/scratch/two_one/app/assets/css/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/images/*"
[two_one]   Checking "/Users/tim/Source/hanami/scratch/two_one/app/assets/css/comic-mono.ttf" against the external pattern "/Users/tim/Source/hanami/scratch/two_one/app/assets/fonts/*"
[two_one]   Read 2 entries for directory "/Users/tim/Source/hanami/scratch/two_one/app/assets/css"
[two_one]   Primary path is "/Users/tim/Source/hanami/scratch/two_one/app/assets/css/comic-mono.ttf" in namespace "file"

Note above that it no longer says “marked as external by the user”, and instead it shows that it’s using the file loader for the font file in the “attempting to load … as a file” line.

And then if I inspect the compiled CSS, things look good too, with it referring to a properly hashed file, and that file actually existing alongside it inside public/assets/:

@font-face{font-family:comic-mono;src:url("./comic-mono-5YVBLHHO.ttf")}

So what’s the takeaway here?

  • We have a bug where files outside of the css/ and js/ assets directories cannot be directly referenced within CSS and JS files and properly included in the bundle
  • This is because we make those directories as “external”, which means esbuild doesn’t process those files
  • I wonder if we even need that external config at all; what happens if we just take it away? We do currently use it inside our esbuild plugin to get the list of dirs whose assets we should manually copy, but what if we just did a direct filesystem crawl at that point (the one we currently do here), instead of using build.initialOptions.external? That might still give us the overall behaviour we want while avoiding this bug for files directly referenced from CSS/JS.

I think this approach is worth pursuing. @svoop — are you interested in having a go?

3

@timriley Thanks a lot for looking into this so swiftly. Your conclusion sounds like a plan: Since (if I understand correctly) only non-external directories can be referenced in other CSS and JS, external directories have to be avoided by implication.

As a side note: From a user perspective, I’m not sure whether collapsing the path to the compiled assets (in my example above by dropping the fonts/ directory) is a good idea. For one, it goes against the principle of least surprise, but more seriously, relative paths like url(../fonts/foobar) break unless they are adapted. And there’s also an elevated risk of collisions e.g. assets/dir1/foobar.jpg being overwritten by assets/dir2/foobar.jpg. I’m not sure about the motives for this collapsing other than maybe shorter paths, but these alone are not worth the hassle IMO because asset helpers hide them from templates anyway.

Finally, I’m interested to contribute to Hanami at some point, yes, but I’m afraid it’s a little early for this. I’m diving into the Ruby code as often as I can and try to make sense of it, but to be honest, I regularly feel like an apprentice surrounded by wizards. :smile: Also, this particular issue involves more TypeScript than Ruby and I’ve never coded TypeScript so far. I’d prefer to work some lower hanging fruit for starters. (Something like Ignore file system meta files like .DS_Store · Issue #141 · hanami/assets · GitHub is probably more my collar size for now.)

4

Encountered the same problem, except my assets were in the slice. Instead of moving them directly into css, I nested them in fonts dir and moved that into css folder. When compiled, they end up in public/assets/_main/css without any further nesting and are used correctly in the compiled file.

I think I understand the issue Tim explained, but also have no experience with TS. Nevertheless I would like to take a crack at this bug and try to resolve it. Nesting fonts in css seems weird. I’ll let you know if I make any interesting progress

5

FYI, I’m already part-way through the fix here: Don't mark non-{js,css} dirs as external by timriley · Pull Request #26 · hanami/assets-js · GitHub

6

I’ve posted a PR with my attempt on helping with the issue Handle duplicates of external compilations by krzykamil · Pull Request #27 · hanami/assets-js · GitHub

7

Thanks to @krzykamil’s excellent work, my original PR (Don't mark non-{js,css} dirs as external by timriley · Pull Request #26 · hanami/assets-js · GitHub) is now finished and ready for some final tests before merging.

@krzykamil and @svoop — would you be able to give this one more try?

8

@timriley Just did a couple of tests on Commits · svoop/testli · GitHub and it’s all looking good now.

Also, I’ve tried what happens when images or fonts collide in name, all good as well, e.g. the first font ends up in the public assets root (subdir dropped), the colliding second in its own subdir. I don’t really understand the advantage of this, but there’s no collision, so there’s no problem.

Thanks a lot @timriley and @krzykamil !! (Hope I can be of more help next time.)

9

@svoop Thanks for checking!

Could you please share the details of your asset structure here (specifically the two fonts), and how you’re referring to them? I want to make sure we’re handling this use case sensibly.

10

Glad I could help :slight_smile:
Tested this on my app again with different structure of my assets and all cases looked fine.

1 Like
11

Actually, after deleting the public folder, and running Procfile again I am getting a weird error,
No asset found for "app.css"

this is from:
slices/main/templates/layouts/app.html.erb
<%= stylesheet_tag "app" %>

But this is probably not connected to the same thing, since it is not loading the whole app.css file, cannot find it.
I cannot really debug it, because:

>> Hanami.app["assets"]["app.js"]
!! #<Dry::Core::Container::KeyError: key not found: "assets">

Maybe this is something wrong with my config, or something I messed up during upgrade to 2.1.0 from release candidate version, but I prefer to mention it anyway, gonna work on it later today and see why is it completely missing the compiled assets.

12

@krzykamil — if you want to debug the assets for a slice, you’ll want to use the "assets" component for that slice in particular. In your case, that would be Main::Slice["assets"] (for files in slices/main/assets/) instead of Hanami.app["assets"] (which is for files in app/assets/).

1 Like
13

Sure, it’s in fact the repo I mentioned in my last comment, just check the last four commits. (I’ve reworded them for clarity.)

The colliding images are (symmetrically) compiled as follows:

app/assets/images/dog1/dog.jpg -> public/assets/dog1/dog.jpg
app/assets/images/dog2/dog.jpg -> public/assets/dog2/dog.jpg

On the other hand, the colliding fonts are (asymmetrically) compiled as follows:

app/assets/fonts/hack/hack-regular.woff2  -> public/assets/hack-regular.woff2
app/assets/fonts/hack2/hack-regular.woff2 -> public/assets/hack2/hack-regular.woff2
                                                           ^^^^^^

You see this in tree GitHub - svoop/testli at after-fix

14

Thanks again for your feedback here, @svoop.

Based on your particular example, where you have e.g. multiple assets in a directory, but only a subset directly referenced by esbuild ("fonts/hack/hack-regular.woff2" and "fonts/hack2/hack-regular.woff2"), I’ve made some adjustments so both of those assets will have consistent keys in the manifest.

They’ll now have keys like this, based on their source files:

  "hack/hack-regular.woff2": {
    "url": "/assets/hack-regular-NP4XGBPU.woff2"
  },
  "hack2/hack-regular.woff2": {
    "url": "/assets/hack2/hack-regular-0B0EF254.woff2"
  }

Whereas previously, they keys would have been:

  • "hack-regular.woff2", for the file processed by esbuild, and
  • "hack2/hack-regular.woff2", for the file not processed by esbuild, and instead just copied over by hanami-assets

If we had the situation where there were e.g. three same-named files (but in different directories) and esbuild bundling two of them due to references within CSS, then I think this would have led to manifest key collisions, and made it impossible to access one of those assets via the manifest, so this is a good fix to be making!

FYI, this is the specific commit in the PR adding this change. I also added this commit to ensure none of the above-mentioned key collisions occur.

I’ve been testing this inside your app (thank you for publishing it!), and I did have to make one other change: I had to remove the ?sha= query string from your references to the font files. Specifically, I had to change this:

@font-face {
  font-family: 'Hack';
  src: url('../fonts/hack/hack-regular.woff2?sha=3114f1256') format('woff2');

To this:

@font-face {
  font-family: 'Hack';
  src: url('../fonts/hack/hack-regular.woff2') format('woff2');

This is because my updated code to generate the consistent manifest keys relies on the input files as determined by esbuild, which in the first case was "app/assets/fonts/hack/hack-regular.woff2?sha=3114f1256".

Naturally, we don’t want that ?sha=xxx text inside our manifest keys :slight_smile: Is there a particular reason that you needed it in your CSS file like this? If not, then I think we’re good to go and I don’t need to do any extra work. But if for some reason this practice is necessary or commonplace (though I had no luck searching the internet for an explanation of why it might be necessary for a CSS font-face src url), then I’ll have to do some extra work to explicitly strip those query params when generating the asset manifest keys.

Aside from this, I’ve tested asset compilation inside your app with the current state of Allow references to files outside js/ and css/ dirs by timriley · Pull Request #26 · hanami/assets-js · GitHub and I’m happy with how it’s working.

Would you mind taking another look yourself, just to confirm?

Thanks again helping me out with this work, it’s going to make for a much better end result! :raised_hands:

15

No, not at all. Some sources of web fonts add this parameter as a cache buster, so there’s a slight risk of people falling into this trap when they integrate a font CSS with their app. So maybe a warning (one of those blue thumbs-up boxes) in the assets guide would be worthwile.

I’m to thank, for all your great work on these inspiring projects. Glad to help just a little, hopefully a bit more in the future.

I’ll do a few more trials these days.

16

Everything looks fine as far as I can see. Also, all the problems I had with my pet project Hanami app have vanished after upping to this fix. :tada:

Thanks a bunch and Happy Easter! -sven

1 Like
17

Thanks again for your help, @svoop! All these changes are now released as hanami-assets (npm package) v2.1.1.