add logging methods instead of changing raise methods

Author: Judahmeek

lib/react_on_rails/engine.rb

@@ -5,8 +5,8 @@
 module ReactOnRails
   class Engine < ::Rails::Engine
     config.to_prepare do
-      if VersionChecker.instance("package.json").raise_if_gem_and_node_package_versions_differ &&
-         VersionChecker.instance("client/package.json").raise_if_gem_and_node_package_versions_differ
+      if VersionChecker.instance("package.json").log_if_gem_and_node_package_versions_differ &&
+         VersionChecker.instance("client/package.json").log_if_gem_and_node_package_versions_differ
         Rails.logger.warn("No 'react-on-rails' entry found in 'dependencies' in package.json or client/package.json.")
       end
       ReactOnRails::ServerRenderingPool.reset_pool

lib/react_on_rails/version_checker.rb

@@ -29,18 +29,32 @@ def raise_if_gem_and_node_package_versions_differ
 
       raise_node_semver_version_warning if node_package_version.semver_wildcard?
 
-      node_major_minor_patch = node_package_version.major_minor_patch
-      gem_major_minor_patch = gem_major_minor_patch_version
-      versions_match = node_major_minor_patch[0] == gem_major_minor_patch[0] &&
-                       node_major_minor_patch[1] == gem_major_minor_patch[1] &&
-                       node_major_minor_patch[2] == gem_major_minor_patch[2]
+      versions_match = compare_versions(node_package_version.major_minor_patch, gem_major_minor_patch_version)
 
       raise_differing_versions_warning unless versions_match
       false
     end
 
+    def log_if_gem_and_node_package_versions_differ
+      return true unless node_package_version.raw
+      return if node_package_version.relative_path?
+
+      log_node_semver_version_warning if node_package_version.semver_wildcard?
+
+      versions_match = compare_versions(node_package_version.major_minor_patch, gem_major_minor_patch_version)
+
+      log_differing_versions_warning unless versions_match
+      false
+    end
+
     private
 
+    def compare_versions(node_major_minor_patch, gem_major_minor_patch)
+      node_major_minor_patch[0] == gem_major_minor_patch[0] &&
+        node_major_minor_patch[1] == gem_major_minor_patch[1] &&
+        node_major_minor_patch[2] == gem_major_minor_patch[2]
+    end
+
     def common_error_msg
       <<-MSG.strip_heredoc
          Detected: #{node_package_version.raw}
@@ -55,10 +69,21 @@ def common_error_msg
 
     def raise_differing_versions_warning
       msg = "**WARNING** ReactOnRails: ReactOnRails gem and node package versions do not match\n#{common_error_msg}"
-      Rails.logger.warn(msg)
+      raise ReactOnRails::Error, msg
     end
 
     def raise_node_semver_version_warning
+      msg = "**WARNING** ReactOnRails: Your node package version for react-on-rails contains a " \
+            "^ or ~\n#{common_error_msg}"
+      raise ReactOnRails::Error, msg
+    end
+
+    def log_differing_versions_warning
+      msg = "**WARNING** ReactOnRails: ReactOnRails gem and node package versions do not match\n#{common_error_msg}"
+      Rails.logger.warn(msg)
+    end
+
+    def log_node_semver_version_warning
       msg = "**WARNING** ReactOnRails: Your node package version for react-on-rails contains a " \
             "^ or ~\n#{common_error_msg}"
       Rails.logger.warn(msg)

spec/react_on_rails/version_checker_spec.rb

@@ -11,7 +11,7 @@ def error(message)
   end
 end
 
-module ReactOnRails
+module ReactOnRails # rubocop:disable Metrics/ModuleLength
   describe VersionChecker do
     describe "#warn_if_gem_and_node_package_versions_differ" do
       let(:logger) { FakeLogger.new }
@@ -24,7 +24,13 @@ module ReactOnRails
         before { stub_gem_version("2.2.5.beta.2") }
 
         it "does not raise" do
-          expect { check_version(node_package_version) }.not_to raise_error
+          expect { check_version_and_raise(node_package_version) }.not_to raise_error
+        end
+
+        it "does not log" do
+          allow(Rails.logger).to receive(:warn)
+          check_version_and_log(node_package_version)
+          expect(Rails.logger).not_to have_received(:warn)
         end
       end
 
@@ -38,10 +44,15 @@ module ReactOnRails
         it "logs" do
           allow(Rails.logger).to receive(:warn)
           message = /ReactOnRails: Your node package version for react-on-rails contains a \^ or ~/
