Make clean task simpler and fix issues (#2389)

lib/tasks/webpacker/clean.rake

@@ -1,9 +1,13 @@
+$stdout.sync = true
+
 require "webpacker/configuration"
 
 namespace :webpacker do
   desc "Remove old compiled webpacks"
   task :clean, [:keep] => ["webpacker:verify_install", :environment] do |_, args|
-    Webpacker.clean(Integer(args.keep || 2))
+    Webpacker.ensure_log_goes_to_stdout do
+      Webpacker.clean(Integer(args.keep || 2))
+    end
   end
 end
 

lib/tasks/webpacker/compile.rake

@@ -1,13 +1,5 @@
 $stdout.sync = true
 
-def ensure_log_goes_to_stdout
-  old_logger = Webpacker.logger
-  Webpacker.logger = ActiveSupport::Logger.new(STDOUT)
-  yield
-ensure
-  Webpacker.logger = old_logger
-end
-
 def yarn_install_available?
   rails_major = Rails::VERSION::MAJOR
   rails_minor = Rails::VERSION::MINOR
@@ -27,7 +19,7 @@ namespace :webpacker do
   desc "Compile JavaScript packs using webpack for production with digests"
   task compile: ["webpacker:verify_install", :environment] do
     Webpacker.with_node_env(ENV.fetch("NODE_ENV", "production")) do
-      ensure_log_goes_to_stdout do
+      Webpacker.ensure_log_goes_to_stdout do
         if Webpacker.compile
           # Successful compilation!
         else

lib/webpacker.rb

@@ -22,6 +22,14 @@ def with_node_env(env)
     ENV["NODE_ENV"] = original
   end
 
+  def ensure_log_goes_to_stdout
+    old_logger = Webpacker.logger
+    Webpacker.logger = ActiveSupport::Logger.new(STDOUT)
+    yield
+  ensure
+    Webpacker.logger = old_logger
+  end
+
   delegate :logger, :logger=, :env, to: :instance
   delegate :config, :compiler, :manifest, :commands, :dev_server, to: :instance
   delegate :bootstrap, :clean, :clobber, :compile, to: :commands

lib/webpacker/commands.rb

@@ -1,25 +1,19 @@
 class Webpacker::Commands
-  delegate :config, :compiler, :manifest, to: :@webpacker
+  delegate :config, :compiler, :manifest, :logger, to: :@webpacker
 
   def initialize(webpacker)
     @webpacker = webpacker
   end
 
-  def clean(count_to_keep = 2)
-    if config.public_output_path.exist? && config.public_manifest_path.exist?
-      files_in_manifest = process_manifest_hash(manifest.refresh)
-      files_to_be_removed = files_in_manifest.flat_map do |file_in_manifest|
-        file_prefix, file_ext = file_in_manifest.scan(/(.*)[0-9a-f]{20}(.*)/).first
-        versions_of_file = Dir.glob("#{file_prefix}*#{file_ext}").grep(/#{file_prefix}[0-9a-f]{20}#{file_ext}/)
-        versions_of_file.map do |version_of_file|
-          next if version_of_file == file_in_manifest
-
-          [version_of_file, File.mtime(version_of_file).utc.to_i]
-        end.compact.sort_by(&:last).reverse.drop(count_to_keep).map(&:first)
+  def clean(count = 2)
+    if config.public_output_path.exist? && config.public_manifest_path.exist? && versions.count > count
+      versions.drop(count).flat_map(&:last).each do |file|
+        File.delete(file) if File.exist?(file)
+        logger.info "Removed #{file}"
       end
-
-      files_to_be_removed.each { |f| File.delete f }
     end
+
+    true
   end
 
   def clobber
@@ -38,11 +32,19 @@ def compile
   end
 
   private
-    def process_manifest_hash(manifest_hash)
-      manifest_hash.values.map do |value|
-        next process_manifest_hash(value) if value.is_a?(Hash)
+    def versions
+      all_files       = Dir.glob("#{config.public_output_path}/**/*")
+      manifest_config = Dir.glob("#{config.public_manifest_path}*")
+
+      packs = all_files - manifest_config - current_version
+      packs.group_by { |file| File.mtime(file).utc.to_i }.sort.reverse
+    end
+
+    def current_version
+      manifest.refresh.values.map do |value|
+        next if value.is_a?(Hash)
 
         File.join(config.root_path, "public", value)
-      end.flatten
+      end.compact
     end
 end