Skip to content

Conversation

@AbanoubGhadban
Copy link
Collaborator

@AbanoubGhadban AbanoubGhadban commented Nov 27, 2025

Summary

Adds concurrent React component rendering support for React on Rails Pro. Closes #2138.

  • async_react_component: Returns AsyncValue immediately, renders components concurrently
  • cached_async_react_component: Async rendering with fragment caching support (same options as cached_react_component)
  • AsyncRendering concern: Controller mixin with enable_async_react_rendering class method

Motivation

Currently, multiple react_component calls in a Rails view run sequentially. Each call makes an HTTP request to the Node renderer, causing delays proportional to the number of components. This PR enables concurrent execution using the async gem (already used in the codebase for streaming).

Usage

Controller Setup

class ProductsController < ApplicationController
  include ReactOnRailsPro::AsyncRendering
  enable_async_react_rendering only: [:show, :index]
end

View Usage

<%# Start async renders (non-blocking) %>
<% header = async_react_component("Header", props: @header_props) %>
<% sidebar = async_react_component("Sidebar", props: @sidebar_props) %>
<% content = async_react_component("Content", props: @content_props) %>

<%# Access values (blocks until ready) %>
<%= header.value %>
<%= sidebar.value %>
<%= content.value %>

With Caching

<% card = cached_async_react_component("ProductCard", cache_key: @product) { @product.to_props } %>
<%= card.value %>

Implementation Details

  • Uses Async::Barrier for coordinating concurrent tasks within a request
  • Cache lookup is synchronous - cache hits return ImmediateAsyncValue (no async overhead)
  • Cache misses trigger async render and write to cache on completion
  • Errors propagate naturally via task.wait
  • Supports all cached_react_component options: :cache_key, :cache_options, :if, :unless

Test plan

  • Unit tests for AsyncValue class
  • Unit tests for ImmediateAsyncValue class
  • Integration tests for async_react_component
  • Integration tests for cached_async_react_component
  • Concurrent execution verification test
  • Error propagation tests
  • Cache hit/miss behavior tests
  • Conditional caching (:if/:unless) tests

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Adds asynchronous server-side rendering for React components with barrier-backed concurrency and optional caching, returning cached results synchronously when available and queuing async renders on cache miss.
    • Controller opt-in to enable async rendering for selected actions.
    • Demo page and sample delayed components illustrating concurrent renders and timing.
  • Tests

    • Extensive tests covering async rendering, caching behavior, error propagation, and async value semantics.

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

…2138)

Add concurrent React component rendering support for React on Rails Pro.
Multiple async_react_component calls now execute their HTTP rendering
requests in parallel instead of sequentially.

New features:
- async_react_component: Returns AsyncValue immediately, renders concurrently
- cached_async_react_component: Async rendering with fragment caching support
- AsyncRendering concern: Controller mixin with enable_async_react_rendering
- Supports all cached_react_component options (:cache_key, :cache_options, :if, :unless)

Usage:
```ruby
class ProductsController < ApplicationController
  include ReactOnRailsPro::AsyncRendering
  enable_async_react_rendering only: [:show]
end
```

```erb
<% header = async_react_component("Header", props: @header_props) %>
<% sidebar = async_react_component("Sidebar", props: @sidebar_props) %>
<%= header.value %>
<%= sidebar.value %>
```

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025

Walkthrough

Adds barrier-backed asynchronous server rendering: new helper APIs for async and cached async component rendering, AsyncValue/ImmediateAsyncValue wrappers, an AsyncRendering controller concern to establish an Async::Barrier context, and supporting tests, demo components, and routes.

Changes

Cohort / File(s) Summary
Helpers (async rendering API)
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
Adds async_react_component, cached_async_react_component, fetch_async_react_component, render_async_react_component_uncached, render_async_react_component_with_cache, and prepare_async_render_options to enqueue/manage async server renders with optional caching and error checks.
Async runtime & wrappers
react_on_rails_pro/lib/react_on_rails_pro/async_value.rb, react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb, react_on_rails_pro/lib/react_on_rails_pro.rb
Introduces AsyncValue (wraps background render task; blocks on .value, propagates exceptions, exposes resolved? and html_safe), ImmediateAsyncValue (sync wrapper for cache hits), and adds require statements to load async components at startup.
AsyncRendering concern & sigs
react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb, react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
Adds AsyncRendering concern with enable_async_react_rendering class method and private wrap_in_async_react_context/check_for_unresolved_async_components to manage an Async::Barrier-backed context.
Type signatures
react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs, react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
Adds RBS signatures for AsyncValue and ImmediateAsyncValue.
Specs & dummy app demo
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb, react_on_rails_pro/spec/react_on_rails_pro/async_value_spec.rb, react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb, react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb, react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb, react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.*, react_on_rails_pro/spec/dummy/config/routes.rb
Adds tests covering async helpers, AsyncValue/ImmediateAsyncValue, demo controller/action/view, delayed client/server components, and route for exercising concurrent rendering and caching semantics.
Changelog
react_on_rails_pro/CHANGELOG.md
Documents the added async rendering helpers and requirement to enable AsyncRendering.

Sequence Diagram(s)

sequenceDiagram
    participant Controller
    participant Helper
    participant Cache as Rails.cache
    participant Barrier as Async::Barrier
    participant Task as Background Task
    participant Component as Node/Server Component

    rect rgb(200,240,220)
    Note over Controller,Cache: Cache hit path (sync)
    Controller->>Helper: cached_async_react_component(name, opts)
    Helper->>Cache: read(cache_key)
    Cache-->>Helper: cached_html
    Helper-->>Controller: ImmediateAsyncValue(cached_html)
    end

    rect rgb(220,220,255)
    Note over Controller,Barrier: Cache miss / uncached async flow
    Controller->>Helper: async_react_component(...) / cached_async_react_component(...)
    Helper->>Cache: read(cache_key)
    Cache-->>Helper: miss
    Helper->>Barrier: barrier.async { schedule render task }
    Barrier->>Task: spawn render task
    Helper-->>Controller: AsyncValue(task)
    Task->>Component: invoke server render (async)
    Component-->>Task: rendered_html (or error)
    Task->>Cache: write(cache_key, rendered_html) -- optional
    Task-->>AsyncValue: complete with rendered_html
    Controller->>AsyncValue: value() (blocks until ready)
    AsyncValue-->>Controller: rendered_html (html_safe)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay extra attention to:
    • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb — cache-key handling, if/unless conditional caching, block vs props behavior, and error cases.
    • lib/react_on_rails_pro/concerns/async_rendering.rb — correct setup/teardown of Async::Barrier and logging/cleanup of unresolved tasks.
    • lib/react_on_rails_pro/async_value.rb / immediate_async_value.rb — blocking semantics, exception propagation, and html_safe handling.
    • Specs/demo — timing assumptions, potential flakiness from delays and concurrency.

Possibly related PRs

Suggested reviewers

  • ihabadham
  • Judahmeek
  • alexeyr-ci2

Poem

🐰 I queued my renders, hopped into a line,

Barriers held steady while each task took time,
Cache gave a wink or the tasks ran and spun,
Ten sleepy components woke up as one,
Hooray — concurrent renders, what fun! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.13% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change, describing the addition of two key async helpers for React component rendering.
Linked Issues check ✅ Passed All requirements from issue #2138 are met: async_react_component returns AsyncValue for concurrent rendering [#2138], cached_async_react_component supports fragment caching with cache_key/cache_options [#2138], AsyncRendering concern enables opt-in [#2138], async gem infrastructure with Async::Barrier coordinates tasks [#2138], cache hits return ImmediateAsyncValue avoiding async overhead [#2138], and comprehensive tests validate concurrency, caching, and error handling [#2138].
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #2138: helper methods for async rendering, AsyncValue/ImmediateAsyncValue wrappers, AsyncRendering concern, comprehensive test coverage, type signatures, demo components, and changelog updates all serve the concurrent rendering feature.
✨ 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 feature/async-react-component-2138

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

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

