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/configuration.rb

@@ -1,12 +1,17 @@
-
 module ReactOnRails
   def self.configure
     yield(configuration)
   end
 
+  DEFAULT_GENERATED_ASSETS_DIRS = [
+    %w(app assets javascripts generated),
+    %w(app assets javascripts stylesheets generated)
+  ].map { |dirs| File.join(*dirs) }.freeze
+
   def self.configuration
     @configuration ||= Configuration.new(
-      server_bundle_js_file: "app/assets/javascripts/generated/server.js",
+      generated_assets_dirs: DEFAULT_GENERATED_ASSETS_DIRS,
+      server_bundle_js_file: File.join(*%w(app assets javascripts generated server.js)),
       prerender: false,
       replay_console: true,
       logging_on_server: true,
@@ -23,17 +28,18 @@ class Configuration
                   :trace, :development_mode,
                   :logging_on_server, :server_renderer_pool_size,
                   :server_renderer_timeout, :raise_on_prerender_error,
-                  :skip_display_none
+                  :skip_display_none, :generated_assets_dirs
 
     def initialize(server_bundle_js_file: nil, prerender: nil, replay_console: nil,
                    trace: nil, development_mode: nil,
                    logging_on_server: nil, server_renderer_pool_size: nil,
                    server_renderer_timeout: nil, raise_on_prerender_error: nil,
-                   skip_display_none: nil)
+                   skip_display_none: nil, generated_assets_dirs: DEFAULT_GENERATED_ASSETS_DIRS)
       self.server_bundle_js_file = if File.exist?(server_bundle_js_file)
                                      server_bundle_js_file
                                    end
 
+      self.generated_assets_dirs = generated_assets_dirs
       self.prerender = prerender
       self.replay_console = replay_console
       self.logging_on_server = logging_on_server

lib/react_on_rails/ensure_assets_compiled.rb

@@ -1,34 +1,50 @@
 module ReactOnRails
   class EnsureAssetsCompiled
-    COMPILED_DIR_NAMES = %w(javascripts stylesheets fonts images).freeze
+    class << self
+      attr_accessor :has_been_run
+      @has_been_run = false
+    end
 
     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)
+      compiled_dirs = ReactOnRails.configuration.generated_assets_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
-      @assets_have_been_compiled = false
+      @webpack_process_checker = webpack_process_checker
     end
 
     def call
-      should_skip_compiling = assets_have_been_compiled || @webpack_assets_checker.up_to_date?
-      webpack_assets_compiler.compile unless should_skip_compiling
-      @assets_have_been_compiled = true
+      loop do
+        should_skip_compiling = self.class.has_been_run || @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
+
+      self.class.has_been_run = 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 +53,45 @@ 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. If you are frequently running\n"\
+           "tests, you can run webpack in watch mode to speed up this process.\n"\
+           "See the official documentation:\n"\
+           "https://github.com/shakacode/react_on_rails/blob/master/docs/additional_reading/rspec_configuration.md\n\n"
+    end
+  end
+
+  class WebpackProcessChecker
+    def running?
+      client_running = check_running_for_type("client")
+      return client_running unless Utils.server_rendering_is_enabled?
+
+      server_running = check_running_for_type("server")
+      fail_if_only_running_for_one_type(client_running, server_running)
+
+      client_running && server_running
+    end
+
+    private
+
+    # We only want to do this if server rendering is enabled.
+    def fail_if_only_running_for_one_type(client_running, server_running)
+      return unless client_running ^ server_running
+      fail "\n\nError: detected webpack is not running for both types of assets:\n"\
+           "***Webpack Client Process Running?: #{client_running}\n"\
+           "***Webpack Server Process Running?: #{server_running}"
+    end
+
+    def check_running_for_type(type)
+      type = type.to_sym
+
+      response = `pgrep -fl 'bin/webpack\s(\\-w|\\-\\-watch)\s\\-\\-config\s.*#{type}.*\\.js'`
+      is_running = Utils.last_process_completed_successfully?
+
+      puts "#{type} webpack process is running: #{response.ai}" if is_running
 
-      puts "Webpack #{type}-rendering assets built.\n\n"
+      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

@@ -1,4 +1,5 @@
 require "rake"
+require "fileutils"
 
 module ReactOnRails
   class WebpackAssetsStatusChecker
@@ -10,18 +11,23 @@ def initialize(args = {})
     end
 
     def up_to_date?
+      create_compiled_dirs_if_missing
       return false unless assets_exist?
       all_compiled_assets.all? { |asset| FileUtils.uptodate?(asset, client_files) }
     end
 
     private
 
+    def create_compiled_dirs_if_missing
+      compiled_dirs.each { |dir| FileUtils.mkdir_p(dir) }
+    end
+
     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,41 +1,62 @@
 require_relative "simplecov_helper"
 require_relative "spec_helper"
 
-class WebpackAssetsCompilerDouble
-  attr_reader :times_ran
-
-  def initialize
-    @times_ran = 0
-  end
-
-  def compile
-    @times_ran += 1
-  end
-end
-
 module ReactOnRails
   describe EnsureAssetsCompiled do
-    let(:compiler) { WebpackAssetsCompilerDouble.new }
-    let(:ensurer) { EnsureAssetsCompiled.new(checker, compiler) }
+    let(:compiler) { double_assets_compiler }
+    let(:ensurer) { EnsureAssetsCompiled.new(assets_checker, compiler, process_checker) }
+    after { ensurer.class.has_been_run = false }
 
     context "when assets are not up to date" do
-      let(:checker) { double_webpack_assets_checker(up_to_date: false) }
+      let(:assets_checker) { double_assets_checker(up_to_date: false) }
+
+      context "and webpack process is running" do
+        let(:process_checker) { double_process_checker(running: true) }
+
+        it "sleeps until assets are up to date" do
+          expect(compiler).not_to receive(:compile)
+
+          thread = Thread.new { ensurer.call }
 
-      it "compiles the webpack bundles" do
-        expect { ensurer.call }.to change { compiler.times_ran }.from(0).to(1)
+          sleep 1
+          allow(assets_checker).to receive(:up_to_date?).and_return(true)
+
+          thread.join
+
+          expect(ensurer.class.has_been_run).to eq(true)
+        end
+      end
+
+      context "and webpack process is NOT running" do
+        let(:process_checker) { double_process_checker(running: false) }
+
+        it "compiles the webpack assets" do
+          expect(compiler).to receive(:compile).once
+          ensurer.call
+        end
       end
     end
 
     context "when assets are up to date" do
-      let(:checker) { double_webpack_assets_checker(up_to_date: true) }
+      let(:assets_checker) { double_assets_checker(up_to_date: true) }
+      let(:process_checker) { double_process_checker(running: false) }
 
-      it "does not compile the webpack bundles if they exist and are up to date" do
-        expect { ensurer.call }.not_to change { compiler.times_ran }
+      it "does nothing" do
+        expect(compiler).not_to receive(:compile)
+        ensurer.call
       end
     end
 
-    def double_webpack_assets_checker(args = {})
+    def double_process_checker(args = {})
+      instance_double(WebpackProcessChecker, running?: args.fetch(:running))
+    end
+
+    def double_assets_checker(args = {})
       instance_double(WebpackAssetsStatusChecker, up_to_date?: args.fetch(:up_to_date))
     end
+
+    def double_assets_compiler
+      instance_double(WebpackAssetsCompiler, :compile)
+    end
   end
 end