Allow URLs and other local paths in version checker

Author: alexeyr

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    react_on_rails (14.0.5)
+    react_on_rails (15.0.0.alpha.1)
       addressable
       connection_pool
       execjs (~> 2.5)

lib/react_on_rails/version_checker.rb

@@ -20,7 +20,7 @@ def initialize(node_package_version)
     # unless the user really knows what they're doing. So we will give a
     # warning if they do not.
     def log_if_gem_and_node_package_versions_differ
-      return if node_package_version.raw.nil? || node_package_version.relative_path?
+      return if node_package_version.raw.nil? || node_package_version.local_path_or_url?
       return log_node_semver_version_warning if node_package_version.semver_wildcard?
 
       node_major_minor_patch = node_package_version.major_minor_patch
@@ -81,33 +81,41 @@ def initialize(package_json)
       end
 
       def raw
+        return @raw if defined?(@raw)
+
         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"]
+            return @raw = 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
+        @raw = nil
       end
 
       def semver_wildcard?
-        raw.match(/[~^]/).present?
+        # See https://docs.npmjs.com/cli/v10/configuring-npm/package-json#dependencies
+        # We want to disallow all expressions other than exact versions
+        # and the ones allowed by local_path_or_url?
+        raw.blank? || raw.match(/[~^><|*-]/).present?
       end
 
-      def relative_path?
-        raw.match(/(\.\.|\Afile:)/).present?
+      def local_path_or_url?
+        # See https://docs.npmjs.com/cli/v10/configuring-npm/package-json#dependencies
+        # All path and protocol "version ranges" include / somewhere,
+        # but we want to make an exception for npm:@scope/pkg@version.
+        !raw.nil? && raw.include?("/") && !raw.start_with?("npm:")
       end
 
       def major_minor_patch
-        return if relative_path?
+        return if local_path_or_url?
 
         match = raw.match(MAJOR_MINOR_PATCH_VERSION_REGEX)
         unless match
-          raise ReactOnRails::Error, "Cannot parse version number '#{raw}' (wildcard versions are not supported)"
+          raise ReactOnRails::Error, "Cannot parse version number '#{raw}' (only exact versions are supported)"
         end
 
         [match[1], match[2], match[3]]

spec/react_on_rails/fixtures/git_package.json

@@ -0,0 +1,10 @@
+{
+  "dependencies": {
+    "babel": "^6.3.26",
+    "react-on-rails": "git://github.com/shakacode/react-on-rails.git",
+    "webpack": "^1.12.8"
+  },
+  "devDependencies": {
+    "babel-eslint": "^5.0.0-beta6"
+  }
+}

spec/react_on_rails/fixtures/semver_range_package.json

@@ -0,0 +1,10 @@
+{
+  "dependencies": {
+    "babel": "^6.3.26",
+    "react-on-rails": ">=1.0.0 <2.0.0",
+    "webpack": "^1.12.8"
+  },
+  "devDependencies": {
+    "babel-eslint": "^5.0.0-beta6"
+  }
+}

spec/react_on_rails/version_checker_spec.rb

@@ -92,7 +92,7 @@ module ReactOnRails # rubocop:disable Metrics/ModuleLength
 
       context "when package json uses a relative path with dots" do
         let(:node_package_version) do
-          double_package_version(raw: "../../..", major_minor_patch: "", relative_path: true)
+          double_package_version(raw: "../../..", major_minor_patch: "", local_path_or_url: true)
         end
 
         before { stub_gem_version("2.0.0.beta.1") }
@@ -116,12 +116,12 @@ module ReactOnRails # rubocop:disable Metrics/ModuleLength
     end
 
     def double_package_version(raw: nil, semver_wildcard: false,
-                               major_minor_patch: nil, relative_path: false)
+                               major_minor_patch: nil, local_path_or_url: false)
       instance_double(VersionChecker::NodePackageVersion,
                       raw: raw,
                       semver_wildcard?: semver_wildcard,
                       major_minor_patch: major_minor_patch,
-                      relative_path?: relative_path)
+                      local_path_or_url?: local_path_or_url)
     end
 
     def check_version_and_raise(node_package_version)
@@ -168,6 +168,12 @@ def check_version_and_log(node_package_version)
 
           specify { expect(node_package_version.semver_wildcard?).to be true }
         end
+
+        context "when package json lists a version range of '>=1.2.3 <2.0.0'" do
+          let(:package_json) { File.expand_path("fixtures/semver_range_package.json", __dir__) }
+
+          specify { expect(node_package_version.semver_wildcard?).to be true }
+        end
       end
 
       context "when package json lists a version of '0.0.2'" do
@@ -177,8 +183,8 @@ def check_version_and_log(node_package_version)
           specify { expect(node_package_version.raw).to eq("0.0.2") }
         end
 
-        describe "#relative_path?" do
-          specify { expect(node_package_version.relative_path?).to be false }
+        describe "#local_path_or_url?" do
+          specify { expect(node_package_version.local_path_or_url?).to be false }
         end
 
         describe "#major" do
@@ -193,8 +199,8 @@ def check_version_and_log(node_package_version)
           specify { expect(node_package_version.raw).to eq("^14.0.0.beta-2") }
         end
 
-        describe "#relative_path?" do
-          specify { expect(node_package_version.relative_path?).to be false }
+        describe "#local_path_or_url?" do
+          specify { expect(node_package_version.local_path_or_url?).to be false }
         end
 
         describe "#major_minor_patch" do
@@ -209,8 +215,8 @@ def check_version_and_log(node_package_version)
           specify { expect(node_package_version.raw).to eq("../../..") }
         end
 
-        describe "#relative_path?" do
-          specify { expect(node_package_version.relative_path?).to be true }
+        describe "#local_path_or_url?" do
+          specify { expect(node_package_version.local_path_or_url?).to be true }
         end
 
         describe "#major" do
@@ -225,8 +231,8 @@ def check_version_and_log(node_package_version)
           specify { expect(node_package_version.raw).to eq("file:///Users/justin/shakacode/react_on_rails") }
         end
 
-        describe "#relative_path?" do
-          specify { expect(node_package_version.relative_path?).to be true }
+        describe "#local_path_or_url?" do
+          specify { expect(node_package_version.local_path_or_url?).to be true }
         end
 
         describe "#major" do
@@ -241,8 +247,24 @@ def check_version_and_log(node_package_version)
           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 }
+        describe "#local_path_or_url?" do
+          specify { expect(node_package_version.local_path_or_url?).to be true }
+        end
+
+        describe "#major" do
+          specify { expect(node_package_version.major_minor_patch).to be_nil }
+        end
+      end
+
+      context "with node version of `git:` URL" do
+        let(:package_json) { File.expand_path("fixtures/git_package.json", __dir__) }
+
+        describe "#raw" do
+          specify { expect(node_package_version.raw).to eq("git://github.com/shakacode/react-on-rails.git") }
+        end
+
+        describe "#local_path_or_url?" do
+          specify { expect(node_package_version.local_path_or_url?).to be true }
         end
 
         describe "#major" do