Track components that haven't opted into cacheability - #2713
Draft
erikaxel wants to merge 1 commit into
Draft
Conversation
- Resolve any ViewComponent::Base descendant as a dependency rather than only registered ones, so a digest can't be silently partial - Resolve unregistered components back from their virtual path, requiring the path to round-trip to the same class - Add regression tests for an untracked child changing without moving the parent's digest or the fragment digest
erikaxel
force-pushed
the
vc-digest-track-all-components
branch
from
September 3, 2026 11:56
d7a7a5a to
3aa981d
Compare
This was referenced Sep 3, 2026
erikaxel
marked this pull request as draft
September 3, 2026 12:25
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Dependency tracking is transitively opt-in.
CacheDigest.constantize_componentreturnsnilunless the constant responds to__vc_cacheable?, so a parent that includesExperimentallyCacheablegets a digest covering only those children that also included it. The digest looks complete and is silently partial. The failure mode is stale HTML with no signal at all.Reproduction
An
app/viewspartial with acacheblock rendering two components, one tracked and one not:Step 3 is the interesting one: the untracked component's change sat invisible until an unrelated tracked component happened to change, then both appeared at once. Stale content surfaces at an arbitrary later time, triggered by an edit somewhere else entirely.
We hit the same thing in our own tree: editing a modal component didn't bust the digest of the component that renders it until the modal itself included the module.
Impact
You can't tell by reading a component whether its digest is complete — you have to walk the whole render tree and check each child for the include. In a codebase of any size that's the difference between the feature being usable and being a trap, and it gets worse as more components are added by people who don't know the rule.
Fix
Once the feature is active at all, any
ViewComponent::Basedescendant counts as a dependency rather than only registered ones. The registry gate exists so applications that never opt in pay nothing, and that property is preserved:dependencies_instill returns[]while the registry is empty. Only the per-constant__vc_cacheable?filter is gone.component_forhas to resolve the other direction too, since an unregistered component isn't in the registry to look up. Registered components are still looked up by name; everything else is derived from the path by reversing the underscoringViewComponent::Baseapplies to the class name, and the derived constant has to underscore back to the same path. A component that overridesvirtual_pathis digested under a path its name doesn't lead to, so it's left unresolved rather than confused with another component — that case still needs the include, and the guide says so.Constantizing already happened before the filter, so no new autoloading: the same constants are resolved, they're just no longer discarded.
Alternative
If you'd rather keep the narrow behaviour, the fallback ask is a development/test warning naming the untracked child, so the gap is at least visible. Happy to rework this into that instead.
Tests
Fail on
main, pass here:test_cache_block_is_invalidated_when_an_untracked_component_changesandtest_cached_markup_of_an_untracked_component_is_not_served_stale(integration) — acacheblock rendering one tracked and one untracked component, mirroring the reproduction above.test_cache_digest_changes_when_an_untracked_child_component_template_changesand..._ruby_file_changes— a cacheable parent whose child never opted in.test_dependencies_are_found_for_components_that_did_not_opt_inandtest_component_for_resolves_a_component_that_did_not_opt_in— unit coverage for both directions.Plus
test_component_for_ignores_a_component_that_moved_its_virtual_pathandtest_dependencies_ignore_constants_that_are_not_componentsfor the guards, and three existing tests updated to the new behaviour.bundle exec rakepasses on Rails 8.1,main, and the default Gemfile; the cacheable suites pass on 7.1, 7.2 and 8.0. The only failures seen elsewhere were pre-existing and unrelated (allocation-count assertions inRenderingAllocationsTeston 8.0, and the Ruby/Rails version-matrix fixture on 7.1/7.2 when run under a Ruby other than the pinned one) — both reproduce onmainunchanged.