Changes to nested assets not reflected in parent asset checksum about broccoli-asset-rev HOT 25 OPEN

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 😢

from broccoli-asset-rev.

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

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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

from broccoli-asset-rev.

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

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

@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.

from broccoli-asset-rev.

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

from broccoli-asset-rev.

@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. 😛

from broccoli-asset-rev.

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

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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

from broccoli-asset-rev.

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

from broccoli-asset-rev.

@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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

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.

from broccoli-asset-rev.

Still an issue.

from broccoli-asset-rev.