@claude
Copy link

claude bot commented Nov 27, 2025

Code Review - PR #2139: Async React Component Rendering

Great work on implementing concurrent React component rendering! This is a valuable performance enhancement. Here's my detailed review:

Strengths

  1. Excellent Test Coverage - Comprehensive unit and integration tests covering:

    • AsyncValue and ImmediateAsyncValue behavior
    • Concurrent execution verification
    • Error propagation
    • Cache hit/miss scenarios
    • Conditional caching with :if/:unless options
  2. Clean API Design - The helper methods follow existing patterns and integrate well:

    • async_react_component mirrors react_component
    • cached_async_react_component mirrors cached_react_component
    • Clear error messages guide developers to proper setup
  3. Good Documentation - Clear inline comments, method-level documentation, and examples in the PR description

  4. Smart Optimization - Cache hits return ImmediateAsyncValue avoiding async overhead when not needed

🔍 Code Quality Observations

1. AsyncValue Error Handling ✅ Good

The .value method properly propagates exceptions via task.wait:

def value
  @task.wait
end

This is correct - exceptions naturally bubble up from the async task.

2. Barrier Cleanup in AsyncRendering Concern ✅ Good

def wrap_in_async_react_context
  Sync do
    @react_on_rails_async_barrier = Async::Barrier.new
    yield
  ensure
    @react_on_rails_async_barrier = nil
  end
end

Proper cleanup ensures barrier is reset after request.

3. Cache Key Construction - Consider Enhancement

In fetch_async_react_component (line 372):

cache_key = ReactOnRailsPro::Cache.react_component_cache_key(component_name, raw_options)

Question: Does this cache key include the async/non-async distinction? If a user calls both cached_react_component and cached_async_react_component with identical props, should they share the same cache entry?

Recommendation: This is likely correct (they should share cache), but worth verifying that the cache semantics match the existing cached_react_component behavior exactly.

⚠️ Potential Issues

1. Missing RBS Type Signatures (Per CLAUDE.md requirements)

The project uses RBS for type checking. The new Ruby files should have corresponding .rbs signatures:

  • sig/react_on_rails_pro/async_value.rbs
  • sig/react_on_rails_pro/immediate_async_value.rbs
  • sig/react_on_rails_pro/concerns/async_rendering.rbs

From CLAUDE.md:

When creating new Ruby files in lib/react_on_rails/:

  1. Create RBS signature: Add sig/react_on_rails/filename.rbs
  2. Validate: Run bundle exec rake rbs:validate

Action Required: Add RBS signatures for the new classes.

2. CHANGELOG Update Missing

According to CLAUDE.md:

IMPORTANT: This is a monorepo with TWO separate changelogs:

  • Pro: /CHANGELOG_PRO.md - for react_on_rails_pro gem

However, I don't see an entry in react_on_rails_pro/CHANGELOG.md for this feature. This is a user-visible feature that should be documented.

Action Required: Add changelog entry under "### Added" section:

- **Async React Component Rendering**: Added `async_react_component` and `cached_async_react_component` helpers for concurrent component rendering. Multiple components now execute HTTP requests to the Node renderer in parallel instead of sequentially, significantly reducing latency when rendering multiple components in a view. Requires `ReactOnRailsPro::AsyncRendering` concern in controller. [PR 2139](https://github.com/shakacode/react_on_rails/pull/2139) by [AbanoubGhadban](https://github.com/AbanoubGhadban).

3. Instance Variable Naming Convention - Minor

In async_rendering.rb (line 46):

@react_on_rails_async_barrier = Async::Barrier.new

The instance variable name is quite long. Consider if @async_barrier would be clearer and avoid collision, similar to how streaming uses @async_barrier in the helper tests. However, this may be intentional to avoid conflicts - if so, a comment explaining the namespace would help.

4. Barrier Wait Not Called - Potential Memory Leak?

Looking at the AsyncRendering concern, tasks are spawned on the barrier but there's no explicit barrier.wait call. In the streaming code, I see:

@async_barrier.wait  # Waits for all tasks

Question: Does Rails automatically wait for the barrier when the request completes, or could there be tasks still running after the response is sent?

Recommendation: Verify that all async tasks complete before the controller action returns. Consider adding @react_on_rails_async_barrier.wait in the ensure block if needed.

📋 Security & Performance

Security: ✅ No Issues Detected

  • No XSS vulnerabilities (HTML is already escaped by existing helpers)
  • No SQL injection risks (uses standard Rails.cache)
  • No command injection (no shell execution)
  • Proper error handling without leaking sensitive data

Performance: ✅ Well Optimized

  • Cache hits avoid async overhead (ImmediateAsyncValue)
  • Concurrent execution reduces total latency
  • Props blocks only evaluated on cache miss

🧪 Testing Recommendations

Your test coverage is excellent! Consider adding one more edge case:

it "handles barrier being set to nil mid-request" do
  @react_on_rails_async_barrier = Async::Barrier.new
  async_value = async_react_component("App", props: { a: 1 })
  @react_on_rails_async_barrier = nil
  
  # Should still complete successfully
  expect(async_value.value).to include('id="App-react-component')
end

This tests resilience if the barrier is cleared unexpectedly.

📝 Documentation Suggestions

  1. Add Usage Warning: Document that .value blocks the fiber (not the thread) which is safe in async context but would block the entire thread outside async context.

  2. Performance Guidance: Add note about when async rendering provides benefits:

    • Multiple components (>= 2)
    • Each component has non-trivial render time
    • May not help if Node renderer is the bottleneck
  3. Example in README/Docs: The PR description examples are excellent - these should go into the official Pro documentation.

🎯 Action Items Summary

Required Before Merge:

  1. ❌ Add RBS type signatures for new classes
  2. ❌ Update react_on_rails_pro/CHANGELOG.md
  3. ❌ Verify barrier lifecycle - ensure tasks complete before response

Recommended:
4. ⚠️ Add edge case test for barrier being cleared
5. ⚠️ Document blocking behavior of .value
6. ⚠️ Verify cache key behavior matches non-async variant

Overall Assessment: Excellent implementation with strong test coverage. The required items are documentation/hygiene rather than functional issues. Once RBS signatures and changelog are added, this should be ready to merge! 🚀


Review generated with attention to React on Rails coding standards per CLAUDE.md

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
react_on_rails_pro/lib/react_on_rails_pro/async_value.rb (1)

12-37: LGTM! Clean async wrapper implementation.

The AsyncValue class provides a clean interface for wrapping async tasks. The methods correctly delegate to the underlying Async::Task.

One observation: AsyncValue exposes component_name via attr_reader, but ImmediateAsyncValue (from the relevant snippets at react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb lines 7-25) does not have this attribute. If these classes are meant to be used polymorphically, consider adding component_name to ImmediateAsyncValue for interface consistency, or document that it's optional.

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

8-26: Consider adding component_name for interface parity with AsyncValue.

AsyncValue exposes component_name as a public attribute, but ImmediateAsyncValue does not. If code needs to introspect the component name from either type of value, this asymmetry could cause issues.

 class ImmediateAsyncValue
-    def initialize(value)
+    def initialize(value, component_name: nil)
       @value = value
+      @component_name = component_name
     end

-    attr_reader :value
+    attr_reader :value, :component_name
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (1)

851-878: Concurrent execution test may be flaky.

The test relies on Async::Task.current.yield to force fiber interleaving and verify concurrent execution. While this approach works, the assertion expect(max_concurrent).to eq(2) could be sensitive to scheduling. Consider adding a comment explaining the test's intent, or using a more deterministic synchronization mechanism.

Consider adding a brief comment:

       it "executes multiple components concurrently" do
         call_count = 0
         max_concurrent = 0
         mutex = Mutex.new

+        # Mock react_component to track concurrent execution.
+        # Uses yield to allow fiber interleaving, verifying both tasks run simultaneously.
         allow(self).to receive(:react_component) do |_name, _opts|
react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (1)

41-51: Consider explicitly stopping the barrier to handle incomplete tasks.

If a view finishes without calling .value on all AsyncValue objects (e.g., due to early return or conditional rendering), tasks may still be running. Calling barrier.stop in ensure would gracefully cancel any incomplete tasks and avoid potential resource leaks.

     def wrap_in_async_react_context
       require "async"
       require "async/barrier"
 
       Sync do
         @react_on_rails_async_barrier = Async::Barrier.new
         yield
       ensure
+        @react_on_rails_async_barrier&.stop
         @react_on_rails_async_barrier = nil
       end
     end
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (2)

376-386: Cache read options may not apply as expected.

Rails.cache.read accepts a second argument for options, but typically read options (like race_condition_ttl) are write-only. If cache_options contains only write-time options, passing them to read is harmless but unnecessary. Consider removing the second argument for clarity:

-    cached_result = Rails.cache.read(cache_key, cache_options)
+    cached_result = Rails.cache.read(cache_key)

416-422: Consider adding &block to method signature for consistency.

The method uses yield implicitly. While this works, other methods in this file (e.g., render_async_react_component_uncached) explicitly declare &block. Adding it here improves readability and consistency.

-  def prepare_async_render_options(raw_options)
+  def prepare_async_render_options(raw_options, &block)
     raw_options.merge(
-      props: yield,
+      props: block.call,
       skip_prerender_cache: true,
       auto_load_bundle: ReactOnRails.configuration.auto_load_bundle || raw_options[:auto_load_bundle]
     )
   end
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16b3908 and 5d78a83.

📒 Files selected for processing (8)
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (2 hunks)
  • react_on_rails_pro/lib/react_on_rails_pro.rb (1 hunks)
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb (1 hunks)
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (1 hunks)
  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb (1 hunks)
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (1 hunks)
  • react_on_rails_pro/spec/react_on_rails_pro/async_value_spec.rb (1 hunks)
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rb,rake}

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS run bundle exec rubocop and fix ALL violations before every commit/push

