diff --git a/lib/propshaft/assembly.rb b/lib/propshaft/assembly.rb index 3fbb11c..3eb5537 100644 --- a/lib/propshaft/assembly.rb +++ b/lib/propshaft/assembly.rb @@ -6,6 +6,7 @@ require "propshaft/compilers" require "propshaft/compiler/css_asset_urls" require "propshaft/compiler/source_mapping_urls" +require "propshaft/dependency_tree" class Propshaft::Assembly attr_reader :config @@ -15,7 +16,7 @@ def initialize(config) end def load_path - @load_path ||= Propshaft::LoadPath.new(config.paths, version: config.version) + @load_path ||= Propshaft::LoadPath.new(config.paths, version: config.version, dependency_tree: dependency_tree) end def resolver @@ -26,6 +27,10 @@ def resolver end end + def dependency_tree + Propshaft::DependencyTree.new(compilers: compilers) + end + def server Propshaft::Server.new(self) end diff --git a/lib/propshaft/asset.rb b/lib/propshaft/asset.rb index 759523f..26e053a 100644 --- a/lib/propshaft/asset.rb +++ b/lib/propshaft/asset.rb @@ -3,9 +3,12 @@ class Propshaft::Asset attr_reader :path, :logical_path, :version + attr_accessor :dependencies, :dependency_fingerprint def initialize(path, logical_path:, version: nil) @path, @logical_path, @version = path, Pathname.new(logical_path), version + @dependencies = nil + @dependency_fingerprint = nil end def content @@ -16,12 +19,27 @@ def content_type Mime::Type.lookup_by_extension(logical_path.extname.from(1)) end + def may_depend? + content_type&.symbol == :css + end + def length content.size end + # A dependency aware digest can be calculated for any asset that has no dependencies or + # that has had its dependency fingerprint set by DependencyTree. + # If it's too soon, we can still calculate the digest based on file contents, + # but there won't be any dependency cache busting. + def digest_too_soon? + may_depend? && dependencies&.any? && !dependency_fingerprint + end + def digest - @digest ||= Digest::SHA1.hexdigest("#{content}#{version}").first(8) + @digest ||= begin + Propshaft.logger.warn("digest dependencies not ready for #{logical_path}") if digest_too_soon? + Digest::SHA1.hexdigest("#{content}#{version}#{dependency_fingerprint}").first(8) + end end def digested_path diff --git a/lib/propshaft/compiler.rb b/lib/propshaft/compiler.rb index b60dd65..7471a66 100644 --- a/lib/propshaft/compiler.rb +++ b/lib/propshaft/compiler.rb @@ -13,6 +13,10 @@ def compile(logical_path, input) raise NotImplementedError end + def find_dependencies(logical_path, input) + Set.new + end + private def url_prefix @url_prefix ||= File.join(assembly.config.relative_url_root.to_s, assembly.config.prefix.to_s).chomp("/") diff --git a/lib/propshaft/compiler/css_asset_urls.rb b/lib/propshaft/compiler/css_asset_urls.rb index b263b7d..8c39295 100644 --- a/lib/propshaft/compiler/css_asset_urls.rb +++ b/lib/propshaft/compiler/css_asset_urls.rb @@ -9,6 +9,14 @@ def compile(logical_path, input) input.gsub(ASSET_URL_PATTERN) { asset_url resolve_path(logical_path.dirname, $1), logical_path, $2, $1 } end + def find_dependencies(logical_path, input) + Set.new.tap do |deps| + input.scan(ASSET_URL_PATTERN).each do |fn, _| + deps << resolve_path(logical_path.dirname, fn) + end + end + end + private def resolve_path(directory, filename) if filename.start_with?("../") diff --git a/lib/propshaft/compilers.rb b/lib/propshaft/compilers.rb index c44d840..79fa6fe 100644 --- a/lib/propshaft/compilers.rb +++ b/lib/propshaft/compilers.rb @@ -30,4 +30,14 @@ def compile(asset) asset.content end end + + def find_dependencies(asset) + Set.new.tap do |deps| + if relevant_registrations = registrations[asset.content_type.to_s] + relevant_registrations.each do |compiler| + deps.merge compiler.new(assembly).find_dependencies(asset.logical_path, asset.content) + end + end + end + end end diff --git a/lib/propshaft/dependency_tree.rb b/lib/propshaft/dependency_tree.rb new file mode 100644 index 0000000..2ca29a6 --- /dev/null +++ b/lib/propshaft/dependency_tree.rb @@ -0,0 +1,61 @@ +class Propshaft::DependencyTree + + def initialize(compilers:) + @compilers = compilers + end + + # a single pass through the dependent assets, calculating all the dependency + # fingerprints we can. + def dependency_fingerprint_pass(dependent_assets) + dependent_assets.each do |asset| + # the fingerprint can be calculated unless a dependency of this asset + # is not ready yet. + next if asset.dependencies.detect{|da| da.digest_too_soon?} + # ok, ready to set the fingerprint, which is the concatenation of the + # digests of each dependent asset. + # the fingerprints need to maintain a stable order, done via the sort. + asset.dependency_fingerprint = asset.dependencies.map(&:digest).sort.join + end + # clear out any ones we are done with. + dependent_assets.delete_if{|asset| asset.dependency_fingerprint.present?} + end + + # After we know the assets that depend on other propshaft assets, we can iterate + # through the list, calculating the dependency fingerprint for any asset with + # children with known digests. Once we run out of dependent assets without + # digests, we are done. But if we notice that we aren't making any progress on + # an iteration, it means there is an asset dependency cycle. In that case we + # bail out and warn. + def set_dependency_fingerprints(dependent_assets) + # There will be N iterations where N is the depth of the longest dependency chain. + loop do + initial_count = dependent_assets.size + dependency_fingerprint_pass(dependent_assets) + break if dependent_assets.empty? # success, all done + if dependent_assets.size == initial_count + # failing to make progress + cyclic_assets = dependent_assets.map{|a| a.logical_path.to_s}.join(', ') + Propshaft.logger.warn "Dependency cycle between #{cyclic_assets}" + break + end + end + end + + def traverse(mapped) + dependent_assets = Set.new + mapped.each_pair do |path, asset| + next unless asset.may_depend? # skip static asset types + # get logical paths of dependencies + deps = @compilers.find_dependencies(asset) + # convert logical paths to asset objects + # asset references that aren't managed by propshaft will not be in the mapping + # and can be ignored since they won't have digests + asset.dependencies = deps.map{|d| mapped[d]}.compact + # If no dependencies, the normal digest will be fine for this asset. + # Otherwise we will need to perform dependency tree traversal to + # create dependency-aware digests. Keep a list of such assets to visit. + dependent_assets << asset if asset.dependencies.any? + end + set_dependency_fingerprints(dependent_assets) + end +end diff --git a/lib/propshaft/load_path.rb b/lib/propshaft/load_path.rb index ab236b2..e6e2c52 100644 --- a/lib/propshaft/load_path.rb +++ b/lib/propshaft/load_path.rb @@ -3,9 +3,10 @@ class Propshaft::LoadPath attr_reader :paths, :version - def initialize(paths = [], version: nil) - @paths = dedup(paths) - @version = version + def initialize(paths = [], version: nil, dependency_tree: nil) + @paths = dedup(paths) + @version = version + @dependency_tree = dependency_tree end def find(asset_name) @@ -51,6 +52,7 @@ def assets_by_path mapped[logical_path.to_s] ||= Propshaft::Asset.new(file, logical_path: logical_path, version: version) end if path.exist? end + @dependency_tree&.traverse(mapped) end end diff --git a/test/fixtures/assets/dependent/child1.css b/test/fixtures/assets/dependent/child1.css new file mode 100644 index 0000000..0ce6e22 --- /dev/null +++ b/test/fixtures/assets/dependent/child1.css @@ -0,0 +1 @@ +@import url('child2.css'); diff --git a/test/fixtures/assets/dependent/child2.css b/test/fixtures/assets/dependent/child2.css new file mode 100644 index 0000000..bd6c258 --- /dev/null +++ b/test/fixtures/assets/dependent/child2.css @@ -0,0 +1 @@ +p { background: blue; } diff --git a/test/fixtures/assets/dependent/cyclic1.css b/test/fixtures/assets/dependent/cyclic1.css new file mode 100644 index 0000000..8e212a0 --- /dev/null +++ b/test/fixtures/assets/dependent/cyclic1.css @@ -0,0 +1 @@ +@import url('cyclic2.css'); diff --git a/test/fixtures/assets/dependent/cyclic2.css b/test/fixtures/assets/dependent/cyclic2.css new file mode 100644 index 0000000..2717edc --- /dev/null +++ b/test/fixtures/assets/dependent/cyclic2.css @@ -0,0 +1 @@ +@import url('cyclic1.css'); diff --git a/test/fixtures/assets/dependent/main.css b/test/fixtures/assets/dependent/main.css new file mode 100644 index 0000000..d4dda30 --- /dev/null +++ b/test/fixtures/assets/dependent/main.css @@ -0,0 +1 @@ +@import url('child1.css'); diff --git a/test/propshaft/dependency_tree_test.rb b/test/propshaft/dependency_tree_test.rb new file mode 100644 index 0000000..7ef4997 --- /dev/null +++ b/test/propshaft/dependency_tree_test.rb @@ -0,0 +1,42 @@ +require "test_helper" +require "minitest/mock" +require "propshaft/asset" +require "propshaft/assembly" +require "propshaft/compilers" + +class Propshaft::DependencyTreeTest < ActiveSupport::TestCase + setup do + @assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config| + config.paths = [ Pathname.new("#{__dir__}/../fixtures/assets/dependent") ] + config.output_path = Pathname.new("#{__dir__}/../fixtures/output") + config.prefix = "/assets" + }) + @assembly.compilers.register "text/css", Propshaft::Compiler::CssAssetUrls + end + + test "cyclic assets do not cause a loop" do + @assembly.load_path.assets + end + + def write_child2(col) + File.open("#{__dir__}/../fixtures/assets/dependent/child2.css", "w") do |file| + file.puts "p { background: #{col}; }" + end + end + + test "modification of a child affects dependent asset digests" do + write_child2('blue') # ensure no leftovers from previous runs + prev_assets = @assembly.load_path.assets + prev_main_digest = prev_assets.detect{|a| a.logical_path.to_s == 'main.css'}.digest + write_child2('red') + @assembly.load_path.cache_sweeper.execute + changed_assets = @assembly.load_path.assets + changed_main_digest = changed_assets.detect{|a| a.logical_path.to_s == 'main.css'}.digest + assert_not_equal(prev_main_digest, changed_main_digest) + write_child2('blue') # restore the original + @assembly.load_path.cache_sweeper.execute + final_assets = @assembly.load_path.assets + final_main_digest = final_assets.detect{|a| a.logical_path.to_s == 'main.css'}.digest + assert_equal(prev_main_digest, final_main_digest) + end +end