-          # expect { check_version(node_package_version) }.to raise_error(message)
-          check_version(node_package_version)
+          check_version_and_log(node_package_version)
           expect(Rails.logger).to have_received(:warn).with(message)
         end
+
+        it "raises" do
+          allow(Rails.logger).to receive(:warn)
+          message = /ReactOnRails: Your node package version for react-on-rails contains a \^ or ~/
+          expect { check_version_and_raise(node_package_version) }.to raise_error(message)
+        end
       end
 
       context "when gem and node package major versions differ" do
@@ -54,10 +65,15 @@ module ReactOnRails
         it "logs" do
           allow(Rails.logger).to receive(:warn)
           message = /ReactOnRails: ReactOnRails gem and node package versions do not match/
-          # expect { check_version(node_package_version) }.to raise_error(message)
-          check_version(node_package_version)
+          check_version_and_log(node_package_version)
           expect(Rails.logger).to have_received(:warn).with(message)
         end
+
+        it "raises" do
+          allow(Rails.logger).to receive(:warn)
+          message = /ReactOnRails: ReactOnRails gem and node package versions do not match/
+          expect { check_version_and_raise(node_package_version) }.to raise_error(message)
+        end
       end
 
       context "when gem and node package major versions match and minor differs" do
@@ -70,10 +86,15 @@ module ReactOnRails
         it "logs" do
           allow(Rails.logger).to receive(:warn)
           message = /ReactOnRails: ReactOnRails gem and node package versions do not match/
-          # expect { check_version(node_package_version) }.to raise_error(message)
-          check_version(node_package_version)
+          check_version_and_log(node_package_version)
           expect(Rails.logger).to have_received(:warn).with(message)
         end
+
+        it "raises" do
+          allow(Rails.logger).to receive(:warn)
+          message = /ReactOnRails: ReactOnRails gem and node package versions do not match/
+          expect { check_version_and_raise(node_package_version) }.to raise_error(message)
+        end
       end
 
       context "when gem and node package major, minor versions match and patch differs" do
@@ -86,10 +107,15 @@ module ReactOnRails
         it "logs" do
           allow(Rails.logger).to receive(:warn)
           message = /ReactOnRails: ReactOnRails gem and node package versions do not match/
-          # expect { check_version(node_package_version) }.to raise_error(message)
-          check_version(node_package_version)
+          check_version_and_log(node_package_version)
           expect(Rails.logger).to have_received(:warn).with(message)
         end
+
+        it "raises" do
+          allow(Rails.logger).to receive(:warn)
+          message = /ReactOnRails: ReactOnRails gem and node package versions do not match/
+          expect { check_version_and_raise(node_package_version) }.to raise_error(message)
+        end
       end
 
       context "when package json uses a relative path with dots" do
@@ -100,7 +126,27 @@ module ReactOnRails
         before { stub_gem_version("2.0.0.beta.1") }
 
         it "does not raise" do
-          expect { check_version(node_package_version) }.not_to raise_error
+          expect { check_version_and_raise(node_package_version) }.not_to raise_error
+        end
+
+        it "does not log" do
+          allow(Rails.logger).to receive(:warn)
+          check_version_and_log(node_package_version)
+          expect(Rails.logger).not_to have_received(:warn)
+        end
+      end
+
+      context "when package json doesn't exist" do
+        let(:node_package_version) do
+          double_package_version(raw: nil)
+        end
+
+        it "raise method returns true" do
+          expect(check_version_and_raise(node_package_version)).to be(true)
+        end
+
+        it "log method returns true" do
+          expect(check_version_and_log(node_package_version)).to be(true)
         end
       end
     end
@@ -114,11 +160,16 @@ def double_package_version(raw: nil, semver_wildcard: false,
                       relative_path?: relative_path)
     end
 
-    def check_version(node_package_version)
+    def check_version_and_raise(node_package_version)
       version_checker = VersionChecker.new(node_package_version)
       version_checker.raise_if_gem_and_node_package_versions_differ
     end
 
+    def check_version_and_log(node_package_version)
+      version_checker = VersionChecker.new(node_package_version)
+      version_checker.log_if_gem_and_node_package_versions_differ
+    end
+
     describe VersionChecker::NodePackageVersion do
       subject(:node_package_version) { described_class.new(package_json) }
 
@@ -221,6 +272,14 @@ def check_version(node_package_version)
           specify { expect(node_package_version.major_minor_patch).to be_nil }
         end
       end
+
+      context "with non-existing package.json" do
+        let(:package_json) { File.expand_path("fixtures/nonexistent_package.json", __dir__) }
+
+        describe "#raw" do
+          specify { expect(node_package_version.raw).to be_nil }
+        end
+      end
     end
   end
 end