Skip to content

Fix RSpec helper optimization gap with private SSR directories #1838

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix RSpec helper optimization gap with private SSR directories #1838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bec534b
Ensure server bundle is included in webpack_generated_files
ihabadham Sep 29, 2025
7bfe46b
Add comprehensive tests for ensure_webpack_generated_files_exists
ihabadham Sep 29, 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
11 changes: 8 additions & 3 deletions lib/react_on_rails/configuration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,15 +320,20 @@ def configure_generated_assets_dirs_deprecation
end

def ensure_webpack_generated_files_exists
return unless webpack_generated_files.empty?

self.webpack_generated_files = [
all_required_files = [
"manifest.json",
server_bundle_js_file,
rsc_bundle_js_file,
react_client_manifest_file,
react_server_client_manifest_file
].compact_blank

Comment on lines +323 to +330
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 | 🟠 Major

🧩 Analysis chain

Confirm ActiveSupport availability for compact_blank (or require the extension)

Array#compact_blank is from ActiveSupport core extensions. If the gem supports older Rails/AS versions, this will break unless the extension is loaded.

Run to verify dependency floor and usage:

🏁 Script executed:

#!/bin/bash
# Check declared minimum Rails/AS and usages of compact_blank
fd -a '*.gemspec' | xargs -I{} rg -n 'add_(runtime_)?dependency.*(rails|activesupport)' {}
rg -n 'compact_blank' -g '!**/tmp/**'

Length of output: 437

🏁 Script executed:

#!/bin/bash
# Correct search for gemspec dependencies on rails/activesupport
echo "Searching gemspec dependencies..."
fd --extension gemspec --exec rg -n "add_(runtime_)?dependency.*(rails|activesupport)" {}

echo "Searching for .compact_blank usage..."
rg -nP "\.compact_blank" -g "!**/tmp/**"

echo "Searching for explicit requires of ActiveSupport core extensions..."
rg -n "require.*active_support/core_ext" -g "!**/tmp/**"

Length of output: 537

#!/bin/bash
echo "Locate `.compact_blank` in configuration:"
rg --no-ignore -nP "\.compact_blank" lib/react_on_rails/configuration.rb

echo -e "\nInspect top of configuration file for requires:"
sed -n '1,50p' lib/react_on_rails/configuration.rb

echo -e "\nSearch for any ActiveSupport core_ext requires across lib/:"
rg --no-ignore -n "require .*active_support/core_ext" -g "lib/**"

Require ActiveSupport or replace compact_blank in configuration.rb
Ensure compact_blank is available on Rails <6.1 by adding

require 'active_support/core_ext/enumerable'

at the top of lib/react_on_rails/configuration.rb—or replace .compact_blank with .reject(&:blank?) to maintain compatibility with Rails 5.2–6.0.

🤖 Prompt for AI Agents 
In lib/react_on_rails/configuration.rb around lines 323 to 330 the call to
.compact_blank may not exist on Rails < 6.1; either add a require for
ActiveSupport by adding require 'active_support/core_ext/enumerable' at the top
of the file, or replace .compact_blank with .reject(&:blank?) on that
all_required_files array to restore compatibility with Rails 5.2–6.0.

if webpack_generated_files.empty?
self.webpack_generated_files = all_required_files
else
missing_files = all_required_files.reject { |file| webpack_generated_files.include?(file) }
self.webpack_generated_files += missing_files if missing_files.any?
end
end

def configure_skip_display_none_deprecation
Expand Down
146 changes: 146 additions & 0 deletions spec/react_on_rails/configuration_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,152 @@ module ReactOnRails
end
end
end

describe "#ensure_webpack_generated_files_exists" do
let(:config) { described_class.new }

before do
# Reset to test defaults
config.server_bundle_js_file = "server-bundle.js"
config.rsc_bundle_js_file = nil
config.react_client_manifest_file = "react-client-manifest.json"
config.react_server_client_manifest_file = "react-server-client-manifest.json"
end

context "when webpack_generated_files has default manifest.json only" do
it "automatically includes server bundle when configured" do
config.webpack_generated_files = %w[manifest.json]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).to eq(%w[
manifest.json
server-bundle.js
react-client-manifest.json
react-server-client-manifest.json
])
end

it "does not duplicate manifest.json" do
config.webpack_generated_files = %w[manifest.json]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files.count("manifest.json")).to eq(1)
end
end

context "when webpack_generated_files is empty" do
it "populates with all required files" do
config.webpack_generated_files = []

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).to eq(%w[
manifest.json
server-bundle.js
react-client-manifest.json
react-server-client-manifest.json
])
end
end

context "when server bundle already included" do
it "does not duplicate entries" do
config.webpack_generated_files = %w[manifest.json server-bundle.js]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).to eq(%w[
manifest.json
server-bundle.js
react-client-manifest.json
react-server-client-manifest.json
])
expect(config.webpack_generated_files.count("server-bundle.js")).to eq(1)
end
end

context "when custom files are configured" do
it "preserves custom files and adds missing critical files" do
config.webpack_generated_files = %w[manifest.json custom-bundle.js]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).to include("manifest.json")
expect(config.webpack_generated_files).to include("custom-bundle.js")
expect(config.webpack_generated_files).to include("server-bundle.js")
expect(config.webpack_generated_files).to include("react-client-manifest.json")
end
end

context "when server bundle is not configured" do
it "does not add nil server bundle" do
config.server_bundle_js_file = nil
config.webpack_generated_files = %w[manifest.json]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).not_to include(nil)
expect(config.webpack_generated_files).not_to include("server-bundle.js")
expect(config.webpack_generated_files).to include("manifest.json")
end
end

context "when RSC bundle is configured" do
it "includes RSC bundle in monitoring" do
config.rsc_bundle_js_file = "rsc-bundle.js"
config.webpack_generated_files = %w[manifest.json]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).to include("rsc-bundle.js")
expect(config.webpack_generated_files).to include("server-bundle.js")
expect(config.webpack_generated_files).to include("manifest.json")
end
end

context "when React manifests are not configured" do
it "does not add nil React manifests" do
config.react_client_manifest_file = nil
config.react_server_client_manifest_file = nil
config.webpack_generated_files = %w[manifest.json]

config.send(:ensure_webpack_generated_files_exists)

expect(config.webpack_generated_files).not_to include(nil)
expect(config.webpack_generated_files).to eq(%w[manifest.json server-bundle.js])
end
end

context "when ensuring server bundle monitoring for RSpec optimization" do
it "ensures server bundle in private directory is monitored with default config" do
# Simulate default generator configuration
config.webpack_generated_files = %w[manifest.json]
config.server_bundle_js_file = "server-bundle.js"
config.server_bundle_output_path = "ssr-generated"

config.send(:ensure_webpack_generated_files_exists)

# Critical: server bundle must be included for RSpec helper optimization to work
expect(config.webpack_generated_files).to include("server-bundle.js")
end

it "handles all files being in different directories" do
# Simulate cross-directory scenario from PR #1798
config.webpack_generated_files = %w[manifest.json]
config.server_bundle_js_file = "server-bundle.js"
config.server_bundle_output_path = "ssr-generated"
config.generated_assets_dir = "public/packs"

config.send(:ensure_webpack_generated_files_exists)

# All critical files should be monitored regardless of directory
expect(config.webpack_generated_files).to include("manifest.json")
expect(config.webpack_generated_files).to include("server-bundle.js")
end
end
end
end
end

Expand Down
Loading