From 9a07b6446ce0707efc23431adba58971d0c471ab Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Wed, 4 Dec 2024 21:30:34 -0600 Subject: [PATCH] add logging methods instead of changing raise methods --- lib/react_on_rails/engine.rb | 4 +- lib/react_on_rails/version_checker.rb | 37 +++++++-- spec/react_on_rails/version_checker_spec.rb | 83 ++++++++++++++++++--- 3 files changed, 104 insertions(+), 20 deletions(-) diff --git a/lib/react_on_rails/engine.rb b/lib/react_on_rails/engine.rb index f6b684f09..ccde8e0da 100644 --- a/lib/react_on_rails/engine.rb +++ b/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 diff --git a/lib/react_on_rails/version_checker.rb b/lib/react_on_rails/version_checker.rb index 264acafe6..c5b7a2783 100644 --- a/lib/react_on_rails/version_checker.rb +++ b/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) diff --git a/spec/react_on_rails/version_checker_spec.rb b/spec/react_on_rails/version_checker_spec.rb index a46b13f80..11b42a797 100644 --- a/spec/react_on_rails/version_checker_spec.rb +++ b/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