Changes to nested assets not reflected in parent asset checksum

[shilov]

Checksums of files that reference other assets should take into account the names of those nested assets after they've been revisioned.

A simple example of when this surfaces is anytime you have a file such as a CSS file, referencing another file such as a background image, and the background image is changed but not renamed. In this instance, the CSS file checksum remains the same, even though it correctly points to the new background image checksum.

I don't actively use broccoli so I can't offer much insight beyond this bug report, but I figured it might help given that those of us who actually care about fingerprinting are also the ones affected by such issues :) Hope this helps.

[rickharrison]

Why should it matter whether the fingerprint matches the before replacement code or the after? Either way, it is a unique reference.

[shilov]

Well, to use my original example, if the CSS file has been cached by users then they'll continue to see the same image. The problem becomes even more complex when you consider CDNs that rely on filenames being unique.

[rickharrison]

Do you have any suggestions on how to combat this problem? There is a race condition of updating the file, then computing checksum, then updating the file, etc.

[shilov]

I'm not familiar with Broccoli but we work around this issue by revisioning in stages, with images and fonts being first, then CSS and then JS. It's tedious, esp. because we use JS to load other JS.

I'm considering rewriting our deployment process to scan for file references then using the relationships to determine the revisioning order.

Sorry if this isn't very helpful, I have no idea how practical these suggestions would be in the context of Broccoli.

This may not be an all too common problem, but it might help to make mention of this somewhere to warn people because it isn't something people would typically look out for.

[rickharrison]

Are there any use cases other than the following, which would trigger this issue:

1. Deploy app to production
2. Update an image, while not changing the filename at all.
3. Make no other changes other than the image change
4. Re-deploy

[shilov]

That's pretty much it. Probably just as likely to affect assets referenced in templates.

[rickharrison]

@rwjblue Any thoughts on this issue? I'm not sure how to do this without being too opinionated about the build process.

[rickharrison]

So one thing you could do is just run the assetRev plugin twice. First you could rev images and rewrite css, and then you can run it again and rev the css.

[pwfisher]

Our bespoke build tasks (which we are now replacing with broccoli-asset-rev) had a two-step revisioning process. We defined "leaf assets" (with no references to other versioned assets) and versioned them in an initial step before processing CSS and JS.

[krisselden]

The idea there are "safe" changes to make to an asset post fingerprinting is fundamentally flawed, especially something like the 'prepend' option. This would over write existing CDN assets (that are long lived) with a CSS that points at a new host, this is completely against the point of fingerprinting.

If you are fingerprinting content correctly, then at deploy time, you should never overwrite an existing asset.

[stefanpenner]

So one thing you could do is just run the assetRev plugin twice. First you could rev images and rewrite css, and then you can run it again and rev the css.

It's theoretically, N deep, running multiple times wont fix the issue in all cases.

chain:

index.html -> css -> css -> image

I don't have time to make the implementation changes, but I will gladly work with you (or something who has time to implement) to ensure we do the right thing here.

[nathanhammond]

@rickharrison This is likely to become more of an issue in the world of Ember CLI >= 2.7 world. We explicitly document how to use prefixes which will, on next build, not be updated.

The solution for this requires doing a walk of every single asset and then building them into an inclusion graph. If that turns out to be a tree then we can simply walk the tree by depth up to the root.

Circular references will of course ruin our day on that one, and we'll need to detect if that occurs in the graph. For circular references we'll need a different strategy. I propose that any asset that is part of a circular reference gets an unmodified on rewrite hash and an appended timestamp.

[rickharrison]

Do you see this as being a change to asset-rev or is this really a re-write?

[nathanhammond]

@rickharrison There's a fair amount of work to do this but I believe it still belongs in this addon. It's a rewrite of some of the core pieces of this addon, but a lot of the supporting structure and API doesn't need to change so it isn't a full rewrite. Note that I'm not volunteering you for this effort, unless you want to do it. 😛

[stefanpenner]

@nathanhammond we should sync up with @chadhietala as he has some incremental dep graph work we can piggy back on.

[GCheung55]

Is there any update or link to the dep graph work that @chadhietala did? I'm curious to see how it could be leveraged here.

[nathanhammond]

@GCheung55 That's not actually correlated here. This involves introspection into file content to handle replacement and running leaf-to-root.

[blimmer]

I just ran into this problem as well. The repro steps above are definitely the problem I ran into. This definitely was surprising to me because the JS references to the file updated correctly, but not the CSS. We didn't notice this until we deployed to prod and someone noticed in QA 😢

[blimmer]

does anyone have insight into the status of this issue? this issue has required us to manually cache-bust filenames of images we're only using css (we're just cache-busting all images to be safe right now) and it has required us to turn off SRI.

cc @stefanpenner

[nathanhammond]

@blimmer Nobody is working on this at present. It needs to be converted to use https://github.com/assetgraph/assetgraph to do the right thing.

[jamesarosen]

I just experienced this via background-image. We changed the SVG image, which got a new fingerprint. The next build inserted the right reference into the CSS, but didn't update the CSS file's fingerprint. The old one with that fingerprint is cached forever in browsers.

[pzuraq]

We experienced this too, caused a conflict with SRI which caused some issues. We can't turn off SRI so the only option for us in the meantime is to specify a non-deterministic "hash" function.

[cyril-sf]

in our case, the root cause was a change in the sourcemaps but not in the js file.

We suspect that a package might have been updated between builds (and it shows that we need to switch to yarn) leading to different sourcemaps but same js file.

But we'd have hoped that this was cover by Ember-CLI. It seems like the build steps need to be different:

1. build js
2. build sourcemaps
3. fingerprint sourcemaps
4. append sourcemaps to js file
5. fingerprint js file

Not sure how it's done today.

[machty]

Also hitting this due to a somewhat strange setup in our build tooling for how we develop/test cordova apps: we use a test version of cordova.js which dynamically inserts a script tag to log a sibling cordova_plugins.js file. When cordova_plugins.js changes, it gets a new fingerprint, which correctly updates the reference in cordova.js, but cordova.js's SHA doesn't get recomputed so it ends up not uploading to S3. Gonna try to run asset rev twice.

[MiguelMadero]

Still an issue.