Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ nav_order: 6

## main

* Share sidecar directory listings and source reads during cold cache digests, and hash shared Ruby and sidecar files once per digest computation or boot-time precomputation.

*Joel Hawksley*

* Avoid missing-template digest errors for unresolvable interpolated render paths in components and included helpers.

*Joel Hawksley*
Expand Down
8 changes: 4 additions & 4 deletions lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -667,10 +667,10 @@ def sidecar_files(extensions)
(sidecar_files - [identifier] + sidecar_directory_files + nested_component_files).uniq
end

# `sidecar_files`, looked up once per component while eager loading:
# every component looks up its superclasses' templates and translations
# too, when it compiles, loads its translations, and computes its cache
# digest.
# `sidecar_files`, looked up once per component during eager loading or a
# digest computation: every component looks up its superclasses' templates
# and translations too, when it compiles, loads its translations, and
# computes its cache digest.
#
# @private
def __vc_sidecar_files(extensions)
Expand Down
18 changes: 10 additions & 8 deletions lib/view_component/cache_digest/digestor_lookup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,18 @@ def digest(finder:, **)
# everything it renders, which for components rendering a shared helper
# module is most of the application.
def tree(name, finder, partial = false, seen = {})
pruned = CacheDigest.memo&.fetch(:pruned, nil)
cached = finder.digest_cache[name] if pruned && partial
return super unless cached
CompileCache.with_sidecar_files_cache do
pruned = CacheDigest.memo&.fetch(:pruned, nil)
cached = finder.digest_cache[name] if pruned && partial
next super unless cached

path = ActionView::TemplatePath.parse(name)
template = find_template(finder, path.name, [path.prefix], partial, [])
return super unless template
path = ActionView::TemplatePath.parse(name)
template = find_template(finder, path.name, [path.prefix], partial, [])
next super unless template

pruned[name] = cached
seen[template.identifier] ||= ActionView::Digestor::Node.create(name, name.gsub(%r{/_}, "/"), template, partial)
pruned[name] = cached
seen[template.identifier] ||= ActionView::Digestor::Node.create(name, name.gsub(%r{/_}, "/"), template, partial)
end
end

private
Expand Down
8 changes: 5 additions & 3 deletions lib/view_component/cache_digest/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,10 @@ def source_files(component)
end

def ruby_files(component)
component_ancestors(component).map(&:identifier).concat(module_files(component))
.compact.uniq.select { |path| ::File.exist?(path) }
CacheDigest.memoize([:ruby_files, component]) do
component_ancestors(component).map(&:identifier).concat(module_files(component))
.compact.uniq.select { |path| ::File.exist?(path) }
end
end

# Sidecar template files plus inline templates, which live in the Ruby
Expand Down Expand Up @@ -243,7 +245,7 @@ def ruby_sources(component)
end

def file_digest(path)
ActiveSupport::Digest.hexdigest(::File.binread(path))
CacheDigest.memoize([:file_digest, path]) { ActiveSupport::Digest.hexdigest(::File.binread(path)) }
end

# Built once at load time rather than memoized, so no class-level state
Expand Down
31 changes: 17 additions & 14 deletions lib/view_component/compile_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,28 @@ def invalidate!
cache.clear
end

# Source files can't change while the application eager loads: share
# directory listings, sidecar lookups, and file contents within the block.
# Fiber storage keeps the caches from surviving boot.
# Share directory listings, sidecar lookups, and file contents within one
# eager load or digest computation, but never across reloads.
def with_sidecar_files_cache
Fiber[:__vc_sidecar_files] = {}
Fiber[:__vc_sidecar_directories] = {}
Fiber[:__vc_source_files] = {}
Fiber[:__vc_path_parser] = ActionView::Resolver::PathParser.new
yield
ensure
Fiber[:__vc_sidecar_files] = nil
Fiber[:__vc_sidecar_directories] = nil
Fiber[:__vc_source_files] = nil
Fiber[:__vc_path_parser] = nil
return yield if Fiber[:__vc_sidecar_files]

