Skip to content

Track discovered component dependencies without per-component caching opt-in - #2720

Open
joelhawksley wants to merge 2 commits into
mainfrom
joelhawksley-caching-pr-strategy
Open

joelhawksley wants to merge 2 commits into
mainfrom
joelhawksley-caching-pr-strategy

Conversation

@joelhawksley

Copy link
Copy Markdown
Member

What are you trying to accomplish?

Consolidate and supersede #2711 and #2713, building on Erik Axel Nielsen's fixes for silently incomplete component cache digests.

Once experimental caching is enabled, literal component renders and # Template Dependency: declarations should track every discovered ViewComponent::Base descendant, including components that never included ViewComponent::ExperimentallyCacheable. Changes to those dependencies must invalidate enclosing fragments rather than serve stale HTML.

What approach did you choose and why?

Use one shared registration path for literal renders and explicit declarations. Record the discovered component's actual class name and virtual path in the existing registry before emitting its synthetic dependency path. Resolution remains registry-based; it never attempts to reconstruct a class name using camelize.

This preserves acronym names and overridden virtual paths while allowing dependency tracking to follow transitive render trees. The registry continues storing names, not class objects, to avoid pinning stale autoloaded constants. Existing registration invalidation and idempotency are reused.

Keep activation and caching behavior separate: applications that never opt in do not discover or register dependencies; discovery does not enable component output caching or digest-aware cache blocks inside unopted-in component templates. Dynamic renders still need explicit declarations.

Unlike #2711's new resolver helper, this does not add a broad rescue. Unexpected autoload and digest failures continue to raise, preserving the policy merged in #2712. The component-template cache behavior merged in #2714 is unchanged.

Anything you want to highlight for special attention from reviewers?

Regression coverage includes unopted-in template and Ruby dependencies, explicit declarations, transitive invalidation through an unopted-in child with a custom virtual path to an acronym-named grandchild, actual fragment output after source changes, independent registration through both discovery paths, the disabled-feature boundary, and explicit-dependency autoload failures.

The sandbox's Zeitwerk inflector maps http_untracked_component to HTTPUntrackedComponent without changing Active Support's global inflections, so the acronym fixture specifically exercises a name that cannot be recovered through the former path.camelize proposal.

Validation:

  • Full default suite: 653 Minitests, 5 engine compatibility tests, and 8 RSpec examples, all passing.
  • Focused caching suites: 98 tests / 212 assertions, passing on Rails 8.1 with Ruby 4.0 and Rails 7.1 with Ruby 3.3.
  • Changed Ruby files and caching guide pass StandardRB; all new ERB templates pass ERB lint; git diff --check passes.
  • StandardRB reports two pre-existing code-block indentation offenses in the changelog, confirmed against unchanged main. YARD lint reports three pre-existing tag-group separator offenses in CacheDigest; none are introduced by this change.

Erik is credited as a co-author for the original proposals and regression coverage.

Consolidate the fixes proposed in #2711 and #2713. Register literal and declared dependencies by their actual class names and virtual paths, preserving acronym names, custom paths, and transitive invalidation without enabling output caching or suppressing digest errors.

Co-authored-by: Erik Axel Nielsen <erikaxel@lucalabs.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62a65b38-1933-46c3-aa8b-de40801e2c7f
Split the guide sentence flagged by Vale, remove directives for a Rails cop not loaded by StandardRB, and remove a redundant line continuation rejected by current RuboCop.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62a65b38-1933-46c3-aa8b-de40801e2c7f

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant