Fix: Add Rails 5.2-6.0 compatibility for compact_blank

[justin808]

Summary

Adds require "active_support/core_ext/enumerable" to ensure Rails 5.2-6.0 compatibility when using compact_blank method.

Details

The compact_blank method used in ensure_webpack_generated_files_exists was introduced in Rails 6.1. By requiring the ActiveSupport core extension, we ensure this method is available in Rails 5.2-6.0 environments.

Changes

• Added require "active_support/core_ext/enumerable" at the top of lib/react_on_rails/configuration.rb
• This ensures compact_blank is available for older Rails versions

Testing

• All existing RSpec tests pass (54 examples in configuration_spec.rb)
• RuboCop passes with no violations

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Improved webpack asset handling to dynamically include additional bundles when the Pro features are present.

• Bug Fix / Compatibility

  • Added a compatibility fallback for an array utility to ensure behavior on older Rails versions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Configuration updated to require ActiveSupport blank/Enumerable extensions and refactor webpack_generated_files assembly to start with base files, append ReactOnRailsPro files when detected, and use compact_blank to remove nils.

Changes

Cohort / File(s)	Summary
Configuration refactor lib/react_on_rails/configuration.rb	Added requires for ActiveSupport enumerable/object blank extensions. Rewrote ensure_webpack_generated_files_exists to build webpack_generated_files incrementally: always include "manifest.json" and server_bundle_js_file; if ReactOnRailsPro is present, append rsc_bundle_js_file, react_client_manifest_file, and react_server_client_manifest_file; finalize with compact_blank.

Sequence Diagram(s)

sequenceDiagram
    participant Config as Configuration
    participant Pro as ReactOnRailsPro (optional)
    participant FS as Files List

    Note over Config: ensure_webpack_generated_files_exists
    Config->>FS: start list ["manifest.json", server_bundle_js_file]
    alt ReactOnRailsPro detected
        Pro-->>Config: provide rsc_bundle_js_file, react_client_manifest_file, react_server_client_manifest_file
        Config->>FS: append provided Pro files
    end
    Config->>Config: call compact_blank on list
    Config->>FS: assign webpack_generated_files

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Single-file change with limited logic.
• Pay attention to correct requires and behavior of compact_blank on older Rails.
• Verify Pro detection and that appended values are correct and nil-safe.

Poem

🐰 A tiny patch hops through the trees,
Files gather neatly with nimble ease,
Pro bits join only when they're found,
Blanks are pruned and order's sound,
Configs snug — the build hums free.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding Rails 5.2-6.0 compatibility for the compact_blank method, which is the core fix in the PR.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/webpack-ci-fixes

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0dc07ac and 6334ed3.

📒 Files selected for processing (1)

