-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Digests of files that can have dependencies on other files in the load path need to reflect those dependencies #188
Conversation
…d path need to reflect those dependencies
For some reason, I'm not getting digested assets in CSS files with this. I do get digests from the asset helpers though. |
@wlipa What do you mean exactly? That |
Ah, I see the problem. |
In content_with_compile_dependencies, the references will need to be returned in a stable order in order for digests to be stable. Maybe they are if it's based on source code order? Not 100% sure on set merging. |
Should be stable given the scan will always return them in order. But good question about set guaranteeing a stable order. Will investigate. |
As far as I can tell, since Ruby 2.5, the insert order is guaranteed in a set, and that includes merging. So we should be good. |
This is checking out for me now. 👍🏼 |
Thanks for your help with this, @wlipa. Got some good inspiration for this approach from your work, and I think we ended up in a nice place without complicating the domain model much. |
Yeah sounds good. It's definitely simpler. The only thing I noticed that's a bit of a trade-off is reading all the dependent assets into memory to do that sum. Could be a lot with big images. But maybe it's not that much of a problem in practice. |
Yeah but we read them into memory anyway to digest them individually. Although of course it's perhaps a higher peak if a single css file references a ton of images. Still, doubt it's an issue. Your images shouldn't be that big in the first place if referenced by css! |
CSS files that reference other CSS files via
@import
or images viaurl()
need to have their digest reflect changes in the dependencies. This PR adds that by including the content of all dependencies in the calculation for such assets with dependencies.This takes some inspiration from #175 but avoids the dependency tree tracking.