Files:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS ensure files end with a newline character before committing/pushing

Files:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
🧠 Learnings (19)
📓 Common learnings
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.
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
📚 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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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 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/immediate_async_value.rb
  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.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:

  • react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb
  • react_on_rails_pro/lib/react_on_rails_pro/async_value.rb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Use Rails helpers (`appFactories`, `appEval`) in Playwright tests instead of UI-based setup to leverage factory_bot and arbitrary Ruby code execution

Applied to files:

  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/async_value_spec.rb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.

Applied to files:

  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.

Applied to files:

  • react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.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:

  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.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:

  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro.rb
  • react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
🧬 Code graph analysis (4)
react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/async_value.rb (6)
  • initialize (15-18)
  • value (22-24)
  • attr_reader (12-37)
  • resolved? (26-28)
  • to_s (30-32)
  • html_safe (34-36)
react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb (2)
react_on_rails_pro/lib/react_on_rails_pro/async_value.rb (4)
  • value (22-24)
  • resolved? (26-28)
  • to_s (30-32)
  • html_safe (34-36)
react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb (3)
  • resolved? (15-17)
  • to_s (19-21)
  • html_safe (23-25)
react_on_rails_pro/lib/react_on_rails_pro.rb (1)
react_on_rails_pro/spec/dummy/client/node-renderer.js (1)
  • require (8-8)
react_on_rails_pro/lib/react_on_rails_pro/async_value.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/immediate_async_value.rb (5)
  • initialize (8-26)
  • initialize (9-11)
  • resolved? (15-17)
  • to_s (19-21)
  • html_safe (23-25)
