Fix bug at resolving react-server-client-manifest.json file (#1818)

* Enhance server bundle handling by adding support for react_server_client_manifest_file in server_bundle? method and refactoring react_server_manifest_path assignment to use bundle_js_file_path. This improves configuration flexibility and maintains consistency in bundle management.

* Fix failing tests after react_server_client_manifest_file changes

- Add react_server_client_manifest_file parameter to mock_bundle_configs helper
- Update react_server_client_manifest_file_path tests to mock bundle_js_file_path instead of public_bundles_full_path
- Ensure trailing newline in spec file

These changes fix the CI failures introduced by using bundle_js_file_path for manifest resolution.

Co-authored-by: Abanoub Ghadban <[email protected]>

* Revert "Fix failing tests after react_server_client_manifest_file changes"

This reverts commit 81c385f.

* Enhance tests for react_server_client_manifest_file_path

- Added support for react_server_client_manifest_file in tests.
- Updated test cases to mock bundle_js_file_path instead of public_bundles_full_path.
- Improved caching behavior tests for development and non-development environments.
- Ensured proper error handling when manifest file name is nil.

These changes improve test coverage and reliability for the react_server_client_manifest_file_path method.

---------

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Abanoub Ghadban <[email protected]>

Author: AbanoubGhadban

lib/react_on_rails/utils.rb

@@ -112,7 +112,8 @@ def self.bundle_js_file_path(bundle_name)
     private_class_method def self.server_bundle?(bundle_name)
       config = ReactOnRails.configuration
       bundle_name == config.server_bundle_js_file ||
-      bundle_name == config.rsc_bundle_js_file
+      bundle_name == config.rsc_bundle_js_file ||
+      bundle_name == config.react_server_client_manifest_file
     end
 
     private_class_method def self.handle_missing_manifest_entry(bundle_name, is_server_bundle)
@@ -170,7 +171,7 @@ def self.react_server_client_manifest_file_path
               "react_server_client_manifest_file is nil, ensure it is set in your configuration"
       end
 
-      @react_server_manifest_path = File.join(public_bundles_full_path, asset_name)
+      @react_server_manifest_path = bundle_js_file_path(asset_name)
     end
 
     def self.running_on_windows?

spec/react_on_rails/utils_spec.rb

@@ -98,6 +98,8 @@ def mock_dev_server_running
             .and_return("server-bundle.js")
           allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
             .and_return("rsc-bundle.js")
+          allow(ReactOnRails).to receive_message_chain("configuration.react_server_client_manifest_file")
+            .and_return("react-server-client-manifest.json")
         end
 
         subject do
@@ -786,90 +788,50 @@ def mock_dev_server_running
     end
 
     describe ".react_server_client_manifest_file_path" do
+      let(:asset_name) { "react-server-client-manifest.json" }
+
       before do
         described_class.instance_variable_set(:@react_server_manifest_path, nil)
-        allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file)
-          .and_return("react-server-client-manifest.json")
+        allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file).and_return(asset_name)
         allow(Rails.env).to receive(:development?).and_return(false)
       end
 
       after do
         described_class.instance_variable_set(:@react_server_manifest_path, nil)
       end
 
-      context "when in development environment" do
-        before do
-          allow(Rails.env).to receive(:development?).and_return(true)
-          allow(described_class).to receive(:public_bundles_full_path)
-            .and_return("/path/to/generated/assets")
-        end
-
-        it "does not use cached path" do
-          # Call once to potentially set the cached path
-          described_class.react_server_client_manifest_file_path
-
-          # Change the configuration value
-          allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file)
-            .and_return("changed-manifest.json")
-
-          # Should use the new value
-          expect(described_class.react_server_client_manifest_file_path)
-            .to eq("/path/to/generated/assets/changed-manifest.json")
-        end
+      it "calls bundle_js_file_path with the correct asset name and returns its value" do
+        allow(described_class).to receive(:bundle_js_file_path).with(asset_name).and_return("/some/path/#{asset_name}")
+        result = described_class.react_server_client_manifest_file_path
+        expect(described_class).to have_received(:bundle_js_file_path).with(asset_name)
+        expect(result).to eq("/some/path/#{asset_name}")
       end
 
-      context "when not in development environment" do
-        before do
-          allow(described_class).to receive(:public_bundles_full_path)
-            .and_return("/path/to/generated/assets")
-        end
-
-        it "caches the path" do
-          # Call once to set the cached path
-          expected_path = "/path/to/generated/assets/react-server-client-manifest.json"
-          expect(described_class.react_server_client_manifest_file_path).to eq(expected_path)
-
-          # Change the configuration value
-          allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file)
-            .and_return("changed-manifest.json")
-
-          # Should still use the cached path
-          expect(described_class.react_server_client_manifest_file_path).to eq(expected_path)
-        end
+      it "caches the path when not in development" do
+        allow(described_class).to receive(:bundle_js_file_path).with(asset_name).and_return("/some/path/#{asset_name}")
+        result1 = described_class.react_server_client_manifest_file_path
+        result2 = described_class.react_server_client_manifest_file_path
+        expect(described_class).to have_received(:bundle_js_file_path).once.with(asset_name)
+        expect(result1).to eq("/some/path/#{asset_name}")
+        expect(result2).to eq("/some/path/#{asset_name}")
       end
 
-      context "with different manifest file names" do
-        before do
-          allow(described_class).to receive(:public_bundles_full_path)
-            .and_return("/path/to/generated/assets")
-        end
-
-        it "returns the correct path for default manifest name" do
-          allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file)
-            .and_return("react-server-client-manifest.json")
-
-          expect(described_class.react_server_client_manifest_file_path)
-            .to eq("/path/to/generated/assets/react-server-client-manifest.json")
-        end
-
-        it "returns the correct path for custom manifest name" do
-          allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file)
-            .and_return("custom-server-client-manifest.json")
-
-          expect(described_class.react_server_client_manifest_file_path)
-            .to eq("/path/to/generated/assets/custom-server-client-manifest.json")
-        end
+      it "does not cache the path in development" do
+        allow(Rails.env).to receive(:development?).and_return(true)
+        allow(described_class).to receive(:bundle_js_file_path).with(asset_name).and_return("/some/path/#{asset_name}")
+        result1 = described_class.react_server_client_manifest_file_path
+        result2 = described_class.react_server_client_manifest_file_path
+        expect(described_class).to have_received(:bundle_js_file_path).twice.with(asset_name)
+        expect(result1).to eq("/some/path/#{asset_name}")
+        expect(result2).to eq("/some/path/#{asset_name}")
       end
 
-      context "with nil manifest file name" do
+      context "when manifest file name is nil" do
         before do
-          allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file)
-            .and_return(nil)
-          allow(described_class).to receive(:public_bundles_full_path)
-            .and_return("/path/to/generated/assets")
+          allow(ReactOnRails.configuration).to receive(:react_server_client_manifest_file).and_return(nil)
         end
 
-        it "raises an error when the manifest file name is nil" do
+        it "raises an error" do
           expect { described_class.react_server_client_manifest_file_path }
             .to raise_error(ReactOnRails::Error, /react_server_client_manifest_file is nil/)
         end