Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 24, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b7c076
Enhance server bundle handling by adding support for react_server_cli…
AbanoubGhadban Sep 24, 2025
81c385f
Fix failing tests after react_server_client_manifest_file changes
github-actions[bot] Sep 24, 2025
19e3650
Revert "Fix failing tests after react_server_client_manifest_file cha…
AbanoubGhadban Sep 24, 2025
2f267d8
Enhance tests for react_server_client_manifest_file_path
AbanoubGhadban Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/react_on_rails/utils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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?
Expand Down
94 changes: 28 additions & 66 deletions spec/react_on_rails/utils_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +101 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Good stub; add manifest-specific resolution tests via bundle_js_file_path

Stubbing react_server_client_manifest_file here is correct to exercise server-bundle detection. Please add focused tests that call bundle_js_file_path(asset_name) directly to verify private/public path selection for the manifest (enforcement on/off), ensuring the new “treat manifest as server bundle” behavior is covered end-to-end.

Example tests to add under describe ".bundle_js_file_path" (inside the packer context):

context "when resolving react_server_client_manifest via bundle_js_file_path" do
  let(:asset_name) { "react-server-client-manifest.json" }
  let(:public_path) { File.expand_path(File.join(packer_public_output_path, asset_name)) }
  let(:ssr_generated_path) { File.expand_path(File.join("ssr-generated", asset_name)) }

  before do
    # Not in manifest
    mock_missing_manifest_entry(asset_name)

    allow(ReactOnRails).to receive_message_chain("configuration.react_server_client_manifest_file")
      .and_return(asset_name)
  end

  context "with enforce_private_server_bundles=false" do
    before do
      allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path").and_return("ssr-generated")
      allow(ReactOnRails).to receive_message_chain("configuration.enforce_private_server_bundles").and_return(false)
    end

    it "prefers the private path when it exists" do
      allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
      allow(File).to receive(:exist?).with(public_path).and_return(true)

      expect(described_class.bundle_js_file_path(asset_name)).to eq(ssr_generated_path)
    end

    it "falls back to public path if private does not exist" do
      allow(File).to receive(:exist?).with(ssr_generated_path).and_return(false)
      allow(File).to receive(:exist?).with(public_path).and_return(true)

      expect(described_class.bundle_js_file_path(asset_name)).to eq(public_path)
    end
  end

  context "with enforce_private_server_bundles=true" do
    before do
      allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path").and_return("ssr-generated")
      allow(ReactOnRails).to receive_message_chain("configuration.enforce_private_server_bundles").and_return(true)
    end

    it "always returns the private path and does not check public directory" do
      expect(File).not_to receive(:exist?).with(public_path)
      expect(described_class.bundle_js_file_path(asset_name)).to eq(ssr_generated_path)
    end
  end
end

To confirm production alignment, run:

🏁 Script executed:

#!/bin/bash
# Verify server_bundle? treats the manifest as server-related and that the path method delegates correctly.
rg -nP -C3 'server_bundle\?' --type rb
rg -nP -C2 'react_server_client_manifest_file' --type rb
rg -nP -C3 'def\s+react_server_client_manifest_file_path' lib/react_on_rails/utils.rb

Length of output: 53

🏁 Script executed:

#!/bin/bash
# Locate the Utils file
fd utils.rb

# Search for server_bundle?, manifest getter, and bundle_js_file_path definitions/usages
rg -n 'server_bundle\?' -C3 .
rg -n 'react_server_client_manifest_file' -C3 .
rg -n 'bundle_js_file_path' -C3 lib/react_on_rails/utils.rb

Length of output: 20783

Add manifest-specific bundle_js_file_path specs
Ensure bundle_js_file_path treats react_server_client_manifest_file as a server bundle—add focused examples under spec/react_on_rails/utils_spec.rb (packer context) that verify private vs. public path selection with enforce_private_server_bundles set to false and true.

🤖 Prompt for AI Agents 
In spec/react_on_rails/utils_spec.rb around lines 101-102, add focused examples
in the packer context to verify that bundle_js_file_path treats
react_server_client_manifest_file as a server bundle: mock
ReactOnRails.configuration.react_server_client_manifest_file to
"react-server-client-manifest.json" and add two examples asserting the returned
path uses the public path when enforce_private_server_bundles is false and uses
the private (server) path when enforce_private_server_bundles is true; keep the
tests scoped to bundle_js_file_path behavior and only toggle
enforce_private_server_bundles for each example.

end

subject do
Expand Down Expand Up @@ -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
Expand Down
Loading