Register dependency tracker so nested partials invalidate parent caches - #620
Register dependency tracker so nested partials invalidate parent caches#620rosa wants to merge 1 commit into
Conversation
Jbuilder ships `Jbuilder::DependencyTracker` and its railtie requires it, but nothing registers it with `ActionView::DependencyTracker`. Without a registered tracker, `find_dependencies` returns `[]` for every `.jbuilder` template, so the digestor sees no nested-partial dependencies: a `json.cache!` fragment that renders inner partials never mixes those partials' digests into its own cache key. Editing an inner partial then leaves outer fragments serving stale cached JSON until they happen to invalidate for some unrelated reason. ERB doesn't have this problem because Action View registers `ERBTracker`. This is a regression from rails#504, which rewrote the tracker as a standalone class and dropped the `register_tracker :jbuilder` call (and the `ERBTracker` inheritance that used to supply it) without re-adding it. It has shipped unregistered since 2.12.0. Register the tracker from the same `on_load(:action_view)` block that registers the template handler, and declare `supports_view_paths?` so `register_tracker passes` the view paths through to the tracker. Otherwise, Rails wraps it in a lambda that drops them and the tracker's own wildcard (`# Template Dependency: foo/*`) resolution silently returns nothing.
|
While I believe that this is a needed improvement that restores prior behavior, there have been several releases since the regression was introduced. Should integration with the dependency tracker be guarded by an engine configuration value, in case applications that depend on intermediate versions have relied on the absence of the dependency tracker integration? |
|
@seanpdoyle, thanks a lot for checking this! That's a good point 🤔 I think it'd be a bit strange to rely on stale cached content, where you might have a version of a template somewhere and another version somewhere else... maybe with different attributes and values... But it's possible! I did wonder why this had gone unnoticed for so long. I mean, we had this broken in our app since 2024. The reason was that we don't have that many examples of partial jbuilder templates inside a cached fragment, where we also change the partial, and the specific partial that triggered the bug for us changed very rarely. I suppose this is normal for folks as well? Personally, I wouldn't add a configuration setting for a bug like this because I can't imagine how you rely on this behaviour in particular, but maybe you have a case in mind? |
2 participants
We ran into a caching issue at Basecamp with a template that looked like this:
We changed the
people/_person.json.jbuildertemplate and deployed, yet responses using that template kept returning a stale cached version of the person.We traced this down to nothing registering Jbuilder's dependency tracker with Action View. Its railtie requires it, but nothing registers it with
ActionView::DependencyTracker. Without a registered tracker,find_dependenciesreturns[]for every.jbuildertemplate, so the digestor sees no nested-partial dependencies: ajson.cache!fragment that renders inner partials never mixes those partials' digests into its own cache key. Editing an inner partial then leaves outer fragments serving stale cached JSON until they happen to invalidate for some unrelated reason. ERB doesn't have this problem because Action View registersERBTracker.This is a regression from #504, which rewrote the tracker as a standalone class and dropped the
register_tracker :jbuildercall (and theERBTrackerinheritance that used to supply it) without re-adding it. It has shipped unregistered since 2.12.0.Register the tracker from the same
on_load(:action_view)block that registers the template handler, and declaresupports_view_paths?soregister_tracker passesthe view paths through to the tracker. Otherwise, Rails wraps it in a lambda that drops them and the tracker's own wildcard (# Template Dependency: foo/*) resolution silently returns nothing.