Enable use as a Git dependency (#1664)

* Enable use as a Git dependency * Allow URLs and other local paths in version checker --------- Co-authored-by: Alexey Romanov <[email protected]>
shakacode · Dec 23, 2024 · 4339d30 · 4339d30
1 parent 98fcc29
commit 4339d30
Show file tree

Hide file tree

Showing 8 changed files with 92 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,8 +20,10 @@ Changes since the last non-beta release.
 
 #### Fixed
 
+- Incorrect type and confusing name for `ReactOnRails.registerStore`, use `registerStoreGenerators` instead. [PR 1651](https://github.com/shakacode/react_on_rails/pull/1651) by [alexeyr-ci](https://github.com/alexeyr-ci).
 - 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).
+- Enable use as a `git:` dependency. [PR 1664](https://github.com/shakacode/react_on_rails/pull/1664) by [alexeyr-ci](https://github.com/alexeyr-ci).
 
 #### Added
 - Added streaming server rendering support:
@@ -35,9 +37,6 @@ Changes since the last non-beta release.
 #### Changed
 - Console replay script generation now awaits the render request promise before generating, allowing it to capture console logs from asynchronous operations. This requires using a version of the Node renderer that supports replaying async console logs. [PR #1649](https://github.com/shakacode/react_on_rails/pull/1649) by [AbanoubGhadban](https://github.com/AbanoubGhadban).
 
-#### Fixed
-- Incorrect type and confusing name for `ReactOnRails.registerStore`, use `registerStoreGenerators` instead. [PR 1651](https://github.com/shakacode/react_on_rails/pull/1651) by [alexeyr-ci](https://github.com/alexeyr-ci).
-
 ### [14.0.5] - 2024-08-20
 #### Fixed
 - Should force load react-components which send over turbo-stream [PR #1620](https://github.com/shakacode/react_on_rails/pull/1620) by [theforestvn88](https://github.com/theforestvn88).

diff --git a/Gemfile.lock b/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)

diff --git a/lib/react_on_rails/version_checker.rb b/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]]

diff --git a/package-scripts.yml b/package-scripts.yml
@@ -14,6 +14,22 @@ scripts:
       description: Run eslint in debug mode.
       script: DEBUG=eslint:cli-engine nps eslint
 
+  build:
+    prepack:
+      description: Build the project in the prepack script.
+      # This is necessary when used as a Git dependency since we don't have the dist directory in the repo.
+      # Depending on the package manager, `prepack`, `prepare`, or both may be run.
+      # They may also be run when publishing or in other cases, so we want to cover all of them.
+      # 1. If the project is already built, do nothing;
+      # 2. Build the project but ignore TypeScript errors from missing devDependencies;
+      # 3. Check if the project is built now;
+      # 4. If it failed, print an error message (still follow https://docs.npmjs.com/cli/v8/using-npm/scripts#best-practices).
+      script: >
+        [ -f node_package/lib/ReactOnRails.js ] ||
+          (npm run build >/dev/null 2>&1 || true) &&
+          [ -f node_package/lib/ReactOnRails.js ] ||
+          { echo 'Building react-on-rails seems to have failed!'; }
+
   format:
     default:
       description: Format files using prettier.

diff --git a/package.json b/package.json
@@ -61,6 +61,8 @@
     "test": "jest node_package/tests",
     "clean": "rm -rf node_package/lib",
     "start": "nps",
+    "prepack": "nps build.prepack",
+    "prepare": "nps build.prepack",
     "prepublishOnly": "yarn run build",
     "build": "yarn run clean && yarn run tsc --declaration",
     "build-watch": "yarn run clean && yarn run tsc --watch",

diff --git a/spec/react_on_rails/fixtures/git_package.json b/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"
+  }
+}
diff --git a/spec/react_on_rails/fixtures/semver_range_package.json b/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"
+  }
+}
diff --git a/spec/react_on_rails/version_checker_spec.rb b/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