Skip to content
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7e4d04b
tmp
AbanoubGhadban Jun 11, 2024
4fc054c
add support for streaming rendered component using renderToPipeableSt…
AbanoubGhadban Jun 17, 2024
4f95d2d
put ROR scripts after the first rendered chunk
AbanoubGhadban Jun 18, 2024
a651535
remove log statements
AbanoubGhadban Jul 23, 2024
337c5c2
add stream_react_component_async helper function
AbanoubGhadban Jun 20, 2024
849a0c5
add helper function to render a whole view
AbanoubGhadban Jul 24, 2024
7ca17d1
fix failing jest tests
AbanoubGhadban Jul 28, 2024
8e1c42d
linting
AbanoubGhadban Jul 28, 2024
66d1cc2
linting
AbanoubGhadban Jul 28, 2024
b086123
remove redundant new line when context is not prepended
AbanoubGhadban Jul 28, 2024
5d29c97
rename stream_react_component_async to stream_react_component
AbanoubGhadban Jul 28, 2024
80f463e
fix error caused by process on browser
AbanoubGhadban Jul 28, 2024
1d4a68e
remove new line appended to the page when has no rails context
AbanoubGhadban Jul 28, 2024
d3a4c64
fix the problem of not updating the first streaming chunk
AbanoubGhadban Jul 30, 2024
5b96a6a
rename stream_react_component_internal to internal_stream_react_compo…
AbanoubGhadban Aug 19, 2024
f1c3dbf
add unit tests for rails_context_if_not_already_rendered
AbanoubGhadban Aug 19, 2024
7cebb2f
remove :focus tag from rails_context_if_not_already_rendered spec
AbanoubGhadban Aug 20, 2024
096231d
make render function returns Readable stream instead of PassThrough
AbanoubGhadban Aug 24, 2024
809322c
use validateComponent function instead of manually validating it
AbanoubGhadban Oct 13, 2024
f6af606
linting
AbanoubGhadban Oct 13, 2024
8937c5f
add some comments
AbanoubGhadban Oct 15, 2024
bf3ef6c
don't return extra null at he end of the stream
AbanoubGhadban Oct 24, 2024
ebf180d
update CHANGELOG.md
AbanoubGhadban Oct 28, 2024
b6d9fd5
update docs
AbanoubGhadban Oct 29, 2024
2d49ce7
update docs
AbanoubGhadban Oct 29, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac
### [Unreleased]
Changes since the last non-beta release.

