Improve EnsureAssetsCompiled logic to wait for webpack process to finish

Because there may be situations where a user is running a webpack
process in watch mode in the background that will recompile the
assets, it is possible that if the build time is extremely long,
the user could start tests before the build completes. In this case,
EnsureAssetsCompiled would start compiling the assets again even
though there is already a webpack process doing that.

This commit adds behavior to EnsureAssetsCompiled to check whether
or not the webpack process is running in the background, and will
defer the responsibility of compilation to that process, re-checking
the compiled assets every second until they are up to date.

Author: robwise

lib/react_on_rails/ensure_assets_compiled.rb

@@ -5,30 +5,44 @@ class EnsureAssetsCompiled
     def self.build
       client_dir = Rails.root.join("client")
       compiled_dirs = COMPILED_DIR_NAMES.map { |dir| Rails.root.join("app", "assets", dir, "generated") }
-      checker = WebpackAssetsStatusChecker.new(client_dir: client_dir, compiled_dirs: compiled_dirs)
+
+      assets_checker = WebpackAssetsStatusChecker.new(client_dir: client_dir, compiled_dirs: compiled_dirs)
+      process_checker = WebpackProcessChecker.new
       compiler = WebpackAssetsCompiler.new
-      new(checker, compiler)
+
+      new(assets_checker, compiler, process_checker)
     end
 
-    attr_reader :webpack_assets_checker, :webpack_assets_compiler, :assets_have_been_compiled
+    attr_reader :webpack_assets_checker, :webpack_assets_compiler, :webpack_process_checker, :assets_have_been_compiled
 
-    def initialize(webpack_assets_checker, webpack_assets_compiler)
+    def initialize(webpack_assets_checker, webpack_assets_compiler, webpack_process_checker)
       @webpack_assets_compiler = webpack_assets_compiler
       @webpack_assets_checker = webpack_assets_checker
+      @webpack_process_checker = webpack_process_checker
       @assets_have_been_compiled = false
     end
 
     def call
-      should_skip_compiling = assets_have_been_compiled || @webpack_assets_checker.up_to_date?
-      webpack_assets_compiler.compile unless should_skip_compiling
+      loop do
+        should_skip_compiling = assets_have_been_compiled || @webpack_assets_checker.up_to_date?
+        break if should_skip_compiling
+
+        if webpack_process_checker.running?
+          sleep 1
+        else
+          webpack_assets_compiler.compile
+          break
+        end
+      end
+
       @assets_have_been_compiled = true
     end
   end
 
   class WebpackAssetsCompiler
     def compile
       compile_type(:client)
-      compile_type(:server) if ReactOnRails.configuration.server_bundle_js_file.present?
+      compile_type(:server) if Utils.server_rendering_is_enabled?
     end
 
     private
@@ -37,11 +51,30 @@ def compile_type(type)
       puts "\n\nBuilding Webpack #{type}-rendering assets..."
       build_output = `cd client && npm run build:#{type}`
 
-      if build_output =~ /error/i
-        fail "Error in building assets!\n#{build_output}"
-      end
+      fail "Error in building assets!\n#{build_output}" unless Utils.last_process_completed_successfully?
 
       puts "Webpack #{type}-rendering assets built.\n\n"
     end
   end
+
+  class WebpackProcessChecker
+    def running?
+      is_running = check_running_for_type("client")
+      is_running &&= check_running_for_type("server") if Utils.server_rendering_is_enabled?
+      is_running
+    end
+
+    private
+
+    def check_running_for_type(type)
+      type = type.to_sym
+
+      response = `pgrep -fl 'bin/webpack\s\\-w(atch)?\s\\-\\-config\s.*#{type}.*\\.js'`
+      is_running = Utils.last_process_completed_successfully?
+
+      puts "#{type} webpack process is running: #{response.ai}" if is_running
+
+      is_running
+    end
+  end
 end

lib/react_on_rails/utils.rb

@@ -1,7 +1,17 @@
+require "english"
+
 module ReactOnRails
   module Utils
     def self.object_to_boolean(value)
       [true, "true", "yes", 1, "1", "t"].include?(value.class == String ? value.downcase : value)
     end
+
+    def self.server_rendering_is_enabled?
+      ReactOnRails.configuration.server_bundle_js_file.present?
+    end
+
+    def self.last_process_completed_successfully?
+      $CHILD_STATUS.exitstatus == 0
+    end
   end
 end

