Skip to content
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

provide cache digest for view #542

Merged
merged 0 commits into from
Aug 12, 2022
Merged

provide cache digest for view #542

merged 0 commits into from
Aug 12, 2022

Conversation

ermolaev
Copy link

@ermolaev ermolaev commented Dec 3, 2020

fix if changed view, cache not expired

but, I think dynamicly modify view_context.lookup_context.view_paths bad for perfomance

#234

@ermolaev ermolaev force-pushed the master branch 2 times, most recently from fdde2c4 to d098bf5 Compare December 5, 2020 10:55
@@ -52,7 +52,9 @@ def render_in(view_context, &block)
self.class.compile(raise_errors: true)

@view_context = view_context
@lookup_context ||= view_context.lookup_context
@lookup_context ||= view_context.lookup_context.tap do |c|
c.view_paths.unshift(Rails.root.join("app/components"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do this at the Railtie level instead of the component level?

I'm a little concerned about performance since this would run at least once per-component render.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cached lookup_context in class attribute, bun it work only rails 6.0, 6.1, older versions is a hard work for support

@joelhawksley
Copy link
Member

@ermolaev thank you for the PR!

I think we need to work to find a solution that works consistently across our supported Rails versions (>=5). I'm wary of introducing incomplete support for caching in the manner proposed by this PR.

I recognize that this is probably not the answer you wanted to hear, and that taking on caching support for ViewComponent is no small effort, but I hope you agree that we need to take the time to do this right ❤️

Base automatically changed from master to main December 21, 2020 21:44
@stevehodgkiss
Copy link

This looks like it's almost there for Rails 6! It works for cache blocks with no dependencies, but with dependent view components the dependency tracker in ActionView fails find them. For example, with <%= render Shared::Something.new %> inside cache "key" do it logs "Couldn't find template for digesting: Shareds/Shared". To properly digest templates with dependencies, an extension or replacement of the ERB tracker in ActionView::DependencyTracker is probably needed, to return template paths for view component render calls.

@joelhawksley joelhawksley removed their request for review March 8, 2021 16:12
@ermolaev ermolaev merged commit 4241a14 into ViewComponent:main Aug 12, 2022
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.

4 participants