diff --git a/CHANGELOG.md b/CHANGELOG.md index 93dc9e73b..95002c46d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,12 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac ### [Unreleased] Changes since the last non-beta release. -#### Added(https://github.com/AbanoubGhadban). +#### Fixed + +- Changed the ReactOnRails' version checker to use `ReactOnRails.configuration.node_modules_location` to determine the location of the package.json that the `react-on-rails` dependency is expected to be set by. +- Also, all errors that would be raised by the version checking have been converted to `Rails.Logger` warnings to avoid any breaking changes. [PR 1657](https://github.com/shakacode/react_on_rails/pull/1657) by [judahmeek](https://github.com/judahmeek). + +#### Added - Added streaming server rendering support: - [PR #1633](https://github.com/shakacode/react_on_rails/pull/1633) by [AbanoubGhadban](https://github.com/AbanoubGhadban). - New `stream_react_component` helper for adding streamed components to views diff --git a/lib/react_on_rails/engine.rb b/lib/react_on_rails/engine.rb index 17d9b3c3e..66c2d07c7 100644 --- a/lib/react_on_rails/engine.rb +++ b/lib/react_on_rails/engine.rb @@ -5,9 +5,7 @@ module ReactOnRails class Engine < ::Rails::Engine config.to_prepare do - if File.exist?(VersionChecker::NodePackageVersion.package_json_path) - VersionChecker.build.raise_if_gem_and_node_package_versions_differ - end + VersionChecker.build.log_if_gem_and_node_package_versions_differ ReactOnRails::ServerRenderingPool.reset_pool end end diff --git a/lib/react_on_rails/version_checker.rb b/lib/react_on_rails/version_checker.rb index 62d450ccd..04a8e6a43 100644 --- a/lib/react_on_rails/version_checker.rb +++ b/lib/react_on_rails/version_checker.rb @@ -19,8 +19,9 @@ def initialize(node_package_version) # For compatibility, the gem and the node package versions should always match, # unless the user really knows what they're doing. So we will give a # warning if they do not. - def raise_if_gem_and_node_package_versions_differ - return if node_package_version.relative_path? + def log_if_gem_and_node_package_versions_differ + return if node_package_version.raw.nil? || node_package_version.relative_path? + return log_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 @@ -28,9 +29,7 @@ def raise_if_gem_and_node_package_versions_differ node_major_minor_patch[1] == gem_major_minor_patch[1] && node_major_minor_patch[2] == gem_major_minor_patch[2] - raise_differing_versions_warning unless versions_match - - raise_node_semver_version_warning if node_package_version.semver_wildcard? + log_differing_versions_warning unless versions_match end private @@ -46,15 +45,15 @@ def common_error_msg MSG end - def raise_differing_versions_warning - msg = "**ERROR** ReactOnRails: ReactOnRails gem and node package versions do not match\n#{common_error_msg}" - raise ReactOnRails::Error, msg + 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 raise_node_semver_version_warning - msg = "**ERROR** ReactOnRails: Your node package version for react-on-rails contains a " \ + def log_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 + Rails.logger.warn(msg) end def gem_version @@ -74,7 +73,7 @@ def self.build end def self.package_json_path - Rails.root.join("client", "package.json") + Rails.root.join(ReactOnRails.configuration.node_modules_location, "package.json") end def initialize(package_json) @@ -82,13 +81,17 @@ def initialize(package_json) end def raw - parsed_package_contents = JSON.parse(package_json_contents) - if parsed_package_contents.key?("dependencies") && - parsed_package_contents["dependencies"].key?("react-on-rails") - parsed_package_contents["dependencies"]["react-on-rails"] - else - raise ReactOnRails::Error, "No 'react-on-rails' entry in package.json dependencies" + if File.exist?(package_json) + parsed_package_contents = JSON.parse(package_json_contents) + if parsed_package_contents.key?("dependencies") && + parsed_package_contents["dependencies"].key?("react-on-rails") + return parsed_package_contents["dependencies"]["react-on-rails"] + end end + msg = "No 'react-on-rails' entry in the dependencies of #{NodePackageVersion.package_json_path}, " \ + "which is the expected location according to ReactOnRails.configuration.node_modules_location" + Rails.logger.warn(msg) + nil end def semver_wildcard? @@ -96,7 +99,7 @@ def semver_wildcard? end def relative_path? - raw.match(%r{(\.\.|\Afile:///)}).present? + raw.match(/(\.\.|\Afile:)/).present? end def major_minor_patch diff --git a/spec/react_on_rails/fixtures/yalc_package.json b/spec/react_on_rails/fixtures/yalc_package.json new file mode 100644 index 000000000..1cf6e7b6a --- /dev/null +++ b/spec/react_on_rails/fixtures/yalc_package.json @@ -0,0 +1,10 @@ +{ + "dependencies": { + "babel": "^6.3.26", + "react-on-rails": "file:.yalc/react-on-rails", + "webpack": "^1.12.8" + }, + "devDependencies": { + "babel-eslint": "^5.0.0-beta6" + } +} diff --git a/spec/react_on_rails/version_checker_spec.rb b/spec/react_on_rails/version_checker_spec.rb index 7a35b55dd..8c4369570 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 } @@ -23,8 +23,10 @@ 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 + 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 @@ -35,9 +37,11 @@ module ReactOnRails before { stub_gem_version("2.2.5") } - it "does raise" do - error = /ReactOnRails: Your node package version for react-on-rails contains a \^ or ~/ - expect { check_version(node_package_version) }.to raise_error(error) + it "logs" do + allow(Rails.logger).to receive(:warn) + message = /ReactOnRails: Your node package version for react-on-rails contains a \^ or ~/ + check_version_and_log(node_package_version) + expect(Rails.logger).to have_received(:warn).with(message) end end @@ -48,9 +52,11 @@ module ReactOnRails before { stub_gem_version("12.0.0.beta.1") } - it "raises" do - error = /ReactOnRails: ReactOnRails gem and node package versions do not match/ - expect { check_version(node_package_version) }.to raise_error(error) + it "logs" do + allow(Rails.logger).to receive(:warn) + message = /ReactOnRails: ReactOnRails gem and node package versions do not match/ + check_version_and_log(node_package_version) + expect(Rails.logger).to have_received(:warn).with(message) end end @@ -61,9 +67,11 @@ module ReactOnRails before { stub_gem_version("13.1.0") } - it "raises" do - error = /ReactOnRails: ReactOnRails gem and node package versions do not match/ - expect { check_version(node_package_version) }.to raise_error(error) + it "logs" do + allow(Rails.logger).to receive(:warn) + message = /ReactOnRails: ReactOnRails gem and node package versions do not match/ + check_version_and_log(node_package_version) + expect(Rails.logger).to have_received(:warn).with(message) end end @@ -74,9 +82,11 @@ module ReactOnRails before { stub_gem_version("13.0.0") } - it "raises" do - error = /ReactOnRails: ReactOnRails gem and node package versions do not match/ - expect { check_version(node_package_version) }.to raise_error(error) + it "logs" do + allow(Rails.logger).to receive(:warn) + message = /ReactOnRails: ReactOnRails gem and node package versions do not match/ + check_version_and_log(node_package_version) + expect(Rails.logger).to have_received(:warn).with(message) end end @@ -87,8 +97,20 @@ 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 + 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 "log method returns nil" do + expect(check_version_and_log(node_package_version)).to be_nil end end end @@ -102,14 +124,32 @@ 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) } + describe "#build" do + it "initializes NodePackageVersion with ReactOnRails.configuration.node_modules_location" do + allow(ReactOnRails).to receive_message_chain(:configuration, :node_modules_location).and_return("spec/dummy") + root_package_json_path = File.expand_path("../../package.json", __dir__) + allow(Rails).to receive_message_chain(:root, :join).and_return(root_package_json_path) + message = "No 'react-on-rails' entry in the dependencies of #{root_package_json_path}, which is " \ + "the expected location according to ReactOnRails.configuration.node_modules_location" + allow(Rails.logger).to receive(:warn) + described_class.build.raw + expect(Rails.logger).to have_received(:warn).with(message) + end + end + describe "#semver_wildcard?" do context "when package json lists an exact version of '0.0.2'" do let(:package_json) { File.expand_path("fixtures/normal_package.json", __dir__) } @@ -193,6 +233,46 @@ def check_version(node_package_version) specify { expect(node_package_version.major_minor_patch).to be_nil } end end + + context "with node version of 'file:.yalc/react-on-rails'" do + let(:package_json) { File.expand_path("fixtures/yalc_package.json", __dir__) } + + describe "#raw" do + specify { expect(node_package_version.raw).to eq("file:.yalc/react-on-rails") } + end + + describe "#relative_path?" do + specify { expect(node_package_version.relative_path?).to be true } + end + + describe "#major" do + specify { expect(node_package_version.major_minor_patch).to be_nil } + end + end + + context "with package.json without react-on-rails dependency" do + let(:package_json) { File.expand_path("../../package.json", __dir__) } + + describe "#raw" do + it "returns nil" do + root_package_json_path = File.expand_path("fixtures/nonexistent_package.json", __dir__) + allow(Rails).to receive_message_chain(:root, :join).and_return(root_package_json_path) + expect(node_package_version.raw).to be_nil + end + end + end + + context "with non-existing package.json" do + let(:package_json) { File.expand_path("fixtures/nonexistent_package.json", __dir__) } + + describe "#raw" do + it "returns nil" do + root_package_json_path = File.expand_path("fixtures/nonexistent_package.json", __dir__) + allow(Rails).to receive_message_chain(:root, :join).and_return(root_package_json_path) + expect(node_package_version.raw).to be_nil + end + end + end end end end