begin
Fiber[:__vc_sidecar_files] = {}
Fiber[:__vc_sidecar_directories] = {}
Fiber[:__vc_source_files] = {}
Fiber[:__vc_path_parser] = ActionView::Resolver::PathParser.new
yield
ensure
Fiber[:__vc_sidecar_files] = nil
Fiber[:__vc_sidecar_directories] = nil
Fiber[:__vc_source_files] = nil
Fiber[:__vc_path_parser] = nil
end
end

def path_parser
# Its regular expression depends on registered handlers, formats and locales.
# Reuse it during eager loading, but not across reloads.
# Reuse it during eager loading or a digest computation, but not across reloads.
Fiber[:__vc_path_parser] || ActionView::Resolver::PathParser.new
end

Expand Down
29 changes: 29 additions & 0 deletions test/sandbox/test/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,35 @@ def test_source_file_contents_are_discarded_after_an_error
end
end

def test_nested_file_caches_preserve_the_outer_cache
Dir.mktmpdir do |directory|
path = File.join(directory, "temporary_component.html.erb")
File.write(path, "original")

ViewComponent::CompileCache.with_sidecar_files_cache do
parser = ViewComponent::CompileCache.path_parser
assert_equal "original", ViewComponent::CompileCache.read_file(path)

ViewComponent::CompileCache.with_sidecar_files_cache do
assert_same parser, ViewComponent::CompileCache.path_parser
File.write(path, "edited")
assert_equal "original", ViewComponent::CompileCache.read_file(path)
end

assert_same parser, ViewComponent::CompileCache.path_parser
assert_equal "original", ViewComponent::CompileCache.read_file(path)

assert_raises(RuntimeError) do
ViewComponent::CompileCache.with_sidecar_files_cache { raise "nested failure" }
end
assert_same parser, ViewComponent::CompileCache.path_parser
assert_equal "original", ViewComponent::CompileCache.read_file(path)
end

assert_equal "edited", ViewComponent::CompileCache.read_file(path)
end
end

def test_template_path_parser_is_shared_only_while_eager_loading
parser = nil
ViewComponent::CompileCache.with_sidecar_files_cache do
Expand Down
56 changes: 56 additions & 0 deletions test/sandbox/test/cacheable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,62 @@ def test_digest_builds_each_component_once
clear_digest_cache
end

def test_cold_digest_shares_sidecar_directory_listings
clear_digest_cache
directory = File.dirname(CacheableDiamondComponent.identifier)
scans = 0
glob = Dir.method(:glob)
scan = lambda do |pattern, *args|
scans += 1 if pattern == "#{directory}/*"
glob.call(pattern, *args)
end

Dir.stub(:glob, scan) { CacheableDiamondComponent.cache_digest }

assert_equal 1, scans
ensure
clear_digest_cache
end

def test_file_caching_preserves_the_digest
clear_digest_cache
uncached = ->(&block) { block.call }
without_memoization = ->(_key, &block) { block.call }
expected = ViewComponent::CompileCache.stub(:with_sidecar_files_cache, uncached) do
ViewComponent::CacheDigest.stub(:memoize, without_memoization) do
CacheableDiamondComponent.cache_digest
end
end
clear_digest_cache

assert_equal expected, CacheableDiamondComponent.cache_digest
ensure
clear_digest_cache
end

def test_shared_source_files_are_hashed_once_per_computation
resolver = ViewComponent::CacheDigest::Resolver.instance
path = CacheableComponent.identifier
reads = 0
binread = File.method(:binread)
read = lambda do |filename, *args|
reads += 1 if filename == path
binread.call(filename, *args)
end
expected = resolver.source_for(CacheableComponent)

File.stub(:binread, read) do
ViewComponent::CacheDigest.memoizing do
assert_equal expected, resolver.source_for(CacheableComponent)
resolver.source_for(CacheableSubclassComponent)
assert_equal 1, reads
end

assert_equal expected, resolver.source_for(CacheableComponent)
assert_equal 2, reads
end
end

# The children of a dependency whose digest is cached are left out of the
# tree, so an expiry while the tree is built has to start the digest over.
def test_digest_recomputes_when_digests_expire_while_the_tree_is_built
Expand Down
Loading