Pass the template format to the digestor #35293
Conversation
This commit passes the template format to the digestor in order to come up with a key. Before this commit, the digestor would depend on the side effect of the template renderer setting the rendered_format on the lookup context. I would like to remove that mutation, so I've changed this to pass the template format in to the digestor. I've introduced a new instance variable that will be alive during a template render. When the template is being rendered, it pushes the current template on to a stack, setting `@current_template` to the template currently being rendered. When the cache helper asks the digestor for a key, it uses the format of the template currently on the stack.
I've chased down a bunch of |
I only removed those because they had no impact on the change. I can undo that change, but it has no impact on the tests. Since the digestor doesn't look at the |
| @@ -234,7 +234,7 @@ def fragment_name_with_digest(name, virtual_path, digest_path) | |||
| if virtual_path || digest_path | |||
| name = controller.url_for(name).split("://").last if name.is_a?(Hash) | |||
|
|
|||
| digest_path ||= digest_path_from_virtual(virtual_path) | |||
| digest_path ||= digest_path_from_template(@current_template) | |||
There was a problem hiding this comment.
I'm taking this is the main difference? We've now captured the currently rendering template and can derive the format directly from it?
| else | ||
| cache_key = [ name, finder.rendered_format, dependencies ].flatten.compact.join(".") | ||
| cache_key = [ name, format, dependencies ].flatten.compact.join(".") |
There was a problem hiding this comment.
I don't see how the old code would get a rendered_format here? Did you already consolidate rendered_format= on the finder/lookup_context or am I misremembering?
There was a problem hiding this comment.
It's pretty confusing, but the template renderer would set it on the lookup context. The partial renderer could do it too.
There was a problem hiding this comment.
Uhhh, right, they both set the same info but from slightly different vantage points. Got it, thanks 👍
This commit passes the template format to the digestor in order to come
up with a key. Before this commit, the digestor would depend on the
side effect of the template renderer setting the rendered_format on the
lookup context. I would like to remove that mutation, so I've changed
this to pass the template format in to the digestor.
I've introduced a new instance variable that will be alive during a
template render. When the template is being rendered, it pushes the
current template on to a stack, setting
@current_templateto thetemplate currently being rendered. When the cache helper asks the
digestor for a key, it uses the format of the template currently on the
stack.
I removed the
rendered_format=side effect in #35265 but the tests failed. It turns out that the digest caching code depends on that side effect, so this PR aims to remove the digest cache's dependency on therendered_formatsetter. This PR fixes the Railties test in #35265