Move Pro-specific utilities and configurations to Pro gem #1875
Conversation
- Updated CHANGELOG.md with breaking changes for RSC configs and streaming helpers - Updated react_on_rails_pro/CHANGELOG.md to document additions to Pro gem - Moved RSC configuration documentation to Pro gem docs - Added Pro-only view helpers section consolidating all Pro helpers - Added prominent Pro feature notices to streaming documentation - Removed inaccurate claims about error behavior Related to #1874
Moved RSC-specific configurations from open-source to Pro gem: - rsc_bundle_js_file - react_client_manifest_file - react_server_client_manifest_file Changes: - Removed RSC configs from ReactOnRails::Configuration - Added RSC configs to ReactOnRailsPro::Configuration with same defaults - Updated all code references to use Pro config when available - Updated Pro dummy app to use ReactOnRailsPro.configure for RSC config - Removed RSC config tests from open-source spec - Added comprehensive RSC config tests to Pro spec Files modified: - lib/react_on_rails/configuration.rb - lib/react_on_rails/utils.rb - lib/react_on_rails/test_helper/webpack_assets_status_checker.rb - lib/react_on_rails/packer_utils.rb - react_on_rails_pro/lib/react_on_rails_pro/configuration.rb - react_on_rails_pro/spec/dummy/config/initializers/*.rb - spec/react_on_rails/configuration_spec.rb - react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb Related to #1874
Code Review: Move Pro-specific utilities and configurations to Pro gemSummaryThis PR successfully migrates RSC-specific configurations from the open-source gem to the Pro gem. The approach is well-structured with clear documentation and migration paths. However, there are several critical bugs that must be fixed before merging. 🚨 Critical Issues1. Broken References in
|
- Remove RSC utility methods from ReactOnRails::Utils: - rsc_bundle_js_file_path - react_client_manifest_file_path - react_server_client_manifest_file_path - Add these methods to ReactOnRailsPro::Utils with same functionality - Update rsc_support_enabled? in open-source to delegate to Pro gem - Update all internal Pro gem references to use ReactOnRailsPro::Utils: - react_on_rails_pro/lib/react_on_rails_pro/request.rb - react_on_rails_pro/lib/react_on_rails_pro/utils.rb - Update webpack_assets_status_checker.rb to call ReactOnRailsPro::Utils - Remove RSC utility tests from open-source spec (lines 452-839) - Removed rsc_bundle_js_file_path tests - Removed react_client_manifest_file_path tests - Removed react_server_client_manifest_file_path tests - Update Pro gem test mocks to reference ReactOnRailsPro::Utils: - react_on_rails_pro/spec/react_on_rails_pro/utils_spec.rb - react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Review - PR #1875: Move Pro-specific utilities and configurations to Pro gemThank you for this well-structured PR! This is a great architectural improvement that creates clear separation between open-source and Pro features. ✅ Strengths1. Excellent Architecture & Design
2. Documentation Quality
3. Code Quality
|
WalkthroughThis PR moves RSC (React Server Components) configurations, path resolvers, and streaming view helpers out of the open-source ReactOnRails gem into the react_on_rails_pro gem. OSS now delegates RSC detection/paths to Pro when present and removes streaming/RSC public APIs and related utils/tests; Pro adds the implementations, docs, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant OSS as ReactOnRails (OSS)
participant PRO as ReactOnRailsPro (Pro)
participant CFG as Configuration
rect rgb(245,250,255)
Note over OSS,PRO: Asset/path detection and delegation
OSS->>OSS: need to check server bundle or RSC support
OSS->>PRO: react_on_rails_pro? / rsc_support_enabled? (delegation)
alt Pro present
PRO->>CFG: read rsc_* config values
CFG-->>PRO: rsc paths & enable flag
PRO-->>OSS: support=true, paths
else Pro absent
OSS-->>OSS: support=false, use OSS-only logic
end
end
rect rgb(235,255,235)
Note over PRO: Streaming/rendering (Pro-only)
Client->>PRO: request stream_react_component / rsc_payload_react_component
PRO->>PRO: run_stream_inside_fiber -> internal_*_stream...
PRO->>PRO: build initial chunk + hydration metadata + subsequent chunks (NDJSON for RSC payload)
PRO-->>Client: streamed response (HTML or NDJSON)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (11)
Comment |
Add tests for the three RSC utility methods moved to Pro gem: - rsc_support_enabled?: Tests enabled/disabled states - rsc_bundle_js_file_path: Tests delegation to bundle_js_file_path, caching behavior in development vs production - react_client_manifest_file_path: Tests packer integration with dev server running/not running, caching behavior - react_server_client_manifest_file_path: Tests delegation, caching behavior, error handling when manifest is nil Total: ~210 lines of tests ensuring all RSC utility methods work correctly in the Pro gem with proper caching and error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
AbanoubGhadban
force-pushed
the
move-pro-utils-and-configs-to-pro-gem
branch
from
d81bb5e to
a978ea6
Compare
Code Review: Move Pro-specific utilities and configurations to Pro gemThank you for this well-structured refactoring! This PR successfully moves React Server Components (RSC) and streaming-related configurations and utilities from the open-source gem to the Pro gem, improving separation of concerns. Here's my detailed review: ✅ Strengths
🔍 Potential Issues & Concerns1. Caching Logic Inconsistency (lib/react_on_rails_pro/utils.rb:22-27)The RSC bundle path caching doesn't reset instance variables properly: def self.rsc_bundle_js_file_path
return @rsc_bundle_path if @rsc_bundle_path && !Rails.env.development?
bundle_name = ReactOnRailsPro.configuration.rsc_bundle_js_file
@rsc_bundle_path = ReactOnRails::Utils.bundle_js_file_path(bundle_name)
endIssue: If Recommendation: Consider adding error handling or resetting cache on failure. 2. Missing
|
Fix the rsc_support_enabled? method to return true/false instead of nil when enable_rsc_support is false or nil. Issue: The expression `respond_to?(:enable_rsc_support) && enable_rsc_support` returns nil when enable_rsc_support is falsy, not an explicit false. Solution: Use double bang (!!) to coerce the result to a boolean: `respond_to?(:enable_rsc_support) && !!enable_rsc_support` This fixes 3 failing tests: - ReactOnRailsPro::Utils.rsc_support_enabled? when RSC support is enabled - ReactOnRailsPro::Utils.rsc_support_enabled? when RSC support is disabled - ReactOnRailsPro::Utils cache helpers .bundle_hash and .bundle_file_name 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Review: Move Pro-specific utilities and configurations to Pro gemOverall AssessmentThis is a well-structured refactoring that clearly separates Pro-only features from the open-source gem. The migration is comprehensive and follows good practices for breaking changes. The code quality is excellent with proper test coverage. ✅ Strengths1. Excellent Documentation
2. Strong Test Coverage
3. Clean Code Architecture
4. Good Error Handling
🔍 Issues Found1. Potential Bug in
|
Moved all streaming-related helper methods from open-source to Pro gem: Public methods moved to ReactOnRailsProHelper: - stream_react_component - Progressive SSR using React 18+ streaming - rsc_payload_react_component - RSC payload rendering with NDJSON format Private methods moved to ReactOnRailsProHelper: - run_stream_inside_fiber - Fiber management for streaming context - internal_stream_react_component - Internal HTML streaming implementation - internal_rsc_payload_react_component - Internal RSC payload implementation - build_react_component_result_for_server_streamed_content - Stream chunk transformation Changes: - Removed 6 streaming methods from lib/react_on_rails/helper.rb (~162 lines) - Added 6 streaming methods to react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (+176 lines) - Added RuboCop ModuleLength disable/enable for Pro helper module - No backward compatibility (clean removal per requirements) All methods remain accessible through Rails helper inclusion mechanism. Comprehensive test coverage exists in Pro helper spec. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Comprehensive Code Review - PR #1875I've completed a thorough review of this pull request. Overall, this is a well-structured refactoring that successfully moves Pro-specific features to the Pro gem. Here's my detailed feedback: ✅ Code Quality & ArchitectureStrengths:
Recommendations:
🐛 Potential IssuesHigh Priority:
def stream_react_component(component_name, options = {})
raise ReactOnRails::Error,
"stream_react_component is a React on Rails Pro feature. " \
"Please visit https://www.shakacode.com/react-on-rails-pro for licensing information."
endMedium Priority:
⚡ Performance ConsiderationsPositive:
Note:
🔒 Security ConcernsAll Clear:
🧪 Test Coverage & MigrationConcerns:
📋 Additional Recommendations
# In ReactOnRails::Configuration#initialize
if rsc_bundle_js_file.present?
warn "DEPRECATION WARNING: rsc_bundle_js_file should be configured in ReactOnRailsPro.configure, not ReactOnRails.configure"
end
📊 Summary
Overall Assessment: This is a well-executed refactoring that properly separates Pro features. The main concerns are:
Recommendation: Address the Doctor.rb references and error message improvements before merging. The incomplete phases should be finished according to the checklist in the PR description. Great work on this refactoring! The separation is clean and the migration path is well-documented. 🚀 |
Critical bugfix discovered during code review: - Lines 67-68 were still calling ReactOnRails.configuration instead of ReactOnRailsPro.configuration - Updated to use ReactOnRailsPro.configuration.react_client_manifest_file - Updated to use ReactOnRailsPro.configuration.react_server_client_manifest_file - Extracted config to local variable to fix RuboCop line length violation This ensures RSC manifest file configurations are read from the correct Pro configuration after they were moved from open-source to Pro gem in Phase 2. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed 3 test failures in utils_spec.rb that were caused by mocking non-existent ReactOnRails.configuration.rsc_bundle_js_file. Changes: - Updated tests to mock ReactOnRailsPro.configuration instead - Added proper Pro gem presence mocking with stub_const - Tests now correctly verify open-source bundle_js_file_path behavior when RSC bundles are present with Pro gem installed - Applied RuboCop auto-corrections for style compliance The open-source bundle_js_file_path function correctly handles RSC bundles by checking Pro configuration (lines 117-121 in utils.rb), so these tests ensure that behavior works as expected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Review for PR #1875: Move Pro-specific utilities and configurations to Pro gemI've completed a comprehensive review of this PR. Overall, this is a well-structured refactoring that successfully separates Pro features from the open-source gem. However, I've identified several critical issues that need to be addressed before merging. Critical Issues1. BREAKING BUG: Undefined Method References in Configuration (lib/react_on_rails/configuration.rb:319-321)Location: lib/react_on_rails/configuration.rb:313-323 This method references rsc_bundle_js_file, react_client_manifest_file, and react_server_client_manifest_file which were removed from the Configuration class. These are no longer attr_accessors on the open-source configuration. Impact: This will cause NoMethodError exceptions when webpack_generated_files is empty and the configuration is initialized. Recommended Fix: Update the ensure_webpack_generated_files_exists method to conditionally add Pro manifest files only when Pro is available, checking ReactOnRails::Utils.react_on_rails_pro? and accessing ReactOnRailsPro.configuration. 2. Test Compatibility Issues: Tests Still Reference Removed ConfigurationLocation: spec/react_on_rails/utils_spec.rb:62, 99-101 The tests still mock the removed configuration attributes. These tests will fail because these configuration options no longer exist on ReactOnRails.configuration. Recommended Fix: Update these tests to mock ReactOnRailsPro.configuration instead, with proper Pro gem availability checks. 3. Doctor Diagnostics Still Look for Old ConfigurationLocation: lib/react_on_rails/doctor.rb:679-681 The doctor is still looking for config.rsc_bundle_js_file in ReactOnRails.configure blocks, but this should now be in ReactOnRailsPro.configure blocks. Recommended Fix: Update the doctor to look for RSC configs in ReactOnRailsPro.configure blocks and warn if found in the old location with migration guidance. Code Quality & Best PracticesExcellent Architectural Decisions✅ Proper separation of concerns: RSC features are now clearly in the Pro gem where they belong ✅ Good use of feature detection: The code properly checks for Pro gem availability before accessing Pro configs ✅ Clean delegation: ReactOnRails::Utils.rsc_support_enabled? properly delegates to ReactOnRailsPro::Utils.rsc_support_enabled? ✅ Documentation is thorough: Both CHANGELOGs are well-documented with migration examples ✅ Pro utility methods were properly moved with good comments explaining the migration Performance & SecurityNo Performance Concerns - The changes don't introduce any performance regressions. The conditional Pro checks add minimal overhead and are appropriately cached. No Security Issues - The security model remains the same. Server bundle privacy enforcement (enforce_private_server_bundles) correctly stays in the open-source gem. Test Coverage Assessment
Recommendations:
Documentation QualityStrengths:
Suggestions:
Final RecommendationsMust Fix Before Merge:
Should Fix:
Nice to Have:
Overall AssessmentArchitecture: ⭐⭐⭐⭐⭐ Excellent separation of concerns Implementation: ⭐⭐⭐ Documentation: ⭐⭐⭐⭐ Very thorough Test Coverage: ⭐⭐⭐ Adequate but needs fixes Migration Path: ⭐⭐⭐⭐ Well-documented but could use runtime guidance Verdict: This is a valuable refactoring that improves the codebase architecture. However, the critical bug in ensure_webpack_generated_files_exists must be fixed before merging, along with the test updates. Once these issues are resolved, this will be ready to merge. Great work on the thorough documentation and systematic approach! The phased checklist in the PR description shows excellent planning. |
Added inline RuboCop disable comment for the Pro configuration double since we're mocking a module from the Pro gem that doesn't exist in the open-source test environment. Using a regular double is appropriate here. Changes: - Reverted complex instance_double setup - Added `# rubocop:disable RSpec/VerifiedDoubles` inline comment - RuboCop now passes with 0 offenses 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed test failures caused by attempting to mock configuration on an empty Module stub. The stubbed ReactOnRailsPro module now properly implements the configuration getter/setter methods. Changes: - Created proper mock Pro module with working configuration methods - Set configuration directly instead of using allow().to receive() - Matches the real Pro gem's module structure This fixes 4 test failures: - returns private path when it exists even if public path also exists - fallbacks to public path when private path does not exist and public path exists - returns configured path if both private and public paths do not exist - enforces private RSC bundles and never checks public directory 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
PR Review: Move Pro-specific utilities and configurations to Pro gemThank you for working on this important refactoring! This is a well-structured migration that improves code separation between the open-source and Pro versions. Here's my detailed review: 🐛 Critical Bug1. Undefined method references in
|
Code Review - PR #1875: Move Pro-specific utilities and configurations to Pro gemOverviewThis is a well-structured WIP PR that migrates RSC-specific configurations from the open-source gem to the Pro gem. The implementation demonstrates good architectural separation and maintains backward compatibility where appropriate. ✅ Strengths1. Excellent Documentation
2. Clean Architectural Separation
3. Robust Implementation
4. Good Test Coverage
🔍 Issues & RecommendationsCritical Issues1. Potential Bug in Location: The current implementation has a logical flaw: webpack_generated_files = @webpack_generated_files.map do |bundle_name|
# Check if this is a Pro RSC manifest file
if ReactOnRails::Utils.react_on_rails_pro?
pro_config = ReactOnRailsPro.configuration
if bundle_name == pro_config.react_client_manifest_file
ReactOnRailsPro::Utils.react_client_manifest_file_path
elsif bundle_name == pro_config.react_server_client_manifest_file
ReactOnRailsPro::Utils.react_server_client_manifest_file_path
else
ReactOnRails::Utils.bundle_js_file_path(bundle_name)
end
else
ReactOnRails::Utils.bundle_js_file_path(bundle_name)
end
endProblem: When Pro is NOT available, if someone has these manifest files in their Recommendation: Add a guard or warning when Pro-specific files are detected without Pro: webpack_generated_files = @webpack_generated_files.map do |bundle_name|
if ReactOnRails::Utils.react_on_rails_pro?
pro_config = ReactOnRailsPro.configuration
if bundle_name == pro_config.react_client_manifest_file
ReactOnRailsPro::Utils.react_client_manifest_file_path
elsif bundle_name == pro_config.react_server_client_manifest_file
ReactOnRailsPro::Utils.react_server_client_manifest_file_path
else
ReactOnRails::Utils.bundle_js_file_path(bundle_name)
end
else
# Warn if Pro-only files are specified without Pro gem
pro_manifest_files = ["react-client-manifest.json", "react-server-client-manifest.json"]
if pro_manifest_files.include?(bundle_name)
Rails.logger.warn("[ReactOnRails] #{bundle_name} requires React on Rails Pro")
end
ReactOnRails::Utils.bundle_js_file_path(bundle_name)
end
endMedium Priority Issues2. Missing Trailing Newlines Check According to
3. Incomplete Implementation (Expected for WIP) The PR description shows only Phase 1 & 2 are complete. Before merging:
4. Test Coverage Gap While configuration tests are good, consider adding integration tests for:
📝 Minor Suggestions1. Code Style - Consistent Conditionals In Current: is_bundle_running_on_server = bundle_name == ReactOnRails.configuration.server_bundle_js_file
# Check Pro RSC bundle if Pro is available
if ReactOnRails::Utils.react_on_rails_pro? && defined?(ReactOnRailsPro)
is_bundle_running_on_server ||= (bundle_name == ReactOnRailsPro.configuration.rsc_bundle_js_file)
endSuggestion: The 2. Documentation Enhancement In # React Server Components and Streaming SSR are Pro features.
# These configurations were moved to Pro in v16.2.0 for better separation.
# See PR #1874 for details.3. Error Messages When RSC configs are accessed on open-source gem (if applicable), ensure error messages guide users to Pro documentation. 🔒 Security Review✅ No security concerns identified:
⚡ Performance Considerations✅ Performance looks good:
📋 Pre-Merge ChecklistBefore marking this PR as ready for review:
🎯 Overall AssessmentStatus: Good foundation, but incomplete (as expected for WIP) Code Quality: ⭐⭐⭐⭐ (4/5) - Well-structured with one potential bug to fix Documentation: ⭐⭐⭐⭐⭐ (5/5) - Excellent migration guides and clear explanations Testing: ⭐⭐⭐ (3/5) - Config tests good, needs integration tests Recommendation:
Great work on maintaining backward compatibility and clear separation of concerns! 👍 |
Code Review - PR #1875: Move Pro-specific utilities and configurations to Pro gemOverall, this is a well-executed refactoring that clearly separates Pro features from the open-source gem. The migration path is well-documented, and the architecture is sound. However, I've identified one critical bug that must be fixed before merging, along with several recommendations for improvement. 🔴 Critical Issue - Must FixBug in
|
PR Review: Move Pro-specific utilities and configurations to Pro gemI've reviewed PR #1875 and have the following feedback organized by category: ✅ Code Quality & ArchitectureExcellent separation of concerns:
Code organization:
Best practices followed:
🔒 Security ConsiderationsLicense validation:
Access control:
|
AbanoubGhadban
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/react_on_rails/configuration.rb (1)
313-323: Critical: NoMethodError when calling removed RSC configuration methods.The
ensure_webpack_generated_files_existsmethod referencesrsc_bundle_js_file,react_client_manifest_file, andreact_server_client_manifest_file(lines 319-322), but these attributes were removed from theConfigurationclass (line 70-72). This will raise aNoMethodErrorwhen this method is called during configuration setup.Since these are now Pro-only features, you need to conditionally check for their presence. Consider this fix:
def ensure_webpack_generated_files_exists return unless webpack_generated_files.empty? self.webpack_generated_files = [ "manifest.json", server_bundle_js_file, - rsc_bundle_js_file, - react_client_manifest_file, - react_server_client_manifest_file + (ReactOnRails::Utils.react_on_rails_pro? && defined?(ReactOnRailsPro) ? ReactOnRailsPro.configuration.rsc_bundle_js_file : nil), + (ReactOnRails::Utils.react_on_rails_pro? && defined?(ReactOnRailsPro) ? ReactOnRailsPro.configuration.react_client_manifest_file : nil), + (ReactOnRails::Utils.react_on_rails_pro? && defined?(ReactOnRailsPro) ? ReactOnRailsPro.configuration.react_server_client_manifest_file : nil) ].compact_blank endreact_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
120-129: Update error message to reference the actual minimum version instead of retracted 15.0.0The error message references react_on_rails version "15.0.0 or higher", but that version was retracted. Version 15.0.0 has been retracted, and users should upgrade directly to v16.0.0. Update the message to say "16.0.0 or higher" to reflect the current minimum released version with RSC support.
🧹 Nitpick comments (15)
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb (1)
46-46: Consider making test logging conditional for cleaner output.While logging to stdout is helpful for debugging, it can make test output verbose. Consider making this conditional based on an environment variable to keep output clean by default:
Rails.logger = Logger.new(ENV['VERBOSE_TESTS'] ? $stdout : IO::NULL)Or adjust the log level to only show warnings and errors:
Rails.logger = Logger.new($stdout) Rails.logger.level = ENV['VERBOSE_TESTS'] ? Logger::DEBUG : Logger::WARNThis allows developers to opt into verbose logging when needed without cluttering the default test output.
react_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rb (1)
66-76: Nice code cleanup.Introducing the local
configvariable improves readability without changing functionality. This is a good refactoring practice.lib/react_on_rails/test_helper/webpack_assets_status_checker.rb (1)
53-62: Ensure returned paths are filesystem paths for stale checksstale_generated_files calls File.exist?/File.mtime on these values. ReactOnRailsPro::Utils.react_client_manifest_file_path can return a dev-server URL, which will cause false “stale” results. Prefer always returning a filesystem path here.
Suggested change:
- Resolve both Pro manifest names via ReactOnRails::Utils.bundle_js_file_path to guarantee a local path (manifest lookup → file path fallback), or guard against URLs before returning.
Example diff:
- if bundle_name == pro_config.react_client_manifest_file - ReactOnRailsPro::Utils.react_client_manifest_file_path - elsif bundle_name == pro_config.react_server_client_manifest_file - ReactOnRailsPro::Utils.react_server_client_manifest_file_path + if bundle_name == pro_config.react_client_manifest_file + ReactOnRails::Utils.bundle_js_file_path(bundle_name) + elsif bundle_name == pro_config.react_server_client_manifest_file + ReactOnRails::Utils.bundle_js_file_path(bundle_name)Can you confirm whether asset_uri_from_packer ever returns an http(s) URL for the client manifest in your test env? If yes, the above change (or equivalent URL-to-path handling) is required for correct staleness detection.
docs/api-reference/view-helpers-api.md (1)
120-159: Pro-only helpers section looks correct; minor copy nitConsider tightening “Used in conjunction with RSC support” → “Used with RSC support” for brevity.
spec/react_on_rails/utils_spec.rb (1)
247-267: Good isolation of Pro behavior in OSS specThe local ReactOnRailsPro stub plus forcing react_on_rails_pro?=true keeps tests hermetic. Optionally, also stub ReactOnRailsPro::Utils if future code paths consult it.
spec/dummy/spec/packs_generator_spec.rb (2)
237-244: Simplify and harden the Utils stub (avoid double‑stubbing and Class override).Use a Module stub and a single allow; reduces risk of constant shape mismatches and duplicate stubs.
- stub_const("ReactOnRailsPro::Utils", Class.new do - def self.rsc_support_enabled? - true - end - end) - allow(ReactOnRailsPro::Utils).to receive_messages( - rsc_support_enabled?: true - ) + stub_const("ReactOnRailsPro::Utils", Module.new) + allow(ReactOnRailsPro::Utils).to receive(:rsc_support_enabled?).and_return(true)
315-319: Clarify context nesting.“when RSC support is disabled” is nested under “when RSC support is enabled”, which is confusing. Consider renaming the outer context to “when ReactOnRails Pro is available” and toggle RSC per inner contexts, or lift the disabled case to a sibling context.
react_on_rails_pro/docs/configuration.md (1)
125-164: Add a short note on path resolution and dev‑server behavior for RSC files.Clarify that the server‑side manifest isn’t served by the dev server and where each path is resolved from.
################################################################################ # REACT SERVER COMPONENTS (RSC) CONFIGURATION ################################################################################ + # Note: + # - config.rsc_bundle_js_file and config.react_server_client_manifest_file are treated as + # server bundles/outputs and are resolved from private (non‑public) output paths first. + # They are not served by the dev server. + # - config.react_client_manifest_file may resolve to an HTTP URL when the dev server is running. + # - If migrating from OSS, move any prior RSC settings to this Pro initializer.react_on_rails_pro/lib/react_on_rails_pro/utils.rb (1)
22-52: Avoid memoization in test to reduce flakiness.Align cache guards with other helpers by bypassing cache in both development and test.
- return @rsc_bundle_path if @rsc_bundle_path && !Rails.env.development? + return @rsc_bundle_path if @rsc_bundle_path && !(Rails.env.development? || Rails.env.test?) @@ - return @react_client_manifest_path if @react_client_manifest_path && !Rails.env.development? + return @react_client_manifest_path if @react_client_manifest_path && !(Rails.env.development? || Rails.env.test?) @@ - return @react_server_manifest_path if @react_server_manifest_path && !Rails.env.development? + return @react_server_manifest_path if @react_server_manifest_path && !(Rails.env.development? || Rails.env.test?)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
110-118: Validate required RSC paths when RSC is enabled.Fail fast if any RSC path is blank to avoid late resolution errors.
def setup_config_values configure_default_url_if_not_provided validate_url validate_remote_bundle_cache_adapter setup_renderer_password setup_assets_to_copy setup_execjs_profiler_if_needed check_react_on_rails_support_for_rsc + validate_rsc_required_paths end @@ + def validate_rsc_required_paths + return unless enable_rsc_support + + %i[rsc_bundle_js_file react_client_manifest_file react_server_client_manifest_file].each do |attr| + value = public_send(attr) + if value.blank? + raise ReactOnRailsPro::Error, "config.#{attr} must be set when enable_rsc_support is true" + end + end + endAlso applies to: 125-129
react_on_rails_pro/spec/react_on_rails_pro/utils_spec.rb (3)
251-296: Rails.env stubbing: prefer replacing the inquirer over stubbing predicate.
allow(Rails.env).to receive(:development?)works but is brittle. Safer and RuboCop‑friendly: return a fresh StringInquirer and avoid predicate stubs.Example:
- allow(Rails.env).to receive(:development?).and_return(false) + allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production"))And for the dev case:
- allow(Rails.env).to receive(:development?).and_return(true) + allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("development"))
309-348: Avoid receive_message_chain; stub dev_server explicitly.
receive_message_chain("dev_server.running?")is discouraged by rubocop‑rspec and is brittle. Stubdev_serverto a double withrunning?:- before do - allow(::Shakapacker).to receive_message_chain("dev_server.running?") - .and_return(false) - end + before do + dev_server = instance_double(::Shakapacker::DevServer, running?: false) + allow(::Shakapacker).to receive(:dev_server).and_return(dev_server) + endSimilarly, where you use
manifest.lookup!, prefer stubbingmanifestto a double responding tolookup!to avoid message chains.
464-475: Clarify “singular day” wording.Test description says “singular day” but expectation keeps “day(s)”. Either change description or (optionally) pluralize in implementation/expectation.
Example description tweak:
- it "returns attribution comment with singular day" do + it "returns attribution comment with day count wording" doreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (2)
184-191: Docs/code mismatch: helper does not raise if RSC disabled.Comment says “raises if RSC support is not enabled,” but the method doesn’t check. Per learnings, validation happens elsewhere (pack generation/registration), so the helper should not add another check. Update the comment to reflect that failure occurs upstream, not here.
Based on learnings
- # @raise [ReactOnRailsPro::Error] if RSC support is not enabled in configuration + # Note: If RSC is disabled, an error will arise during pack generation or registration, not here.
257-273: Cache write‑through buffers every chunk; consider bounding.
buffered_chunksgrows with stream length; for very chatty streams this can grow large beforeRails.cache.write. Consider:
- Cap number of buffered chunks or
- Stream to a temp file and read back for write, or
- Enforce an
:expires_indefault forcache_options.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.gitignore(1 hunks)CHANGELOG.md(1 hunks)docs/api-reference/configuration.md(1 hunks)docs/api-reference/view-helpers-api.md(1 hunks)lib/react_on_rails/configuration.rb(1 hunks)lib/react_on_rails/helper.rb(0 hunks)lib/react_on_rails/packer_utils.rb(1 hunks)lib/react_on_rails/test_helper/webpack_assets_status_checker.rb(1 hunks)lib/react_on_rails/utils.rb(2 hunks)react_on_rails_pro/CHANGELOG.md(1 hunks)react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb(3 hunks)react_on_rails_pro/docs/configuration.md(1 hunks)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb(5 hunks)react_on_rails_pro/lib/react_on_rails_pro/request.rb(3 hunks)react_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rb(1 hunks)react_on_rails_pro/lib/react_on_rails_pro/utils.rb(3 hunks)react_on_rails_pro/spec/dummy/config/initializers/react_on_rails.rb(0 hunks)react_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb(1 hunks)react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb(1 hunks)react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb(1 hunks)react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb(1 hunks)react_on_rails_pro/spec/react_on_rails_pro/utils_spec.rb(3 hunks)spec/dummy/spec/packs_generator_spec.rb(2 hunks)spec/react_on_rails/configuration_spec.rb(0 hunks)spec/react_on_rails/utils_spec.rb(2 hunks)
💤 Files with no reviewable changes (3)
- react_on_rails_pro/spec/dummy/config/initializers/react_on_rails.rb
- lib/react_on_rails/helper.rb
- spec/react_on_rails/configuration_spec.rb
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/lib/react_on_rails_pro/request.rbspec/dummy/spec/packs_generator_spec.rblib/react_on_rails/utils.rbspec/react_on_rails/utils_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rbreact_on_rails_pro/lib/react_on_rails_pro/utils.rbreact_on_rails_pro/spec/react_on_rails_pro/request_spec.rblib/react_on_rails/test_helper/webpack_assets_status_checker.rbreact_on_rails_pro/spec/react_on_rails_pro/utils_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rblib/react_on_rails/configuration.rblib/react_on_rails/packer_utils.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rb
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
react_on_rails_pro/CHANGELOG.mdreact_on_rails_pro/docs/configuration.mddocs/api-reference/view-helpers-api.mdCHANGELOG.mddocs/api-reference/configuration.md
🧠 Learnings (7)
📓 Common learnings
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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
PR: shakacode/react_on_rails#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.
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#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:
react_on_rails_pro/lib/react_on_rails_pro/request.rbreact_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rb
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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:
react_on_rails_pro/lib/react_on_rails_pro/request.rbreact_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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:
react_on_rails_pro/lib/react_on_rails_pro/request.rbspec/dummy/spec/packs_generator_spec.rbreact_on_rails_pro/CHANGELOG.mdreact_on_rails_pro/docs/configuration.mdlib/react_on_rails/utils.rbspec/react_on_rails/utils_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rbreact_on_rails_pro/lib/react_on_rails_pro/utils.rblib/react_on_rails/test_helper/webpack_assets_status_checker.rbreact_on_rails_pro/spec/react_on_rails_pro/utils_spec.rbdocs/api-reference/view-helpers-api.mdCHANGELOG.mdreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbdocs/api-reference/configuration.mdlib/react_on_rails/packer_utils.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro/request.rbspec/dummy/spec/packs_generator_spec.rblib/react_on_rails/utils.rbreact_on_rails_pro/lib/react_on_rails_pro/utils.rbdocs/api-reference/view-helpers-api.md
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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:
react_on_rails_pro/CHANGELOG.mdreact_on_rails_pro/lib/react_on_rails_pro/utils.rbdocs/api-reference/view-helpers-api.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#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:
react_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rblib/react_on_rails/test_helper/webpack_assets_status_checker.rbdocs/api-reference/configuration.md
🧬 Code graph analysis (13)
react_on_rails_pro/lib/react_on_rails_pro/request.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/utils.rb (3)
rsc_bundle_js_file_path(22-27)react_client_manifest_file_path(29-34)react_server_client_manifest_file_path(38-48)
spec/dummy/spec/packs_generator_spec.rb (1)
lib/react_on_rails/utils.rb (1)
rsc_support_enabled?(238-242)
lib/react_on_rails/utils.rb (3)
lib/react_on_rails/configuration.rb (1)
configuration(14-57)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)react_on_rails_pro/lib/react_on_rails_pro/utils.rb (1)
rsc_support_enabled?(50-52)
spec/react_on_rails/utils_spec.rb (1)
lib/react_on_rails/configuration.rb (1)
configuration(14-57)
react_on_rails_pro/lib/react_on_rails_pro/server_rendering_js_code.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (2)
lib/react_on_rails/helper.rb (5)
internal_react_component(480-503)create_render_options(363-371)server_rendered_react_component(535-594)build_react_component_result_for_server_rendered_string(377-402)compose_react_component_html_with_spec_and_console(438-445)lib/react_on_rails/react_component/render_options.rb (1)
replay_console(86-88)
react_on_rails_pro/lib/react_on_rails_pro/utils.rb (3)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)lib/react_on_rails/utils.rb (2)
bundle_js_file_path(74-86)rsc_support_enabled?(238-242)lib/react_on_rails/packer_utils.rb (1)
asset_uri_from_packer(80-86)
react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb (2)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)react_on_rails_pro/lib/react_on_rails_pro/server_rendering_pool/node_rendering_pool.rb (2)
renderer_bundle_file_name(32-34)rsc_renderer_bundle_file_name(36-38)
lib/react_on_rails/test_helper/webpack_assets_status_checker.rb (4)
lib/react_on_rails/utils.rb (2)
react_on_rails_pro?(215-223)bundle_js_file_path(74-86)lib/react_on_rails/configuration.rb (1)
configuration(14-57)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)react_on_rails_pro/lib/react_on_rails_pro/utils.rb (2)
react_client_manifest_file_path(29-34)react_server_client_manifest_file_path(38-48)
react_on_rails_pro/spec/react_on_rails_pro/utils_spec.rb (3)
react_on_rails_pro/lib/react_on_rails_pro/utils.rb (4)
rsc_bundle_js_file_path(22-27)rsc_support_enabled?(50-52)react_client_manifest_file_path(29-34)react_server_client_manifest_file_path(38-48)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)lib/react_on_rails/utils.rb (1)
rsc_support_enabled?(238-242)
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (2)
configure(4-7)configuration(9-37)
lib/react_on_rails/packer_utils.rb (3)
lib/react_on_rails/configuration.rb (1)
configuration(14-57)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configuration(9-37)lib/react_on_rails/utils.rb (1)
react_on_rails_pro?(215-223)
react_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb (1)
react_on_rails_pro/spec/dummy/client/node-renderer.js (1)
config(30-66)
🪛 LanguageTool
docs/api-reference/view-helpers-api.md
[style] ~151-~151: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...ormat for client-side consumption. Used in conjunction with RSC support to enable: - Reduced JavaS...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
🔇 Additional comments (17)
.gitignore (1)
78-78: Clarify the purpose of the.fuse_hidden*pattern addition.This change adds a gitignore pattern for FUSE temporary files, which appears unrelated to the PR's objectives of moving Pro-specific utilities and configurations.
Verify whether:
- This pattern is necessary for the PRs functionality, or
- This is an accidental commit of developer environment configuration that should be removed or moved to a personal
.git/info/excludefile instead.react_on_rails_pro/lib/react_on_rails_pro/request.rb (1)
52-52: LGTM! Namespace migration is correct.The changes correctly migrate RSC-related path resolution from
ReactOnRails::UtilstoReactOnRailsPro::Utils, aligning with the broader migration of RSC functionality to the Pro gem.Also applies to: 157-157, 181-182
lib/react_on_rails/packer_utils.rb (1)
58-63: LGTM! Conditional Pro support is well implemented.The refactored logic correctly adds conditional RSC bundle support when Pro is available, while maintaining backward compatibility for open-source users. The defensive checks (
react_on_rails_pro?anddefined?(ReactOnRailsPro)) ensure no errors occur in non-Pro environments.docs/api-reference/configuration.md (1)
107-122: LGTM! Documentation correctly reflects RSC migration to Pro.The documentation update appropriately redirects users to the Pro documentation for RSC-related configuration, maintaining clear boundaries between open-source and Pro features.
CHANGELOG.md (1)
76-108: LGTM! Clear migration documentation.The CHANGELOG entry provides excellent migration guidance with before/after code examples and links to Pro documentation. This will help users upgrade smoothly.
react_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb (1)
10-11: LGTM! Demonstrates correct Pro configuration pattern.This example correctly shows how to configure the RSC bundle in the Pro initializer, providing a good reference for users migrating their configuration.
react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb (1)
23-24: LGTM! Test consolidation and Pro namespace migration.The changes improve test readability by consolidating multiple stub calls into single
receive_messagescalls, and correctly update to useReactOnRailsPro::Utilsfor RSC bundle path resolution.Also applies to: 32-34, 37-37
react_on_rails_pro/CHANGELOG.md (1)
22-33: Docs read well; migration scoped and clearThe relocation of RSC configs and streaming helpers to Pro is explained succinctly with a migration pointer. LGTM.
Also applies to: 36-36
spec/react_on_rails/utils_spec.rb (1)
659-659: Test relocation note is helpfulAcknowledging the move of RSC utils tests to Pro keeps the OSS suite lean. LGTM.
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb (1)
202-260: RSC configuration tests are correct and well-aligned with constantsThe DEFAULT_* constants in configuration.rb (lines 59-61) match exactly with the test expectations:
- "rsc-bundle.js" ✓
- "react-client-manifest.json" ✓
- "react-server-client-manifest.json" ✓
Tests comprehensively cover defaults, explicit assignments, nil values, and combined setup. No brittle expectations.
lib/react_on_rails/utils.rb (1)
236-242: Delegating RSC support detection to Pro looks good.react_on_rails_pro/spec/react_on_rails_pro/utils_spec.rb (2)
229-249: LGTM: rsc_support_enabled? mirrors configuration state.Clean, direct mapping to
ReactOnRailsPro.configuration.enable_rsc_support.
392-401: LGTM: manifest path resolution, caching behavior, and nil‑guard error.
- Correct delegation to
ReactOnRails::Utils.bundle_js_file_path.- Caching toggled by
Rails.env.development?is covered.- Error case when filename is nil is asserted.
Also applies to: 402-425, 427-437
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (4)
126-134: Default immediate_hydration: confirm client-side Pro gating remains.Setting
immediate_hydration: trueby default is fine as long as the client still gates hydration behind Pro (viarailsContext.rorProand related checks).Based on learnings: force-load/hydration gating is enforced client-side for Pro. Please confirm the client path still checks
railsContext.rorProfor immediate hydration.
294-313: Fiber wrapper error path and gating note.Good: clear error if
stream_view_containing_react_componentswas not invoked. If license/Pro gating used to live here, ensure it’s intentionally moved upstream so this method focuses solely on fiber plumbing.Confirm that license/Pro checks and diagnostics are handled earlier in the flow (e.g., when starting the streaming view or in the server renderer), per the migration plan.
325-332: NDJSON chunking looks correct.Transforming stream chunks to
JSON + "\n"for RSC payloads is straightforward and safe for streaming.
334-358: HTML streaming composition is sound; console replay gated.
- First chunk composes full markup/spec tag; subsequent chunks append HTML and optional console script.
- Uses existing helpers, preserves
replay_consolesemantics.
…packAssetsStatusChecker
|
|
||
| --- | ||
|
|
||
| ### cached_react_component and cached_react_component_hash |
There was a problem hiding this comment.
Moved to Pro-Only View Helpers section added below
Pull Request Review: Move Pro-specific utilities and configurations to Pro gemOverall AssessmentThis is a well-structured refactoring that effectively separates Pro-specific features from the open-source codebase. The PR demonstrates good architectural planning with clear phases and comprehensive documentation. The code quality is high, and the migration path is well-documented. Recommendation: Approve with minor suggestions Strengths1. Excellent Architecture & Separation of Concerns
2. Strong Documentation & Migration Guidance
3. Backward Compatibility & Safety
4. Code Quality
Issues & SuggestionsCritical IssuesNone identified. The PR is safe to merge. Moderate Suggestions
Minor Suggestions
Security & PerformanceSecurity: No concerns. Server bundle detection properly handles Pro paths. Performance: Good characteristics with proper memoization and caching. Testing RecommendationsBefore merging complete PR series:
Compliance with CLAUDE.mdAdheres to repository guidelines. Before merging: run bundle exec rubocop and fix all violations. ConclusionExcellent refactoring work. The migration path is clear, documentation is comprehensive, and code quality is high. Suggestions above are minor improvements that enhance robustness but don't block merging Phase 2. Next Steps:
Great work! Reviewed by: Claude Code |
Code Review: PR #1875 - Move Pro-specific utilities and configurations to Pro gemI've completed a comprehensive review of this PR. Overall, this is excellent architectural work that significantly improves code separation and maintainability. ✅ Strengths1. Excellent Architectural Separation
2. Comprehensive Documentation
3. Strong Test Coverage
4. Backward Compatibility
|
1 participant
Summary
This PR migrates Pro-specific configurations and utilities from the open-source React on Rails gem to the React on Rails Pro gem, improving code separation and clarifying feature boundaries.
Closes #1874
Implementation Progress
Phase Checklist
Phase 1: Update Documentation & CHANGELOG
Phase 2: Move RSC Configuration Options to Pro Gem
Phase 3: Move RSC Utility Methods to Pro Gem
rsc_support_enabled?to Prorsc_bundle_js_file_pathto Proreact_server_client_manifest_file_pathto ProPhase 4: Move Streaming Helper Methods to Pro Gem
stream_react_componentto Prorsc_payload_react_componentto ProPhase 5: Move Internal Streaming Methods to Pro Gem
internal_stream_react_componentto Prointernal_rsc_payload_react_componentto Prorun_stream_inside_fiberto ProPhase 6: Move Streaming Render Modes & Options to Pro Gem
:html_streamingrender mode to Pro:rsc_payload_streamingrender mode to ProPhase 7: Move RSC Pack Generation Logic to Pro Gem
client_entrypoint?to ProPhase 8: Update Doctor Diagnostics
Phase 9: Update Generator Templates
Phase 10: Update Both Dummy Apps
Phase 11: Final Cleanup & Verification
What Stays in Open-Source
immediate_hydrationconfig (with Pro marketing badge)server_bundle_output_path(used by core SSR)enforce_private_server_bundles(security for all)component_registry_timeout(used by core)What Moves to Pro
stream_react_component,rsc_payload_react_component):html_streaming,:rsc_payload_streaming)Breaking Changes
For Pro Users
Pro users must update their configuration:
Before:
After:
For Open-Source Users
No breaking changes - open-source users never used these Pro-only features.
Benefits
This change is
Summary by CodeRabbit
Breaking Changes
Documentation
Chores
Tests