• lib/react_on_rails/configuration.rb (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• lib/react_on_rails/configuration.rb

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: rspec-package-tests (3.4, latest)
• GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
• GitHub Check: examples (3.4, latest)
• GitHub Check: build
• GitHub Check: claude-review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[justin808]

@claude review this PR

[claude]

Claude finished @justin808's task —— View job

---

PR Review Complete ✅

Critical Issue Found: The PR description does not match the actual implementation.

---

Summary

This PR aims to add Rails 5.2-6.0 compatibility for the compact_blank method used in lib/react_on_rails/configuration.rb:372. However, there's a significant discrepancy between what the PR description claims and what was actually implemented.

---

Issues Found

🚨 Critical: Missing require Statement

Location: lib/react_on_rails/configuration.rb

The PR description states:

Added require "active_support/core_ext/enumerable" at the top of lib/react_on_rails/configuration.rb

However, this require statement is NOT present in the current code. The file begins with:

# frozen_string_literal: true

# rubocop:disable Metrics/ClassLength

module ReactOnRails
  # ...

There is no require "active_support/core_ext/enumerable" statement anywhere in the file.

Impact: Without this require statement, the compact_blank method will still fail on Rails 5.2-6.0 because:

• compact_blank was introduced in Rails 6.1
• The gem supports rails >= 5.2 (per react_on_rails.gemspec:42)
• Rails 5.2, 6.0 users will encounter a NoMethodError when this code executes

---

Analysis

Current Code

Line 372 uses compact_blank:

self.webpack_generated_files = files.compact_blank

Context Check

I verified that:

1. ✅ This is the only usage of compact_blank in the entire lib/ directory
2. ✅ All other similar code uses .compact instead (8 other occurrences)
3. ❌ No require statement for ActiveSupport enumerable extension exists
4. ✅ Other files properly require their ActiveSupport dependencies:
   • lib/react_on_rails/helper.rb:14 - require "active_support/concern"
   • lib/react_on_rails/utils.rb:6-7 - require "active_support" and core_ext/string
   • lib/react_on_rails/json_output.rb:3 - require "active_support/core_ext/string/output_safety"

---

Recommendations

Option 1: Add the Missing Require (As Described)

Add this line at the top of lib/react_on_rails/configuration.rb after the frozen_string_literal comment:

# frozen_string_literal: true

require "active_support/core_ext/enumerable"

# rubocop:disable Metrics/ClassLength

This would match the PR description and ensure compact_blank is available in Rails 5.2-6.0.

Option 2: Use .compact.reject(&:blank?) Instead (More Explicit)

Replace line 372:

# Before:
self.webpack_generated_files = files.compact_blank

# After:
self.webpack_generated_files = files.compact.reject(&:blank?)

This works on all Rails versions without requiring additional dependencies, though it's slightly more verbose.

Option 3: Use .compact Only (Simplest)

If blank? checking isn't actually needed (i.e., only nil values need filtering), replace line 372:

self.webpack_generated_files = files.compact

Looking at the code, the files array contains:

• "manifest.json" (always present, never blank)
• server_bundle_js_file (could be empty string "")
• Pro-specific files (accessed conditionally)

If server_bundle_js_file can be an empty string "", then compact_blank is appropriate. Otherwise, compact would suffice.

---

Testing Recommendations

1. Add a test for ensure_webpack_generated_files_exists to verify it works correctly with:

   • Empty server_bundle_js_file ("")
   • Nil server_bundle_js_file
   • Pro vs non-Pro scenarios

2. Test on Rails 5.2/6.0 explicitly to ensure the fix works

3. Verify RuboCop passes (already mentioned as passing)

---

Verdict

❌ Request Changes: The PR description claims functionality that isn't implemented. Please either:

1. Add the missing require statement as described, OR
2. Update the implementation to use a Rails 5.2+ compatible approach

The current code will fail on Rails 5.2-6.0 deployments.

---

[claude]

Code Review: PR #2058 - Fix Rails 5.2-6.0 Compatibility for compact_blank

✅ Positive Findings

1. Correct Problem Identification

• The PR correctly identifies that compact_blank was introduced in Rails 6.1 and is not available in Rails 5.2-6.0
• The gemspec shows rails >= 5.2 as a dependency, so this fix is necessary for backward compatibility

2. Proper Solution Approach

• Adding require "active_support/core_ext/enumerable" is the correct way to backport compact_blank to older Rails versions
• The require statement is placed at the top of the file (line 3) following best practices

3. Code Quality Improvements

• The refactored code in ensure_webpack_generated_files_exists is actually more readable than before
• Clearer separation of concerns: base files vs Pro-specific files
• Better variable naming with explicit files array

4. No Breaking Changes

• The functional behavior remains identical
• All existing tests pass (54 examples in configuration_spec.rb)
• RuboCop passes with no violations

🔍 Observations & Suggestions

1. Pro Configuration Access Pattern (lib/react_on_rails/configuration.rb:421-425)
The code directly accesses Pro configuration methods:

pro_config = ReactOnRailsPro.configuration
files << pro_config.rsc_bundle_js_file
files << pro_config.react_client_manifest_file
files << pro_config.react_server_client_manifest_file

Question: Are these methods guaranteed to exist in all Pro versions, or should there be additional nil checks? If Pro users are running an older Pro version, this could raise NoMethodErrors. Consider:

if ReactOnRails::Utils.react_on_rails_pro?
  pro_config = ReactOnRailsPro.configuration
  files << pro_config.rsc_bundle_js_file if pro_config.respond_to?(:rsc_bundle_js_file)
  files << pro_config.react_client_manifest_file if pro_config.respond_to?(:react_client_manifest_file)
  files << pro_config.react_server_client_manifest_file if pro_config.respond_to?(:react_server_client_manifest_file)
end

However, if Pro versioning is tightly coupled with the base gem, this may not be necessary.

2. Test Coverage
While existing tests pass, there's no specific test for the Rails 5.2-6.0 compatibility fix. Consider adding a test that:

• Verifies compact_blank is available after requiring the file
• Tests the ensure_webpack_generated_files_exists method specifically with both nil and empty string values

3. Alternative Implementation (Minor)
The current implementation could be slightly simplified:

files = ["manifest.json", server_bundle_js_file]

if ReactOnRails::Utils.react_on_rails_pro?
  pro_config = ReactOnRailsPro.configuration
  files.concat([
    pro_config.rsc_bundle_js_file,
    pro_config.react_client_manifest_file,
    pro_config.react_server_client_manifest_file
  ])
end

self.webpack_generated_files = files.compact_blank

Using concat might be marginally more efficient than multiple << operations, but the current approach is perfectly fine and arguably more explicit.

🎯 Security & Performance

Security: ✅ No security concerns. This is a configuration file change with no user input handling.

Performance: ✅ No performance impact. The compact_blank operation is O(n) where n is a small fixed number (~5 files max).

📋 Compliance with CLAUDE.md

✅ RuboCop: Passes with no violations
✅ Formatting: Properly formatted
✅ File endings: Newline present
✅ Code style: Follows project conventions
⚠️ Changelog: Should this be added to CHANGELOG.md? This is a bug fix for backward compatibility, which typically qualifies as user-visible.

🎬 Recommendation

APPROVE with minor suggestion: The code is solid and fixes a real compatibility issue. The only suggestion is to consider defensive checks for Pro configuration methods (item #1 above) if Pro version compatibility could be an issue.

Action Items:

1. Consider adding a CHANGELOG.md entry for this bug fix
2. Consider defensive method checks for Pro configuration (if applicable)
3. Optional: Add a specific test for Rails 5.2-6.0 compatibility

Great work on identifying and fixing this backward compatibility issue! 🚀

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d87578 and 0dc07ac.

📒 Files selected for processing (1)

• lib/react_on_rails/configuration.rb (2 hunks)

🧰 Additional context used

🧠 Learnings (11)

📓 Common learnings

Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.

Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.

📚 Learning: 2025-09-16T08:01:11.146Z

Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-10-23T17:22:01.074Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-04-26T21:55:55.874Z

Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-02-18T13:08:01.477Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-02-12T16:38:06.537Z

Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-09-15T21:24:48.207Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-07-08T05:57:29.630Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-02-13T16:50:26.861Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-02-13T16:50:47.848Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.

Applied to files:

• lib/react_on_rails/configuration.rb

📚 Learning: 2025-04-09T12:56:10.756Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadContainer.ts:0-0
Timestamp: 2025-04-09T12:56:10.756Z
Learning: In the react_on_rails codebase, RSC payloads are already stringified using `JSON.stringify()` before being processed by the `escapeScript` function, which handles escaping of special characters. The function only needs to handle specific HTML markers like comments and closing script tags.

Applied to files:

• lib/react_on_rails/configuration.rb

🧬 Code graph analysis (1)

lib/react_on_rails/configuration.rb (2)

lib/react_on_rails/utils.rb (1)

• react_on_rails_pro? (256-264)

react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)

• configuration (9-38)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: rspec-package-tests (3.4, latest)
• GitHub Check: examples (3.4, latest)
• GitHub Check: build
• GitHub Check: claude-review

🔇 Additional comments (1)

lib/react_on_rails/configuration.rb (1)

365-375: Good refactoring to make Pro file inclusion explicit.

The logic correctly builds the webpack generated files list by:

• Starting with core files (manifest.json and server bundle)
• Conditionally adding Pro-specific RSC files when licensed
• Filtering out nil/blank values

This is more maintainable than the previous hard-coded array and properly separates Pro vs. non-Pro concerns.

Note: This approval is contingent on addressing the compact_blank compatibility issue flagged in the previous comment.