### Added
- Added streaming server rendering support:
- New `stream_react_component` helper for adding streamed components to views
- New `streamServerRenderedReactComponent` function in the react-on-rails package that uses React 18's `renderToPipeableStream` API
- Enables progressive page loading and improved performance for server-rendered React components
[PR #1633](https://github.com/shakacode/react_on_rails/pull/1633) by [AbanoubGhadban](https://github.com/AbanoubGhadban).

#### Changed
- Console replay script generation now awaits the render request promise before generating, allowing it to capture console logs from asynchronous operations. This requires using a version of the Node renderer that supports replaying async console logs. [PR #1649](https://github.com/shakacode/react_on_rails/pull/1649) by [AbanoubGhadban](https://github.com/AbanoubGhadban).

Expand Down
1 change: 1 addition & 0 deletions SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Here is the new link:
+ [How React on Rails Works](docs/outdated/how-react-on-rails-works.md)
+ [Client vs. Server Rendering](./docs/guides/client-vs-server-rendering.md)
+ [React Server Rendering](./docs/guides/react-server-rendering.md)
+ [🚀 Next-Gen Server Rendering: Streaming with React 18's Latest APIs](./docs/guides/streaming-server-rendering.md)
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

Fix indentation to match other list items.

The indentation of this line doesn't match other items in the "Basics" section.

Apply this diff to fix the indentation:

-  + [🚀 Next-Gen Server Rendering: Streaming with React 18's Latest APIs](./docs/guides/streaming-server-rendering.md)
+ + [🚀 Next-Gen Server Rendering: Streaming with React 18's Latest APIs](./docs/guides/streaming-server-rendering.md)
📝 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
+ [🚀 Next-Gen Server Rendering: Streaming with React 18's Latest APIs](./docs/guides/streaming-server-rendering.md)
+ [🚀 Next-Gen Server Rendering: Streaming with React 18's Latest APIs](./docs/guides/streaming-server-rendering.md)
🧰 Tools
🪛 Markdownlint

20-20: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)

+ [Render-Functions and the RailsContext](docs/guides/render-functions-and-railscontext.md)
+ [Caching and Performance: React on Rails Pro](https://github.com/shakacode/react_on_rails/wiki).
+ [Deployment](docs/guides/deployment.md).
Expand Down
196 changes: 196 additions & 0 deletions docs/guides/streaming-server-rendering.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
# 🚀 Streaming Server Rendering with React 18

React on Rails Pro supports streaming server rendering using React 18's latest APIs, including `renderToPipeableStream` and Suspense. This guide explains how to implement and optimize streaming server rendering in your React on Rails application.

## Prerequisites

- React on Rails Pro subscription
- React 18 or higher (experimental version)
- React on Rails v15.0.0-alpha.0 or higher

## Benefits of Streaming Server Rendering

- Faster Time to First Byte (TTFB)
- Progressive page loading
- Improved user experience
- Better SEO performance
- Optimal handling of data fetching

## Implementation Steps

1. **Use Experimental React 18 Version**

First, ensure you're using React 18's experimental version in your package.json:

```json
"dependencies": {
"react": "18.3.0-canary-670811593-20240322",
"react-dom": "18.3.0-canary-670811593-20240322"
}
```

2. **Prepare Your React Components**

You can create async React components that return a promise. Then, you can use the `Suspense` component to render a fallback UI while the component is loading.

```jsx
// app/javascript/components/MyStreamingComponent.jsx

import React, { Suspense } from 'react';

const MyStreamingComponent = () => {
return (
<header>
<h1>Streaming Server Rendering</h1>
</header>
<Suspense fallback={<div>Loading...</div>}>
<SlowDataComponent />
</Suspense>
);
};
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

Fix JSX syntax error in the example component.

The component's JSX needs to be wrapped in a parent element or fragment as React components must return a single root element.

 const MyStreamingComponent = () => {
   return (
+    <>
     <header>
       <h1>Streaming Server Rendering</h1>
     </header>
     <Suspense fallback={<div>Loading...</div>}>
       <SlowDataComponent />
     </Suspense>
+    </>
   );
 };
📝 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
const MyStreamingComponent = () => {
return (
<header>
<h1>Streaming Server Rendering</h1>
</header>
<Suspense fallback={<div>Loading...</div>}>
<SlowDataComponent />
</Suspense>
);
};
const MyStreamingComponent = () => {
return (
<>
<header>
<h1>Streaming Server Rendering</h1>
</header>
<Suspense fallback={<div>Loading...</div>}>
<SlowDataComponent />
</Suspense>
</>
);
};


const SlowDataComponent = async () => {
const data = await fetchData();
return <div>{data}</div>;
};

export default MyStreamingComponent;

ReactOnRails.register({ MyStreamingComponent });
```
3. **Add The Component To Your Rails View**

```erb
<!-- app/views/example/show.html.erb -->
<%=
stream_react_component(
'MyStreamingComponent',
props: { greeting: 'Hello, Streaming World!' },
prerender: true
)
%>
<footer>
<p>Footer content</p>
</footer>
```

4. **Render The View Using The `stream_view_containing_react_components` Helper**

Ensure you have a controller that renders the view containing the React components. The controller must include the `ReactOnRails::Controller`, `ReactOnRailsPro::Stream` and `ActionController::Live` modules.

```ruby
# app/controllers/example_controller.rb

class ExampleController < ApplicationController
include ActionController::Live
include ReactOnRails::Controller
include ReactOnRailsPro::Stream

def show
stream_view_containing_react_components(template: 'example/show')
end
end
```

5. **Test Your Application**

You can test your application by running `rails server` and navigating to the appropriate route.


6. **What Happens During Streaming**

When a user visits the page, they'll experience the following sequence:

1. The initial HTML shell is sent immediately, including:
- The page layout
- Any static content (like the `<h1>` and footer)
- Placeholder content for the React component (typically a loading state)

2. As the React component processes and suspense boundaries resolve:
- HTML chunks are streamed to the browser progressively
- Each chunk updates a specific part of the page
- The browser renders these updates without a full page reload

For example, with our `MyStreamingComponent`, the sequence might be:

1. The initial HTML includes the header, footer, and loading state.

```html
<header>
<h1>Streaming Server Rendering</h1>
</header>
<template id="s0">
<div>Loading...</div>
</template>
<footer>
<p>Footer content</p>
</footer>
```

2. As the component resolves, HTML chunks are streamed to the browser:

```html
<template hidden id="b0">
<div>[Fetched data]</div>
</template>

<script>
// This implementation is slightly simplified
document.getElementById('s0').replaceChildren(
document.getElementById('b0')
);
</script>
```

## When to Use Streaming

Streaming SSR is particularly valuable in specific scenarios. Here's when to consider it:

### Ideal Use Cases

1. **Data-Heavy Pages**
- Pages that fetch data from multiple sources
- Dashboard-style layouts where different sections can load independently
- Content that requires heavy processing or computation

2. **Progressive Enhancement**
- When you want users to see and interact with parts of the page while others load
- For improving perceived performance on slower connections
- When different parts of your page have different priority levels

3. **Large, Complex Applications**
- Applications with multiple independent widgets or components
- Pages where some content is critical and other content is supplementary
- When you need to optimize Time to First Byte (TTFB)

### Best Practices for Streaming

1. **Component Structure**
```jsx
// Good: Independent sections that can stream separately
<Layout>
<Suspense fallback={<HeaderSkeleton />}>
<Header />
</Suspense>
<Suspense fallback={<MainContentSkeleton />}>
<MainContent />
</Suspense>
<Suspense fallback={<SidebarSkeleton />}>
<Sidebar />
</Suspense>
</Layout>

// Bad: Everything wrapped in a single Suspense boundary
<Suspense fallback={<FullPageSkeleton />}>
<Header />
<MainContent />
<Sidebar />
</Suspense>
```

2. **Data Loading Strategy**
- Prioritize critical data that should be included in the initial HTML
- Use streaming for supplementary data that can load progressively
- Consider implementing a waterfall strategy for dependent data
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
module.exports = {
preset: 'ts-jest/presets/js-with-ts',
testEnvironment: 'jsdom',
setupFiles: ['<rootDir>/node_package/tests/jest.setup.js'],
};
116 changes: 108 additions & 8 deletions lib/react_on_rails/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,64 @@ def react_component(component_name, options = {})
end
end

# Streams a server-side rendered React component using React's `renderToPipeableStream`.
# Supports React 18 features like Suspense, concurrent rendering, and selective hydration.
# Enables progressive rendering and improved performance for large components.
#
# Note: This function can only be used with React on Rails Pro.
# The view that uses this function must be rendered using the
# `stream_view_containing_react_components` method from the React on Rails Pro gem.
#
# Example of an async React component that can benefit from streaming:
#
# const AsyncComponent = async () => {
# const data = await fetchData();
# return <div>{data}</div>;
# };
#
# function App() {
# return (
# <Suspense fallback={<div>Loading...</div>}>
# <AsyncComponent />
# </Suspense>
# );
# }
#
# @param [String] component_name Name of your registered component
# @param [Hash] options Options for rendering
# @option options [Hash] :props Props to pass to the react component
# @option options [String] :dom_id DOM ID of the component container
# @option options [Hash] :html_options Options passed to content_tag
# @option options [Boolean] :prerender Set to false to disable server-side rendering
# @option options [Boolean] :trace Set to true to add extra debugging information to the HTML
# @option options [Boolean] :raise_on_prerender_error Set to true to raise exceptions during server-side rendering
# Any other options are passed to the content tag, including the id.
def stream_react_component(component_name, options = {})
unless ReactOnRails::Utils.react_on_rails_pro?
raise ReactOnRails::Error,
"You must use React on Rails Pro to use the stream_react_component method."
end

if @rorp_rendering_fibers.nil?
raise ReactOnRails::Error,
"You must call stream_view_containing_react_components to render the view containing the react component"
end

rendering_fiber = Fiber.new do
stream = internal_stream_react_component(component_name, options)
stream.each_chunk do |chunk|
Fiber.yield chunk
end
end

@rorp_rendering_fibers << rendering_fiber

# return the first chunk of the fiber
# It contains the initial html of the component
# all updates will be appended to the stream sent to browser
rendering_fiber.resume
end
Comment on lines +94 to +150
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for Fiber operations.

While the implementation is solid, consider adding error handling for Fiber operations to gracefully handle failures:

 def stream_react_component(component_name, options = {})
   unless ReactOnRails::Utils.react_on_rails_pro?
     raise ReactOnRails::Error,
           "You must use React on Rails Pro to use the stream_react_component method."
   end

   if @rorp_rendering_fibers.nil?
     raise ReactOnRails::Error,
           "You must call stream_view_containing_react_components to render the view containing the react component"
   end

   rendering_fiber = Fiber.new do
+    begin
       stream = internal_stream_react_component(component_name, options)
       stream.each_chunk do |chunk|
         Fiber.yield chunk
       end
+    rescue => e
+      @rorp_rendering_fibers.delete(rendering_fiber)
+      raise ReactOnRails::Error, "Error in streaming component #{component_name}: #{e.message}"
+    end
   end

   @rorp_rendering_fibers << rendering_fiber

   # return the first chunk of the fiber
   # It contains the initial html of the component
   # all updates will be appended to the stream sent to browser
-  rendering_fiber.resume
+  begin
+    rendering_fiber.resume
+  rescue FiberError => e
+    @rorp_rendering_fibers.delete(rendering_fiber)
+    raise ReactOnRails::Error, "Failed to start streaming component #{component_name}: #{e.message}"
+  end
 end
📝 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
# Streams a server-side rendered React component using React's `renderToPipeableStream`.
# Supports React 18 features like Suspense, concurrent rendering, and selective hydration.
# Enables progressive rendering and improved performance for large components.
#
# Note: This function can only be used with React on Rails Pro.
# The view that uses this function must be rendered using the
# `stream_view_containing_react_components` method from the React on Rails Pro gem.
#
# Example of an async React component that can benefit from streaming:
#
# const AsyncComponent = async () => {
# const data = await fetchData();
# return <div>{data}</div>;
# };
#
# function App() {
# return (
# <Suspense fallback={<div>Loading...</div>}>
# <AsyncComponent />
# </Suspense>
# );
# }
#
# @param [String] component_name Name of your registered component
# @param [Hash] options Options for rendering
# @option options [Hash] :props Props to pass to the react component
# @option options [String] :dom_id DOM ID of the component container
# @option options [Hash] :html_options Options passed to content_tag
# @option options [Boolean] :prerender Set to false to disable server-side rendering
# @option options [Boolean] :trace Set to true to add extra debugging information to the HTML
# @option options [Boolean] :raise_on_prerender_error Set to true to raise exceptions during server-side rendering
# Any other options are passed to the content tag, including the id.
def stream_react_component(component_name, options = {})
unless ReactOnRails::Utils.react_on_rails_pro?
raise ReactOnRails::Error,
"You must use React on Rails Pro to use the stream_react_component method."
end
if @rorp_rendering_fibers.nil?
raise ReactOnRails::Error,
"You must call stream_view_containing_react_components to render the view containing the react component"
end
rendering_fiber = Fiber.new do
stream = internal_stream_react_component(component_name, options)
stream.each_chunk do |chunk|
Fiber.yield chunk
end
end
@rorp_rendering_fibers << rendering_fiber
# return the first chunk of the fiber
# It contains the initial html of the component
# all updates will be appended to the stream sent to browser
rendering_fiber.resume
end
def stream_react_component(component_name, options = {})
unless ReactOnRails::Utils.react_on_rails_pro?
raise ReactOnRails::Error,
"You must use React on Rails Pro to use the stream_react_component method."
end
if @rorp_rendering_fibers.nil?
raise ReactOnRails::Error,
"You must call stream_view_containing_react_components to render the view containing the react component"
end
rendering_fiber = Fiber.new do
begin
stream = internal_stream_react_component(component_name, options)
stream.each_chunk do |chunk|
Fiber.yield chunk
end
rescue => e
@rorp_rendering_fibers.delete(rendering_fiber)
raise ReactOnRails::Error, "Error in streaming component #{component_name}: #{e.message}"
end
end
@rorp_rendering_fibers << rendering_fiber
# return the first chunk of the fiber
# It contains the initial html of the component
# all updates will be appended to the stream sent to browser
begin
rendering_fiber.resume
rescue FiberError => e
@rorp_rendering_fibers.delete(rendering_fiber)
raise ReactOnRails::Error, "Failed to start streaming component #{component_name}: #{e.message}"
end
end


# react_component_hash is used to return multiple HTML strings for server rendering, such as for
# adding meta-tags to a page.
# It is exactly like react_component except for the following:
Expand Down Expand Up @@ -330,6 +388,16 @@ def load_pack_for_generated_component(react_component_name, render_options)

private

def internal_stream_react_component(component_name, options = {})
options = options.merge(stream?: true)
result = internal_react_component(component_name, options)
build_react_component_result_for_server_streamed_content(
rendered_html_stream: result[:result],
component_specification_tag: result[:tag],
render_options: result[:render_options]
)
end
Comment on lines +391 to +399
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid modifying the original options hash

In internal_stream_react_component, you are directly modifying the options hash by merging stream?: true. This might lead to unintended side effects if options is used elsewhere. Consider creating a new hash to prevent mutations to the original options.

Apply this diff to prevent side effects:

 def internal_stream_react_component(component_name, options = {})
-  options = options.merge(stream?: true)
+  stream_options = options.merge(stream?: true)
-  result = internal_react_component(component_name, options)
+  result = internal_react_component(component_name, stream_options)
   build_react_component_result_for_server_streamed_content(
     rendered_html_stream: result[:result],
     component_specification_tag: result[:tag],
     render_options: result[:render_options]
   )
 end
📝 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
def internal_stream_react_component(component_name, options = {})
options = options.merge(stream?: true)
result = internal_react_component(component_name, options)
build_react_component_result_for_server_streamed_content(
rendered_html_stream: result[:result],
component_specification_tag: result[:tag],
render_options: result[:render_options]
)
end
def internal_stream_react_component(component_name, options = {})
stream_options = options.merge(stream?: true)
result = internal_react_component(component_name, stream_options)
build_react_component_result_for_server_streamed_content(
rendered_html_stream: result[:result],
component_specification_tag: result[:tag],
render_options: result[:render_options]
)
end


def generated_components_pack_path(component_name)
"#{ReactOnRails::PackerUtils.packer_source_entry_path}/generated/#{component_name}.js"
end
Expand Down Expand Up @@ -361,6 +429,33 @@ def build_react_component_result_for_server_rendered_string(
prepend_render_rails_context(result)
end

def build_react_component_result_for_server_streamed_content(
rendered_html_stream: required("rendered_html_stream"),
component_specification_tag: required("component_specification_tag"),
render_options: required("render_options")
)
content_tag_options_html_tag = render_options.html_options[:tag] || "div"
# The component_specification_tag is appended to the first chunk
# We need to pass it early with the first chunk because it's needed in hydration
# We need to make sure that client can hydrate the app early even before all components are streamed
is_first_chunk = true
rendered_html_stream = rendered_html_stream.transform do |chunk|
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The StreamRequest and StreamDecorator classes allow us to transform the stream of http body chunks

if is_first_chunk
is_first_chunk = false
html_content = <<-HTML
#{rails_context_if_not_already_rendered}
#{component_specification_tag}
<#{content_tag_options_html_tag} id="#{render_options.dom_id}">#{chunk}</#{content_tag_options_html_tag}>
HTML
next html_content.strip
end
chunk
end

rendered_html_stream.transform(&:html_safe)
# TODO: handle console logs
end
Comment on lines +456 to +457
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 30, 2024

Choose a reason for hiding this comment

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

Reminder: Handle console logs.

The TODO comment indicates that handling console logs is pending.

Do you want me to help with handling console logs or open a GitHub issue to track this task?

Copy link
Member

Choose a reason for hiding this comment

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

help

Copy link
Contributor

Choose a reason for hiding this comment

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

@justin808, to handle console logs in the build_react_component_result_for_server_streamed_content method, you can follow these steps:

  1. Capture Console Logs: Modify the server-side rendering logic to capture console logs. This can be done by redirecting the console output to a buffer or a similar structure during the rendering process.

  2. Append Logs to Stream: Once the logs are captured, append them to the streamed content. You can do this by transforming the stream to include the logs at the appropriate point, usually after the main HTML content.

  3. Ensure Logs are Safe: Make sure that the logs are properly escaped or sanitized to prevent any security issues, such as XSS, when they are sent to the client.

Here's a conceptual example of how you might implement this:

def build_react_component_result_for_server_streamed_content(
  rendered_html_stream: required("rendered_html_stream"),
  component_specification_tag: required("component_specification_tag"),
  render_options: required("render_options")
)
  content_tag_options_html_tag = render_options.html_options[:tag] || "div"
  is_first_chunk = true
  console_logs = capture_console_logs do
    # Your rendering logic here
  end

  rendered_html_stream = rendered_html_stream.transform do |chunk|
    if is_first_chunk
      is_first_chunk = false
      html_content = <<-HTML
        #{rails_context_if_not_already_rendered}
        #{component_specification_tag}
        <#{content_tag_options_html_tag} id="#{render_options.dom_id}">#{chunk}</#{content_tag_options_html_tag}>
        <script>console.log(#{console_logs.to_json});</script>
      HTML
      next html_content.strip
    end
    chunk
  end

  rendered_html_stream.transform(&:html_safe)
end

def capture_console_logs
  # Redirect console output to a buffer
  logs = []
  original_console_log = $stdout
  $stdout = StringIO.new

  yield

  logs << $stdout.string
  $stdout = original_console_log
  logs
end

This is a basic example and may need adjustments based on your specific setup and requirements. Let me know if you need further assistance or if you'd like to open a GitHub issue to track this task.

Comment on lines +432 to +457
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

Sanitize HTML tag name to prevent XSS.

The content_tag_options_html_tag is used directly in string interpolation which could be vulnerable to XSS if the tag name contains malicious content.

-content_tag_options_html_tag = render_options.html_options[:tag] || "div"
+content_tag_options_html_tag = render_options.html_options[:tag].presence&.to_s&.strip&.downcase || "div"
+unless content_tag_options_html_tag.match?(/\A[a-z0-9]+\z/)
+  raise ReactOnRails::Error, "Invalid HTML tag name: #{content_tag_options_html_tag}"
+end
📝 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
def build_react_component_result_for_server_streamed_content(
rendered_html_stream: required("rendered_html_stream"),
component_specification_tag: required("component_specification_tag"),
render_options: required("render_options")
)
content_tag_options_html_tag = render_options.html_options[:tag] || "div"
# The component_specification_tag is appended to the first chunk
# We need to pass it early with the first chunk because it's needed in hydration
# We need to make sure that client can hydrate the app early even before all components are streamed
is_first_chunk = true
rendered_html_stream = rendered_html_stream.transform do |chunk|
if is_first_chunk
is_first_chunk = false
html_content = <<-HTML
#{rails_context_if_not_already_rendered}
#{component_specification_tag}
<#{content_tag_options_html_tag} id="#{render_options.dom_id}">#{chunk}</#{content_tag_options_html_tag}>
HTML
next html_content.strip
end
chunk
end
rendered_html_stream.transform(&:html_safe)
# TODO: handle console logs
end
def build_react_component_result_for_server_streamed_content(
rendered_html_stream: required("rendered_html_stream"),
component_specification_tag: required("component_specification_tag"),
render_options: required("render_options")
)
content_tag_options_html_tag = render_options.html_options[:tag].presence&.to_s&.strip&.downcase || "div"
unless content_tag_options_html_tag.match?(/\A[a-z0-9]+\z/)
raise ReactOnRails::Error, "Invalid HTML tag name: #{content_tag_options_html_tag}"
end
# The component_specification_tag is appended to the first chunk
# We need to pass it early with the first chunk because it's needed in hydration
# We need to make sure that client can hydrate the app early even before all components are streamed
is_first_chunk = true
rendered_html_stream = rendered_html_stream.transform do |chunk|
if is_first_chunk
is_first_chunk = false
html_content = <<-HTML
#{rails_context_if_not_already_rendered}
#{component_specification_tag}
<#{content_tag_options_html_tag} id="#{render_options.dom_id}">#{chunk}</#{content_tag_options_html_tag}>
HTML
next html_content.strip
end
chunk
end
rendered_html_stream.transform(&:html_safe)
# TODO: handle console logs
end


def build_react_component_result_for_server_rendered_hash(
server_rendered_html: required("server_rendered_html"),
component_specification_tag: required("component_specification_tag"),
Expand Down Expand Up @@ -404,20 +499,22 @@ def compose_react_component_html_with_spec_and_console(component_specification_t
HTML
end

# prepend the rails_context if not yet applied
def prepend_render_rails_context(render_value)
return render_value if @rendered_rails_context
def rails_context_if_not_already_rendered
return "" if @rendered_rails_context

data = rails_context(server_side: false)

@rendered_rails_context = true

rails_context_content = content_tag(:script,
json_safe_and_pretty(data).html_safe,
type: "application/json",
id: "js-react-on-rails-context")
content_tag(:script,
json_safe_and_pretty(data).html_safe,
type: "application/json",
id: "js-react-on-rails-context")
end
Comment on lines +502 to +513
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Memoize the Rails context script tag

In rails_context_if_not_already_rendered, you generate the Rails context script tag each time the method is called until @rendered_rails_context is set. To optimize performance, consider memoizing the generated script tag so it's only created once.

Apply this diff to memoize the script tag:

 def rails_context_if_not_already_rendered
   return @rails_context_script if @rendered_rails_context

   data = rails_context(server_side: false)
   @rendered_rails_context = true

-  content_tag(:script,
-              json_safe_and_pretty(data).html_safe,
-              type: "application/json",
-              id: "js-react-on-rails-context")
+  @rails_context_script = content_tag(:script,
+                                      json_safe_and_pretty(data).html_safe,
+                                      type: "application/json",
+                                      id: "js-react-on-rails-context")
+  @rails_context_script
 end
📝 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
def rails_context_if_not_already_rendered
return "" if @rendered_rails_context
data = rails_context(server_side: false)
@rendered_rails_context = true
rails_context_content = content_tag(:script,
json_safe_and_pretty(data).html_safe,
type: "application/json",
id: "js-react-on-rails-context")
content_tag(:script,
json_safe_and_pretty(data).html_safe,
type: "application/json",
id: "js-react-on-rails-context")
end
def rails_context_if_not_already_rendered
return @rails_context_script if @rendered_rails_context
data = rails_context(server_side: false)
@rendered_rails_context = true
@rails_context_script = content_tag(:script,
json_safe_and_pretty(data).html_safe,
type: "application/json",
id: "js-react-on-rails-context")
@rails_context_script
end


"#{rails_context_content}\n#{render_value}".html_safe
# prepend the rails_context if not yet applied
def prepend_render_rails_context(render_value)
"#{rails_context_if_not_already_rendered}\n#{render_value}".strip.html_safe
end

def internal_react_component(react_component_name, options = {})
Expand Down Expand Up @@ -520,6 +617,9 @@ def server_rendered_react_component(render_options)
js_code: js_code)
end

# TODO: handle errors for streams
return result if render_options.stream?

Comment on lines +620 to +622
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

Implement error handling for streaming components.

The TODO comment indicates that error handling for streaming components is missing. This could lead to silent failures or unclear error messages in production.

Consider implementing error handling similar to the non-streaming case. Would you like me to help design the error handling mechanism for streaming components?

if result["hasErrors"] && render_options.raise_on_prerender_error
# We caught this exception on our backtrace handler
raise ReactOnRails::PrerenderError.new(component_name: react_component_name,
Expand Down
4 changes: 4 additions & 0 deletions lib/react_on_rails/react_component/render_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ def set_option(key, value)
options[key] = value
end

def stream?
options[:stream?]
end
Comment on lines +110 to +112
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using retrieve_configuration_value_for for consistency

The new stream? method is a good addition for checking if streaming is enabled. However, to maintain consistency with other option retrieval methods in this class, consider using the retrieve_configuration_value_for method. This would allow for a default value to be set in the configuration if not provided in the options.

Also, could you clarify why this method is public while some other option retrieval methods are private? If there's no specific reason for it to be public, consider making it private for encapsulation.

Here's a suggested implementation:

private

def stream?
  retrieve_configuration_value_for(:stream?)
end

This assumes you'll add a corresponding stream? option to the ReactOnRails.configuration object. If that's not the case, please disregard this suggestion.


private

attr_reader :options
Expand Down
Loading
Loading