lib/react_on_rails/webpack_assets_status_checker.rb

@@ -17,11 +17,11 @@ def up_to_date?
     private
 
     def all_compiled_assets
-      @all_compiled_assets ||= make_file_list(make_globs(compiled_dirs)).to_ary
+      @all_compiled_assets = make_file_list(make_globs(compiled_dirs)).to_ary
     end
 
     def client_files
-      @client_files ||= make_file_list(make_globs(client_dir)).to_ary
+      @client_files = make_file_list(make_globs(client_dir)).to_ary
     end
 
     def make_globs(dirs)

spec/dummy/Gemfile.lock

@@ -4,6 +4,7 @@ PATH
     react_on_rails (2.3.0)
       connection_pool
       execjs (~> 2.5)
+      foreman
       rails (>= 3.2)
       rainbow (~> 2.1)
 
@@ -105,6 +106,8 @@ GEM
     erubis (2.7.0)
     execjs (2.6.0)
     ffi (1.9.10)
+    foreman (0.78.0)
+      thor (~> 0.19.1)
     generator_spec (0.9.3)
       activesupport (>= 3.0.0)
       railties (>= 3.0.0)

spec/dummy/Procfile.spec

@@ -0,0 +1,2 @@
+client-static-assets: sh -c 'rm app/assets/javascripts/generated/* || true && cd client && npm run build:dev:client'
+server-static-assets: sh -c 'cd client && npm run build:dev:server'

spec/react_on_rails/ensure_assets_compiled_spec.rb

@@ -1,7 +1,7 @@
 require_relative "simplecov_helper"
 require_relative "spec_helper"
 
-class WebpackAssetsCompilerDouble
+class CompilerDouble
   attr_reader :times_ran
 
   def initialize
@@ -13,29 +13,70 @@ def compile
   end
 end
 
+class ProcessCheckerDouble
+  attr_accessor :is_running
+
+  def initialize(p_is_running)
+    @is_running = p_is_running
+  end
+
+  def running?
+    is_running
+  end
+end
+
+class StatusCheckerDouble
+  attr_accessor :up_to_date
+
+  def initialize(initial)
+    @up_to_date = initial
+  end
+
+  def up_to_date?
+    up_to_date
+  end
+end
+
 module ReactOnRails
   describe EnsureAssetsCompiled do
-    let(:compiler) { WebpackAssetsCompilerDouble.new }
-    let(:ensurer) { EnsureAssetsCompiled.new(checker, compiler) }
+    let(:compiler) { CompilerDouble.new }
+    let(:ensurer) { EnsureAssetsCompiled.new(assets_checker, compiler, process_checker) }
 
     context "when assets are not up to date" do
-      let(:checker) { double_webpack_assets_checker(up_to_date: false) }
+      let(:assets_checker) { StatusCheckerDouble.new(false) }
+
+      context "and webpack process is running" do
+        let(:process_checker) { ProcessCheckerDouble.new(true) }
+
+        it "sleeps until assets are up to date" do
+          thread = Thread.new { ensurer.call }
+
+          sleep 1
+          assets_checker.up_to_date = true
+
+          thread.join
 
-      it "compiles the webpack bundles" do
-        expect { ensurer.call }.to change { compiler.times_ran }.from(0).to(1)
+          expect(compiler.times_ran).to eq(0)
+          expect(ensurer.assets_have_been_compiled).to eq(true)
+        end
+      end
+
+      context "and webpack process is NOT running" do
+        let(:process_checker) { ProcessCheckerDouble.new(false) }
+
+        it "compiles the webpack assets" do
+          expect { ensurer.call }.to change { compiler.times_ran }.from(0).to(1)
+        end
       end
     end
 
     context "when assets are up to date" do
-      let(:checker) { double_webpack_assets_checker(up_to_date: true) }
+      let(:assets_checker) { StatusCheckerDouble.new(true) }
+      let(:process_checker) { ProcessCheckerDouble.new(false) }
 
-      it "does not compile the webpack bundles if they exist and are up to date" do
+      it "does nothing" do
         expect { ensurer.call }.not_to change { compiler.times_ran }
       end
     end
-
-    def double_webpack_assets_checker(args = {})
-      instance_double(WebpackAssetsStatusChecker, up_to_date?: args.fetch(:up_to_date))
-    end
   end
 end