⏰ 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: examples (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: claude-review
🔇 Additional comments (13)
react_on_rails_pro/lib/react_on_rails_pro.rb (1)

23-25: LGTM! Correct integration of async rendering components.

The new requires are appropriately placed and follow the existing pattern of loading concerns before their related classes.

react_on_rails_pro/spec/react_on_rails_pro/immediate_async_value_spec.rb (1)

1-46: LGTM! Comprehensive test coverage for ImmediateAsyncValue.

Tests cover all public methods with clear, focused examples. The test structure follows good RSpec conventions.

react_on_rails_pro/spec/react_on_rails_pro/async_value_spec.rb (1)

1-101: LGTM! Thorough async behavior testing.

The tests effectively validate AsyncValue behavior including:

  • Proper async task coordination using Sync blocks
  • Exception propagation verification
  • Clever use of Async::Barrier to test the unresolved state
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (4)

820-827: LGTM! Good error handling test.

Properly verifies that async_react_component raises an informative error when called outside the async context established by AsyncRendering concern.


890-897: LGTM! Consistent error handling for cached variant.

Matches the error behavior of async_react_component for calls outside async context.


982-992: LGTM! Important validation tests.

Good coverage for argument validation:

  • Props must be passed as a block (not directly)
  • cache_key is required

These tests ensure users get clear error messages for common mistakes.


951-980: LGTM! Conditional caching tests are thorough.

Tests verify both :if and :unless options correctly disable caching when conditions are met, using RandomValue component to detect re-rendering.

react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (1)

1-27: Good documentation with clear usage examples.

The examples cover common use cases (all actions, specific actions, excluding actions) which will help users adopt this feature.

react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (5)

237-249: LGTM!

Clear error message, proper barrier usage, and correct AsyncValue construction.


270-275: LGTM!

Follows the same pattern as cached_react_component with proper validation via check_caching_options!.


393-401: LGTM!

Correctly creates async task without caching when conditional caching is disabled.


403-414: LGTM!

The cache write-through on completion is correctly implemented inside the async task. The benign race condition on concurrent cache misses follows standard cache-aside semantics.


220-422: Run RuboCop to validate the new code.

As per coding guidelines, run bundle exec rubocop and fix any violations before committing these changes.

@AbanoubGhadban AbanoubGhadban force-pushed the feature/async-react-component-2138 branch from b7f640b to 8aaf212 Compare November 27, 2025 15:15
@claude
Copy link

claude bot commented Nov 27, 2025

Code Review: Async React Component Rendering

I've completed a comprehensive review of PR #2139. This is a well-implemented feature with solid test coverage. Below are my findings organized by category:


Strengths

Code Quality

  • Clean API design: The async_react_component and cached_async_react_component helpers follow Rails conventions and match the existing react_component/cached_react_component patterns
  • Excellent error handling: Clear error messages when async context is missing, guiding users to include the concern
  • Proper separation of concerns: AsyncValue and ImmediateAsyncValue classes are well-focused with clear responsibilities
  • Good documentation: Comprehensive YARD docs with examples in the helpers

Test Coverage

  • Comprehensive test suite: Unit tests for both value classes plus extensive integration tests
  • Edge cases covered: Cache hits/misses, conditional caching (:if/:unless), error propagation, concurrent execution verification
  • Real-world demo: The async_components_demo view with 10 delayed components is a great demonstration

Architecture

  • Smart caching strategy: Synchronous cache lookup prevents unnecessary async overhead on cache hits
  • Proper async/await usage: Leverages Async::Barrier correctly for coordinating concurrent tasks within a request
  • Minimal dependencies: Reuses the existing async gem already in the codebase

⚠️ Issues & Recommendations

1. Missing Changelog Entry (CRITICAL)

According to CLAUDE.md, this is a Pro-only feature and should be documented in the Pro changelog:

Action needed: Add an entry to react_on_rails_pro/CHANGELOG.md under ## [Unreleased]### Added:

### Added
- **Concurrent React Component Rendering**: Added `async_react_component` and `cached_async_react_component` helpers for concurrent server-side rendering. Multiple components now render in parallel instead of sequentially, significantly reducing total render time. Requires including `ReactOnRailsPro::AsyncRendering` concern in controllers. [PR 2139](https://github.com/shakacode/react_on_rails/pull/2139) by [AbanoubGhadban](https://github.com/AbanoubGhadban).

2. Potential Memory Concern in to_s and html_safe

In AsyncValue#to_s and AsyncValue#html_safe:

def to_s
  value.to_s  # Calls value.to_s every time
end

def html_safe
  value.html_safe  # Calls value.html_safe every time
end

Issue: These methods call .value (which blocks and retrieves the task result) every time they're invoked. If ERB templates or logging code calls these methods multiple times, the HTML string is fetched repeatedly.

Recommendation: Consider memoizing the result:

def value
  @value ||= @task.wait
end

This is particularly important since Rails may call to_s multiple times during rendering.

3. Thread Safety of Instance Variable Assignment

In AsyncRendering#wrap_in_async_react_context:

def wrap_in_async_react_context
  Sync do
    @react_on_rails_async_barrier = Async::Barrier.new
    yield
  ensure
    @react_on_rails_async_barrier = nil
  end
end

Minor concern: While Rails controllers are typically single-threaded per request, instance variable assignment isn't thread-safe. If a controller action somehow spawns threads (unlikely but possible with background jobs), this could be problematic.

Assessment: Probably fine for typical Rails usage, but document this assumption. If thread-safety is a concern, consider using Thread.current[:react_on_rails_async_barrier] instead.

4. Component Name Not Used in AsyncValue

AsyncValue stores component_name but only exposes it via attr_reader. It's never used in error messages or debugging output.

Suggestion: Enhance error reporting by including component name:

def value
  @task.wait
rescue => e
  raise ReactOnRailsPro::Error, "Error rendering async component '\#{@component_name}': \#{e.message}", e.backtrace
end

This would help developers identify which component failed in complex views with multiple async components.

5. Props Block Evaluation Timing

In prepare_async_render_options:

def prepare_async_render_options(raw_options)
  raw_options.merge(
    props: yield,  # Block evaluated immediately
    skip_prerender_cache: true,
    auto_load_bundle: ReactOnRails.configuration.auto_load_bundle || raw_options[:auto_load_bundle]
  )
end

Observation: The props block is evaluated before the async task starts, meaning expensive prop generation happens synchronously. This is correct behavior (you want props ready when rendering starts), but it differs from lazy evaluation in cached_react_component.

Recommendation: Document this clearly in the helper docs to set correct expectations. Users shouldn't expect prop generation to be concurrent—only the rendering itself.


🔒 Security Assessment

No Security Concerns Found

  • No user input is interpolated into unsafe contexts
  • Cache keys are properly generated using existing ReactOnRailsPro::Cache.react_component_cache_key
  • No new attack surface introduced
  • Error messages don't leak sensitive information

Performance Considerations

Excellent Performance Design

  1. Cache hit path optimized: Returns ImmediateAsyncValue immediately on cache hit—no async overhead
  2. Concurrent execution verified: Tests confirm multiple components render in parallel
  3. Proper use of Async::Barrier: Ensures all tasks complete before response finishes

Minor Optimization Opportunities

  1. Memoize .value (mentioned above) to avoid redundant task.wait calls
  2. Consider connection pooling impact: If rendering makes HTTP requests to Node renderer, ensure the HTTP client connection pool can handle concurrent requests. The default may need tuning for high concurrency.

📋 Testing Feedback

Strong Test Coverage

  • ✅ Unit tests for both value classes
  • ✅ Integration tests covering happy path, errors, caching, conditionals
  • ✅ Concurrent execution verification test
  • ✅ Cache hit/miss behavior tests

Missing Test Cases (Nice to Have)

  1. Large number of concurrent components: Test behavior with 50+ components to identify any connection pool or memory issues
  2. Nested async calls: What happens if react_component internally calls another async operation?
  3. Timeout scenarios: What if a component takes extremely long to render? Does the barrier wait indefinitely?
  4. Controller inheritance: Does the concern work correctly when included in a base controller and used in subclasses?

📖 Documentation Review

Good

  • YARD docs are comprehensive with examples
  • Error messages guide users to the solution
  • Demo page clearly demonstrates the feature

Could Be Better

  • Add migration guide for users converting from react_component to async_react_component
  • Document connection pool sizing recommendations for Node renderer
  • Clarify that prop block evaluation is still synchronous (only rendering is concurrent)

🎯 Action Items Before Merge

MUST Fix

  1. ✅ Add Pro changelog entry (as shown above)

SHOULD Fix

  1. ⚠️ Memoize AsyncValue#value to prevent redundant task.wait calls
  2. ⚠️ Enhance error messages with component name for better debugging

COULD Fix (Optional)

  1. 💡 Add connection pool sizing guidance to docs
  2. 💡 Add test for high concurrency (50+ components)
  3. 💡 Clarify prop evaluation timing in documentation

Overall Assessment

This is a high-quality PR that adds significant value to React on Rails Pro. The implementation is solid, tests are comprehensive, and the API design is intuitive. The async rendering approach is sound and will deliver meaningful performance improvements for views with multiple server-rendered components.

Recommendation: Approve after addressing the mandatory changelog entry and considering the value memoization optimization.

Great work! 🚀

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx (1)

1-21: Clarify 'use client' on server file and consider de‑duplicating markup

This .server.jsx file exports a server render function, so keeping a 'use client'; directive at the top may be confusing unless your bundler explicitly relies on it here—if it’s not required, consider dropping it. Also, the DelayedComponent JSX is identical to the client version; if you expect this to evolve, you might optionally share the small presentational component between server/client variants to keep them in sync.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d78a83 and b7f640b.

📒 Files selected for processing (5)
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb (2 hunks)
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb (1 hunks)
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx (1 hunks)
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx (1 hunks)
  • react_on_rails_pro/spec/dummy/config/routes.rb (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS ensure files end with a newline character before committing/pushing

Files:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
**/*.{rb,rake}

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS run bundle exec rubocop and fix ALL violations before every commit/push

Files:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
🧠 Learnings (17)
📓 Common learnings
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.
📚 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:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 Learning: 2024-07-27T10:08:35.868Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-07-27T10:08:35.868Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.

Applied to files:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 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:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.

Applied to files:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/config/routes.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 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 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/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Use Rails helpers (`appFactories`, `appEval`) in Playwright tests instead of UI-based setup to leverage factory_bot and arbitrary Ruby code execution

Applied to files:

  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
🧬 Code graph analysis (2)
react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (1)
  • enable_async_react_rendering (34-36)
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx (1)
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx (1)
  • DelayedComponent (6-10)
⏰ 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). (6)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: examples (3.2, minimum)
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: claude-review
🔇 Additional comments (4)
react_on_rails_pro/spec/dummy/config/routes.rb (1)

41-41: Route wiring for async demo is consistent

Path, controller action, and named route follow the existing Pro routes convention and line up with the new action/view; no issues spotted.

react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx (1)

1-12: Client DelayedComponent demo looks good

Props/defaults match the server variant and the markup is minimal and clear; this is a fine client-side counterpart for the async demo.

react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb (1)

1-34: Async demo view correctly exercises async_react_component

The view starts all 10 async renders non-blockingly, then resolves them via component.value and reports elapsed time, which neatly demonstrates the intended concurrency behavior.

react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb (1)

6-8: AsyncRendering wiring and demo action are well-scoped

Including ReactOnRailsPro::AsyncRendering and limiting enable_async_react_rendering to :async_components_demo keeps the async context opt-in, and the new action cleanly renders the matching demo template used by the route.

Also applies to: 163-167

@claude
Copy link

claude bot commented Nov 27, 2025

Code Review for PR #2139: Add async_react_component and cached_async_react_component helpers

This is an excellent PR that adds a valuable performance optimization feature to React on Rails Pro! The implementation is well-thought-out and comprehensive. Here's my detailed feedback:


✅ Strengths

1. Excellent Architecture & Design

  • Clean separation of concerns with AsyncValue, ImmediateAsyncValue, and AsyncRendering concern
  • Smart optimization: cache hits return ImmediateAsyncValue (no async overhead), cache misses use AsyncValue
  • Consistent API design that mirrors existing cached_react_component options (:cache_key, :cache_options, :if, :unless)
  • Proper error handling with clear error messages when used incorrectly

2. Comprehensive Test Coverage 🎯

  • Unit tests: AsyncValue and ImmediateAsyncValue thoroughly tested
  • Integration tests: All major scenarios covered including:
    • Cache hit/miss behavior
    • Concurrent execution verification (lines 851-878)
    • Error propagation
    • Conditional caching with :if/:unless
    • Block evaluation optimization
  • Demo page: Excellent real-world example showing 10 components rendering in ~1s vs ~10s

3. Performance Considerations

  • Uses Async::Barrier correctly for coordinating concurrent tasks
  • Proper async lifecycle management with ensure blocks (line 50-52)
  • Warning system for unresolved async components (lines 55-66) - excellent debugging feature!

4. Code Quality

  • Well-documented with clear YARD documentation
  • Follows project style and conventions
  • Good use of Ruby idioms
  • Proper frozen string literals

🔍 Areas for Improvement

1. Missing CHANGELOG Update ⚠️

According to CLAUDE.md, Pro features must update /CHANGELOG_PRO.md. This is a user-visible feature and should have a changelog entry.

Recommendation: Add entry to CHANGELOG_PRO.md:

#### New Features
- [PR 2139](https://github.com/shakacode/react_on_rails/pull/2139) by [AbanoubGhadban](https://github.com/AbanoubGhadban): Add `async_react_component` and `cached_async_react_component` helpers for concurrent component rendering. Multiple React component renders now execute in parallel instead of sequentially, significantly improving page load times when rendering multiple components. Requires controller to include `ReactOnRailsPro::AsyncRendering` concern.

2. Documentation Suggestions

File: react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb

Lines 55-66: The error logging is excellent, but consider:

  • Should this be a warning instead of an error? The system automatically stops the tasks, so it's self-healing
  • Could add a counter to track which components are unresolved for better debugging
  • Consider: Rails.logger.warn might be more appropriate since it's not fatal

File: react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb

Lines 237-249: Consider adding a note in the documentation about when NOT to use async rendering:

# @note Do not use for single components or when components must render sequentially.
#       Best for pages with 3+ independent components that can render in parallel.

3. Potential Edge Cases

Race Condition in Concurrent Cache Writes (Lines 407-411):
Multiple concurrent requests with the same cache key could result in duplicate work. This is generally acceptable but worth noting in documentation.

Current code:

task = @react_on_rails_async_barrier.async do
  result = react_component(component_name, options)
  Rails.cache.write(cache_key, result, cache_options)  # Multiple tasks might write
  result
end

Impact: Low - Rails.cache.write is typically atomic, and duplicate rendering on first hit is acceptable for this use case. Consider documenting this behavior if users report it.

4. Memory Considerations

File: react_on_rails_pro_helper.rb, lines 244-248:
With many async components, all tasks are started immediately and kept in memory until .value is called. For pages with 50+ components, this could use significant memory.

Recommendation: Document this in the helper method:

# @note All async tasks are started immediately. For pages with many components (50+),
#       consider breaking them into smaller groups or using traditional render for some.

5. Error Context (Minor)

File: async_value.rb, line 23:
When an async render fails, the original component name is available but not included in the re-raised exception. This could make debugging harder.

Suggestion:

def value
  @task.wait
rescue => e
  # Re-raise with component context
  raise e, "Error rendering async component '#{@component_name}': #{e.message}", e.backtrace
end

🔒 Security Considerations

✅ No security concerns identified:

  • No user input is processed differently
  • Uses existing react_component helper internally (inherits its XSS protections)
  • Cache keys are properly sanitized by Rails.cache
  • No new external dependencies introduced

🧪 Test Coverage Assessment

Excellent coverage (95%+ estimated):

  • ✅ Unit tests for both value classes
  • ✅ Integration tests for concurrent execution
  • ✅ Cache behavior thoroughly tested
  • ✅ Error propagation verified
  • ✅ Conditional caching tested
  • ⚠️ Missing: Performance regression test (verify concurrent is actually faster than sequential)

Suggestion: Add a performance test to prevent regressions:

it "renders concurrently faster than sequentially" do
  sequential_time = Benchmark.realtime do
    10.times { react_component("SlowComponent", props: {}) }
  end
  
  async_values = 10.times.map { async_react_component("SlowComponent", props: {}) }
  concurrent_time = Benchmark.realtime do
    async_values.each(&:value)
  end
  
  expect(concurrent_time).to be < (sequential_time * 0.3) # 70% faster
end

📝 Additional Notes

Positive Observations:

  1. Lines 820-887 (helper spec): The concurrent execution test using mutex and fiber yielding is clever and correct
  2. Lines 237-249: Proper guard clause prevents misuse outside async context
  3. Lines 360-390: Cache lookup logic is clean and efficient
  4. Demo page: Perfect teaching tool showing 10-fold performance improvement

Questions for Consideration:

  1. Should there be a configuration option for max concurrent renders?
  2. Would a async_react_component_batch helper be useful for cleaner syntax when rendering many components?
  3. Should the concern log when async rendering is enabled? (for debugging/auditing)

🎯 Recommendation

LGTM with minor changes:

  • ✅ Approve after adding CHANGELOG_PRO.md entry
  • Consider the documentation improvements above
  • The edge cases noted are acceptable for v1

Priority: High - This is a high-value feature that's well-implemented. The changelog is the only blocking issue.


Great work on this feature! This will provide significant performance improvements for React on Rails Pro users with multiple server-rendered components. The implementation is solid, well-tested, and follows Ruby/Rails best practices.

🤖 Generated by Claude Code

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7f640b and 8aaf212.

📒 Files selected for processing (6)
  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (1 hunks)
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb (2 hunks)
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb (1 hunks)
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx (1 hunks)
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx (1 hunks)
  • react_on_rails_pro/spec/dummy/config/routes.rb (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • react_on_rails_pro/spec/dummy/config/routes.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx
  • react_on_rails_pro/spec/dummy/app/views/pages/pro/async_components_demo.html.erb
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rb,rake}

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS run bundle exec rubocop and fix ALL violations before every commit/push

Files:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS ensure files end with a newline character before committing/pushing

Files:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
🧠 Learnings (18)
📓 Common learnings
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.
📚 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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.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 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/concerns/async_rendering.rb
  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.

Applied to files:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 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:

  • react_on_rails_pro/spec/dummy/app/controllers/pages_controller.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:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.

Applied to files:

  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
🧬 Code graph analysis (2)
react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (1)
  • enable_async_react_rendering (34-36)
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx (1)
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.client.jsx (1)
  • DelayedComponent (6-10)
🪛 GitHub Actions: React on Rails Pro - Lint
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx

[error] 1-1: ESLint: 'no-promise-executor-return' violation. Return values from promise executor functions cannot be read. (no-promise-executor-return)

🪛 GitHub Check: pro-lint-js-and-ruby
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx

[failure] 18-18:
Return values from promise executor functions cannot be read

⏰ 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: package-js-tests
  • GitHub Check: dummy-app-node-renderer-e2e-tests
  • GitHub Check: rspec-dummy-app-node-renderer
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: examples (3.2, minimum)
🔇 Additional comments (5)
react_on_rails_pro/lib/react_on_rails_pro/concerns/async_rendering.rb (4)

1-27: LGTM! Clear documentation and proper concern structure.

The module documentation effectively explains the purpose and usage patterns with helpful examples. The concern follows Rails conventions properly.


29-37: LGTM! Proper filter registration.

The class method correctly delegates to around_action with filter options, following Rails conventions.


55-66: LGTM! Helpful error message for unresolved tasks.

The check correctly identifies pending tasks and provides clear guidance on how to fix the issue. The error message will help developers debug missing .value calls.


41-53: Remove invalid runtime dependency concern; clarify error handling behavior.

The implementation properly handles cleanup when yield raises an exception:

  1. Error handling is sound: The ensure block (lines 49-52) will execute regardless of exceptions, calling barrier.stop to terminate all pending async tasks. The concern about tasks being "stopped but not logged" is valid—check_for_unresolved_async_components (which logs pending tasks) is skipped on error. However, this appears intentional to avoid log noise on failed actions.

  2. Runtime dependency is satisfied: The async gem is already declared in react_on_rails_pro.gemspec as s.add_runtime_dependency "async", ">= 2.6", so the require statements will not fail due to a missing gem.

If developers should be warned about unresolved tasks even when actions fail, consider moving the unresolved component check into the ensure block to guarantee it always executes.

react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb (1)

6-8: LGTM! Proper async rendering setup for demo.

The controller correctly:

  • Includes the AsyncRendering concern (Line 6)
  • Enables async rendering only for the demo action (Line 8)
  • Provides a well-documented demo action (Lines 163-167)

The implementation follows the exact pattern documented in the concern's examples.

Also applies to: 163-167

Comment on lines +1 to +11
'use client';

import React from 'react';

// Component that simulates a slow render by delaying for 1 second
// Used to demonstrate concurrent rendering with async_react_component
const DelayedComponent = ({ index, delayMs = 1000 }) => (
<div style={{ padding: '10px', margin: '5px', border: '1px solid #ccc' }}>
<strong>Component {index}</strong> - Rendered after {delayMs}ms delay
</div>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove 'use client' directive from server component file.

The file is named DelayedComponent.server.jsx, which indicates a server component, but it contains a 'use client' directive (Line 1). These are contradictory:

  • Server components (.server.jsx) run only on the server
  • The 'use client' directive marks a component boundary for client-side rendering

Based on the async function export pattern (Lines 14-21) and the server-side delay simulation, this should be a server component.

Apply this diff to remove the directive:

-'use client';
-
 import React from 'react';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
'use client';
import React from 'react';
// Component that simulates a slow render by delaying for 1 second
// Used to demonstrate concurrent rendering with async_react_component
const DelayedComponent = ({ index, delayMs = 1000 }) => (
<div style={{ padding: '10px', margin: '5px', border: '1px solid #ccc' }}>
<strong>Component {index}</strong> - Rendered after {delayMs}ms delay
</div>
);
import React from 'react';
// Component that simulates a slow render by delaying for 1 second
// Used to demonstrate concurrent rendering with async_react_component
const DelayedComponent = ({ index, delayMs = 1000 }) => (
<div style={{ padding: '10px', margin: '5px', border: '1px solid #ccc' }}>
<strong>Component {index}</strong> - Rendered after {delayMs}ms delay
</div>
);
🧰 Tools
🪛 GitHub Actions: React on Rails Pro - Lint

[error] 1-1: ESLint: 'no-promise-executor-return' violation. Return values from promise executor functions cannot be read. (no-promise-executor-return)

🤖 Prompt for AI Agents
In
react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
lines 1-11, remove the top `'use client'` directive because this file is a
server component (.server.jsx) and the directive is contradictory; delete the
first line, verify no client-only hooks or browser APIs are used later in the
file, keep the React import and the server-side async component/export as-is,
and run tests/build to ensure no client-only code remains.

- Add RBS signatures for AsyncValue, ImmediateAsyncValue, and AsyncRendering
- Add CHANGELOG entry for the async_react_component feature

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@claude
Copy link

claude bot commented Nov 27, 2025

Code Review: Async React Component Rendering

This is an excellent PR that brings significant performance improvements to React on Rails Pro. The implementation is well-designed, thoroughly tested, and follows best practices. Here's my detailed review:


✅ Strengths

1. Clean Architecture & Design

  • Separation of concerns: AsyncValue and ImmediateAsyncValue are cleanly separated value objects
  • Opt-in pattern: Requires explicit include AsyncRendering and enable_async_react_rendering - prevents accidental usage
  • Smart cache optimization: Cache hits return ImmediateAsyncValue (no async overhead), cache misses use AsyncValue
  • Consistent API: New helpers mirror existing helpers (async_react_componentreact_component, cached_async_react_componentcached_react_component)

2. Robust Error Handling

  • Clear error messages: When barrier is missing, error tells developers exactly what to do
  • Natural error propagation: task.wait re-raises exceptions from async renders at .value call
  • Safety checks: check_for_unresolved_async_components warns about unresolved tasks (prevents memory leaks)
  • Proper cleanup: ensure block stops and nils the barrier

3. Excellent Test Coverage 🎯

  • Unit tests: Both AsyncValue and ImmediateAsyncValue thoroughly tested
  • Integration tests: Comprehensive helper specs covering:
    • Concurrent execution verification
    • Cache hit/miss behavior
    • Error propagation
    • Conditional caching (:if/:unless)
    • Edge cases (props as block, missing cache_key)
  • Real-world demo: async_components_demo.html.erb provides visual proof of concept

4. Performance Optimization

  • Synchronous cache lookups: No async overhead when cache hits - ImmediateAsyncValue returns immediately
  • Minimal overhead: Only creates barrier when needed, cleans up properly
  • Realistic impact: Demo shows 10x speedup (10s → 1s for 10 components with 1s delay each)

5. Type Safety

  • RBS signatures included: All new classes have complete type definitions
  • Runtime checking: Will catch type violations if runtime checking is enabled

6. Documentation & Examples

  • Clear docstrings: Every public method has comprehensive documentation with examples
  • Working demo: async_components_demo provides runnable example
  • Proper changelog entry: Follows project conventions with PR link and attribution

💡 Minor Suggestions (Non-blocking)

1. Helper Method Location (react_on_rails_pro_helper.rb:237-275)

The new async helpers are in ReactOnRailsProHelper, but they depend on @react_on_rails_async_barrier which is set by the controller concern. This creates a tight coupling between view helpers and controller state.

Consideration: This is actually reasonable given Rails' architecture - helpers have access to controller instance variables. The error messages make the requirement clear.

2. Barrier Size Checking (async_rendering.rb:58)

check_for_unresolved_async_components logs a warning about unresolved tasks. This is great for debugging.

Enhancement idea (future): Could potentially add config option to make this raise in test/development environments (stricter enforcement).

3. Test Helper Duplication (react_on_rails_pro_helper_spec.rb:336-363)

The mock_request_and_response helper is quite complex and used across multiple test contexts.

Minor: This is fine for tests, but consider extracting to a shared test helper file if this pattern grows.

4. Component Name in AsyncValue (async_value.rb:13-17)

component_name is stored but only used for initialization. It's exposed as attr_reader but not actively used.

Question: Is this for debugging/logging purposes? If so, consider documenting this intent. If not needed, could be removed to simplify.


🔍 Potential Issues (Please Verify)

1. Async Context Leaking Across Requests ⚠️

In async_rendering.rb:45-52, the Sync block wraps the entire request. The barrier is stored in @react_on_rails_async_barrier.

Concern: In threaded/concurrent server environments (Puma with threads), instance variables on the controller are request-scoped, so this should be safe. But verify:

  • Does the ensure block always run even on exceptions?
  • Could a sudden client disconnect leave tasks running?

Recommendation: Add integration test that simulates request timeout/cancellation to verify cleanup.

2. Cache Key Consistency (react_on_rails_pro_helper.rb:372)

Cache key generation for async uses ReactOnRailsPro::Cache.react_component_cache_key, same as synchronous version.

Question: Does this include the bundle hash when prerender: true? Verify that:

  • Cache keys are identical between cached_react_component and cached_async_react_component with same options
  • Bundle hash is included for server-rendered components

3. Barrier.stop vs Barrier.wait (async_rendering.rb:50)

The ensure block calls @react_on_rails_async_barrier&.stop but the demo tests call @async_barrier.wait.

Verification needed:

  • Does .stop wait for in-flight tasks to complete or kill them immediately?
  • Should it be .wait before .stop to ensure graceful completion?
  • What happens to queued but not-yet-started tasks?

Check docs: Review Async::Barrier#stop behavior to ensure no tasks are orphaned.


📋 Checklist Before Merge

  • RuboCop passes: Run bundle exec rubocop (MANDATORY per CLAUDE.md)
  • Prettier passes: Run rake autofix to ensure formatting
  • RBS validation: Run bundle exec rake rbs:validate for new signatures
  • Integration tests: Verify tests pass locally, not just in CI
  • Demo works: Manually test /async_components_demo route shows ~1s render time
  • Documentation: Consider adding usage example to Pro docs

🎯 Summary

This PR is production-ready with excellent implementation quality. The concurrent rendering feature is:

✅ Well-architected with clear separation of concerns
✅ Thoroughly tested with comprehensive coverage
✅ Performance-optimized with smart cache handling
✅ Type-safe with RBS signatures
✅ Well-documented with clear examples

The minor concerns above are mostly about verification and future enhancements, not blockers.

Recommendation: ✅ APPROVE with verification of the barrier cleanup behavior.

Great work @AbanoubGhadban! This will be a valuable feature for Pro users. 🚀


Review conducted following CLAUDE.md conventions. Generated with Claude Code

Use block statement in Promise executor to avoid implicit return of
setTimeout's timer ID.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@claude
Copy link

claude bot commented Nov 27, 2025

Code Review: Async React Component Rendering

Overall Assessment: ✅ Excellent implementation! This is a well-architected feature with comprehensive testing and clear documentation. The code quality is high and follows the project's conventions.


🎯 Strengths

Architecture & Design

  • Smart dual-value pattern: Using AsyncValue for cache misses and ImmediateAsyncValue for cache hits is elegant and avoids unnecessary async overhead for cached content
  • Proper separation of concerns: The AsyncRendering concern cleanly separates the controller setup from the helper methods
  • Consistent API: The async methods mirror the existing react_component and cached_react_component APIs, making adoption easy

Error Handling

  • Clear error messages: Helpful errors when async context is missing (line 23-25 in helper, line 362-364)
  • Exception propagation: Exceptions from rendering correctly bubble up via task.wait (async_value.rb:23)
  • Unresolved task detection: The check_for_unresolved_async_components method (async_rendering.rb:54-65) catches developer mistakes with clear logging

Testing

  • Comprehensive coverage: 176 lines of integration tests + 147 lines of unit tests
  • Concurrent execution verification: Test proves parallelism works (helper_spec.rb:835-862)
  • Edge cases covered: Cache hits/misses, error propagation, conditional caching, missing options

Type Safety

  • RBS signatures included: All three new classes have proper type definitions
  • Documentation: Clear YARD docs with examples

🔍 Issues & Concerns

🚨 CRITICAL: ESLint Warning in DelayedComponent.server.jsx

Location: react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx:18

await new Promise((resolve) => {
  setTimeout(resolve, delayMs);  // ⚠️ eslint: no-promise-executor-return
});

Issue: According to commit 38d0dfd, this triggered an ESLint error: no-promise-executor-return. The fix applied was to use a block statement, but looking at the current code, it still uses an implicit return.

Expected fix (based on the commit message):

await new Promise((resolve) => {
  setTimeout(resolve, delayMs);
});

Wait, actually this IS the fixed version. Let me check the actual linting...

Actually, upon closer inspection of commit 38d0dfd, the issue was that the code originally had:

await new Promise((resolve) => setTimeout(resolve, delayMs));

And it was changed to use a block statement. The current code is correct! ✅


🤔 Minor Concerns & Suggestions

1. Performance: Cache Lookup on Every Call

Location: react_on_rails_pro_helper.rb:370-372

cached_result = Rails.cache.read(cache_key, cache_options)

While synchronous cache lookup is good (avoids async overhead for hits), it means every call pays the cache lookup cost even when not needed.

Question: Did you consider passing an option to bypass cache lookup for cases where developers know data is fresh? For example:

cached_async_react_component("Card", cache_key: @product, skip_cache: @product.updated_recently?)

Not a blocker, but worth considering for future optimization.


2. Resource Management: Barrier Cleanup

Location: async_rendering.rb:40-51

Sync do
  @react_on_rails_async_barrier = Async::Barrier.new
  yield
  check_for_unresolved_async_components
ensure
  @react_on_rails_async_barrier&.stop
  @react_on_rails_async_barrier = nil
end

Good: The ensure block properly cleans up the barrier.

Question: What happens if yield raises an exception? Will pending tasks be properly terminated?

Looking at the code:

  1. If yield raises, check_for_unresolved_async_components is skipped
  2. But ensure still runs, calling barrier.stop
  3. Async::Barrier#stop should terminate pending tasks

Verification needed: Does Async::Barrier#stop cancel pending tasks or wait for them? If it waits, an exception during yield could cause the request to hang waiting for tasks that might never complete.

Suggestion: Add a comment documenting the expected behavior, or add a test case:

it "terminates pending tasks when an exception occurs during render" do
  # Test that barrier cleanup works even when yield raises
end

3. Code Duplication in Helper Methods

Location: react_on_rails_pro_helper.rb:387-408

The three methods render_async_react_component_uncached, render_async_react_component_with_cache, and prepare_async_render_options handle similar logic with slight variations.

Current:

def render_async_react_component_uncached(component_name, raw_options, &block)
  options = prepare_async_render_options(raw_options, &block)
  task = @react_on_rails_async_barrier.async do
    react_component(component_name, options)
  end
  ReactOnRailsPro::AsyncValue.new(component_name: component_name, task: task)
end

def render_async_react_component_with_cache(component_name, raw_options, cache_key, cache_options, &block)
  options = prepare_async_render_options(raw_options, &block)
  task = @react_on_rails_async_barrier.async do
    result = react_component(component_name, options)
    Rails.cache.write(cache_key, result, cache_options)
    result
  end
  ReactOnRailsPro::AsyncValue.new(component_name: component_name, task: task)
end

These are nearly identical. Could be refactored to:

def render_async_react_component_internal(component_name, raw_options, cache_key: nil, cache_options: nil, &block)
  options = prepare_async_render_options(raw_options, &block)
  task = @react_on_rails_async_barrier.async do
    result = react_component(component_name, options)
    Rails.cache.write(cache_key, result, cache_options) if cache_key
    result
  end
  ReactOnRailsPro::AsyncValue.new(component_name: component_name, task: task)
end

However: The current approach is more explicit and easier to debug. Since these are private methods, the duplication is acceptable. No change needed unless you prefer DRYer code.


4. Documentation: Missing Edge Case

Location: async_rendering.rb:23-24

# @example Enable for all except specific actions
#   class ProductsController < ApplicationController
#     include ReactOnRailsPro::AsyncRendering
#     enable_async_react_rendering except: [:create, :update]
#   end

Missing example: What happens if you call async_react_component in an action that's not enabled?

Expected behavior:

  • It should raise the "AsyncRendering concern" error
  • This is correct, but worth documenting explicitly

Suggestion: Add to the module docs:

# @note Calling async_react_component in actions not covered by enable_async_react_rendering
#       will raise ReactOnRailsPro::Error with a helpful message

5. Type Safety: untyped in RBS

Location: All three RBS files use untyped for several fields:

  • async_value.rbs:5 - @task: untyped
  • async_value.rbs:10 - def value: () -> untyped
  • immediate_async_value.rbs:3 - attr_reader value: untyped

Reasoning: These are inherently dynamic (the value could be any HTML string, the task could be any Async::Task).

Acceptable: Using untyped here is reasonable, but consider:

# async_value.rbs
@task: Async::Task[String]  # If Async gem has RBS types
def value: () -> String      # More specific than untyped

Not a blocker: untyped is fine for now, especially if the async gem doesn't provide RBS signatures.


🎨 Style & Best Practices

✅ Excellent Adherence to Project Standards

  1. Frozen string literals: All files have # frozen_string_literal: true
  2. RuboCop clean: No obvious style violations
  3. YARD documentation: Comprehensive with examples
  4. Test organization: Clear describe blocks with proper contexts
  5. CHANGELOG: Proper format with PR link and attribution

🔒 Security Considerations

✅ No Security Issues Identified

  • No XSS risks: The async methods delegate to react_component which already handles HTML escaping
  • No injection risks: Cache keys are properly constructed using existing ReactOnRailsPro::Cache.react_component_cache_key
  • No resource exhaustion: The barrier limits concurrency to the current request scope
  • No information disclosure: Errors properly logged without leaking sensitive data

📊 Performance Considerations

✅ Performance Wins

  1. Massive latency improvement: 10 components with 1s delay each: 10s → ~1s (10x faster)
  2. Cache hit optimization: ImmediateAsyncValue avoids async overhead for cached content
  3. Minimal memory overhead: Async::Barrier efficiently manages fibers

⚠️ Potential Concerns

  1. Memory usage with many components: Each async task creates a fiber. With 100+ components, memory usage could spike.

    • Mitigation: Document recommended limits in production (e.g., "Use for 2-20 components per page")
  2. Node renderer pool pressure: Concurrent requests to Node renderer could exhaust the pool.

    • Mitigation: The Node renderer pool already handles this, but worth monitoring in production

🧪 Test Coverage Assessment

✅ Excellent Coverage

Unit Tests (148 lines):

  • AsyncValue: value, resolved?, to_s, html_safe, exception handling
  • ImmediateAsyncValue: value, resolved?, to_s, html_safe

Integration Tests (176 lines):

  • Concurrent execution verification ✅
  • Cache hit/miss behavior ✅
  • Error propagation ✅
  • Conditional caching (:if/:unless) ✅
  • Missing options errors ✅

🤷 Missing Test Cases

  1. Barrier cleanup on exception: Does the barrier properly clean up if yield raises?
  2. Multiple barriers in nested controllers: What happens with controller inheritance?
  3. Performance test: Verify that 10 async components are actually ~10x faster than sequential
    • The demo page does this, but no automated test

Suggestion: Add a performance test:

it "executes components concurrently (performance verification)" do
  start = Time.now
  
  values = 10.times.map do |i|
    async_react_component("SlowComponent", props: { delay: 100 })
  end
  
  values.each(&:value)
  elapsed = Time.now - start
  
  # Should take ~100ms (concurrent), not ~1000ms (sequential)
  expect(elapsed).to be < 0.3 # Allow 3x margin for CI variability
end

🚀 Recommendations

Before Merge

  1. Verify ESLint passes: Ensure commit 38d0dfd actually fixed the linting issue
  2. 🤷 Test barrier cleanup: Manually verify that pending tasks are terminated on exception
  3. 📝 Document resource limits: Add a note about recommended component counts (e.g., 2-20 per page)

Future Enhancements (Not Blocking)

  1. Add configuration option: config.max_concurrent_async_components = 20
  2. Add monitoring/instrumentation for async rendering timing
  3. Consider adding a batch_async_react_components helper for very common patterns

✅ Final Verdict

Approve with minor suggestions.

This is a high-quality implementation that:

  • Solves a real performance problem (10x faster for multiple components)
  • Has excellent test coverage
  • Follows project conventions
  • Has clear documentation and examples
  • Handles errors gracefully

The concerns raised are minor and mostly suggestions for future improvements. The code is production-ready.

Great work @AbanoubGhadban! 🎉


Reviewed by: Claude Code
Date: 2025-11-27

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs (1)

1-14: AsyncRendering concern signatures look correct; optional type tightening later

The module nesting, ClassMethods.enable_async_react_rendering(**untyped options) -> void, the instance ivar @react_on_rails_async_barrier, and the private instance methods all line up with the described concern behavior and Rails around_action usage. If/when you add RBS for Async::Barrier and controller/response types, you could optionally tighten these from untyped (e.g., @react_on_rails_async_barrier: ::Async::Barrier? and a more specific return type for wrap_in_async_react_context), but the current surface is fine for an initial introduction. Based on learnings, this matches the intended mapping to the corresponding Ruby concern in lib/react_on_rails_pro/concerns/async_rendering.rb.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8aaf212 and 38d0dfd.

📒 Files selected for processing (5)
  • react_on_rails_pro/CHANGELOG.md (1 hunks)
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs (1 hunks)
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs (1 hunks)
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs (1 hunks)
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/DelayedComponent.server.jsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS ensure files end with a newline character before committing/pushing

Files:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
🧠 Learnings (18)
📓 Common learnings
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.
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG*.md : Use changelog format: `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)` (no hash in PR number)

Applied to files:

  • react_on_rails_pro/CHANGELOG.md
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for Pro-only features, fixes, and changes affecting Pro packages

Applied to files:

  • react_on_rails_pro/CHANGELOG.md
📚 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:

  • react_on_rails_pro/CHANGELOG.md
📚 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:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/CHANGELOG.md
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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 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/CHANGELOG.md
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs
  • react_on_rails_pro/sig/react_on_rails_pro/concerns/async_rendering.rbs
📚 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:

  • react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs
⏰ 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). (6)
  • GitHub Check: examples (3.2, minimum)
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: claude-review
🔇 Additional comments (5)
react_on_rails_pro/CHANGELOG.md (2)

28-28: Changelog entry format and content look good.

The async React component rendering entry follows the established changelog format with the correct PR reference and contributor attribution. The description clearly conveys the feature benefit (parallel HTTP requests) and controller-level requirement.


28-28: File ends with newline character as required.

The changelog entry at line 28 follows the correct format guideline (PR number by username with no hash), and the file properly ends with a newline character per coding standards.

react_on_rails_pro/sig/react_on_rails_pro/immediate_async_value.rbs (2)

1-15: File correctly ends with a newline character.

Verification confirms the file already ends with a newline as required by the coding guidelines. No action needed.


1-15: Interface compatibility concern is invalid; component_name is not accessed by calling code.

Testing and helper code searches reveal that calling code accesses only the .value property on both AsyncValue and ImmediateAsyncValue return values. The .component_name reader is tested only in async_value_spec.rb on AsyncValue instances directly, not on the return values from cached_async_react_component or async_react_component helpers. Both classes are compatible for duck typing through their actual public interface (.value, .resolved?, .to_s, .html_safe). The file correctly ends with a newline.

react_on_rails_pro/sig/react_on_rails_pro/async_value.rbs (1)

1-18: File meets all requirements; type signatures are appropriate and newline is present.

The RBS type signatures for AsyncValue are well-defined and appropriate for an async rendering wrapper. The method signatures (value, resolved?, to_s, html_safe) align with the async rendering flow. The file correctly ends with a newline character, satisfying the coding guideline requirement.

# html = async_value.value # blocks until result is ready
#
class AsyncValue
attr_reader :component_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it ever get used? I only see it in tests. If not, remove it.


module ReactOnRailsPro
# ImmediateAsyncValue is returned when a cached_async_react_component call
# has a cache hit. It provides the same interface as AsyncValue but returns
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, it doesn't, AsyncValue provides component_name as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create async_react_component to pre-render multiple react components concurrently

3 participants