Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def create
format.html { redirect_to @comment, notice: I18n.t(:comment_was_successfully_created) }
end
format.json { render :show, status: :created, location: @comment }
format.turbo_stream
else
if turbo_frame_request?
format.html
Expand Down Expand Up @@ -99,6 +100,10 @@ def inline_form
end
end

def test1508
@comment = Comment.new
end

private

def set_comments
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ def simple; end

def rescript; end

def hotwired
@props = comments_json_string
end
Comment on lines +41 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Mirror the existing render_to_string response handling here.

Line 42 calls comments_json_string, which uses render_to_string. The controller comment above and the index/no_router actions both show that an explicit HTML response is needed afterward. Without render_html, pages#hotwired can render/negotiation-handle differently from the other controller-driven pages.

♻️ Suggested fix
   def hotwired
     `@props` = comments_json_string
+    render_html
   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 hotwired
@props = comments_json_string
end
def hotwired
`@props` = comments_json_string
render_html
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/controllers/pages_controller.rb` around lines 41 - 43, The hotwired
action sets `@props` by calling comments_json_string (which uses render_to_string)
but does not follow with the same explicit HTML response handling used by index
and no_router; update the hotwired method to mirror their flow by invoking the
same render_html call after setting `@props` so the controller returns an explicit
HTML response (i.e., keep hotwired, comments_json_string, and then call
render_html just like index/no_router).


private

def set_comments
Expand Down
30 changes: 30 additions & 0 deletions app/views/comments/_form_1508.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<%= form_for(@comment, html: { class: "flex flex-col gap-4" }) do |f| %>
<% if @comment.errors.any? %>
<div id="error_explanation" class="prose bg-pink-100 p-4 mb-4 border border-pink-200 rounded text-red-800 prose-strong:text-red-800 prose-ul:my-1">
<h2><%= pluralize(@comment.errors.count, "error") %> prohibited this comment from being saved:</h2>

<ul>
<% @comment.errors.full_messages.each do |message| %>
<li><%= message %></li>
<% end %>
</ul>
</div>
<% end %>

<div class="field">
<%= f.label :author, 'Your Name' %><br>
<%= f.text_field :author, class: "px-3 py-1 leading-4 border border-gray-300 rounded" %>
</div>
<div class="field">
<%= f.label :text, 'Say something using markdown...' %><br>
<%= f.text_area :text, class: "px-3 py-1 leading-4 border border-gray-300 rounded" %>
</div>
<div class="actions">
<%= f.submit 'Post', class: "self-start px-3 py-1 font-semibold border-0 rounded text-sky-50 bg-sky-600 hover:bg-sky-800 cursor-pointer" %>
</div>

<h1>Below is a react component which render inside a form (that will proof #1508 is fixed), You can turn-off the flag `force_load` and this below form will not show. </h1>
<div class="mx-10 border border-red-600 rounded">
<%= react_component('HotwiredCommentForm', props: { }, prerender: false, force_load: true) %>
</div>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
<% end %>
10 changes: 10 additions & 0 deletions app/views/comments/create.turbo_stream.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<%= turbo_stream.prepend "comments" do %>
<div>
<h2 class="text-blue-800"><%= @comment.author %></h2>
<span><%= markdown_to_html(@comment.text) %></span>
</div>
<% end %>
Comment on lines +1 to +6
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't prepend into a React-owned list.

client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen.jsx renders CommentList from this.state.$$comments only. This stream inserts raw HTML under #comments without updating React state/store, so the next React render—changing locale is enough—will drop or reorder the streamed comment and leave React reconciling stale DOM. Route the new comment through React state/Redux, or replace the whole React root with fresh props instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/comments/create.turbo_stream.erb` around lines 1 - 6, The turbo
stream in app/views/comments/create.turbo_stream.erb is prepending raw HTML into
the DOM (`#comments`) which conflicts with the React-managed CommentList in
client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen.jsx
(which renders from this.state.$$comments), so instead of inserting HTML via
turbo_stream.prepend and markdown_to_html(`@comment.text`), emit an event or
invoke the client-side API to add the new comment into React/Redux state (e.g.,
dispatch an action to update this.state.$$comments or call a provided addComment
handler on HotwiredCommentScreen) or return JSON/props that cause the React root
to rehydrate with fresh props so React remains the single source of truth.


<%= turbo_stream.update "comment-form" do %>
<%= link_to "New Comment", new_comment_path, data: { turbo_stream: true } %>
<% end %>
7 changes: 7 additions & 0 deletions app/views/comments/new.turbo_stream.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<%= turbo_stream.update "comment-form" do %>
<div class="p-5 m-5 border border-red-600 rounded">
<h2>New Comment</h2>

<%= react_component("HotwiredCommentForm", props: {}, prerender: false, force_load: true) %>
</div>
<% end %>
3 changes: 3 additions & 0 deletions app/views/comments/test1508.turbo_stream.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= turbo_stream.replace dom_id(@comment), target: "_top" do %>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

turbo-rails turbo_stream.replacehelper supported options, and whether_top is only meaningful for Turbo Frame navigation.

💡 Result:

The turbo_stream.replace helper in turbo-rails supports the following main options, based on official documentation and source code: - target: Required DOM ID (string, model/record, or array) of the element to replace. - Rendering options for content: partial:, locals: {}, block content &block, or direct content string/HTML. - method: :morph (or "morph" string): Uses morphing instead of direct replacement/innerHTML for the replace action. Examples from docs: <%= turbo_stream.replace "clearance_5", "

Replace...
" %> <%= turbo_stream.replace clearance, partial: "clearances/clearance", locals: { title: "Hello" } %> <%= turbo_stream.replace clearance, "
Morph the dom target
", method: :morph %> Additional attributes like attributes can be passed to the underlying tag via turbo_stream_action_tag, but _top is not a supported option for turbo_stream.replace. The _top value is a special data-turbo-frame target for Turbo Frames (e.g., data: { turbo_frame: "_top" } on links/forms), used to break out of nested frames and navigate the top-level page. It has no meaning or support in Turbo Streams actions like replace, which target arbitrary DOM elements by ID/CSS via target/targets attributes, independent of frame navigation.

Citations:


Remove unsupported target: "_top" parameter from turbo_stream.replace.

Line 1 passes target: "_top" to turbo_stream.replace, but _top is not a supported option for this method. The target parameter for Turbo Stream actions identifies the DOM element to replace by ID, while _top is a Turbo Frame navigation directive used to break out of nested frames (e.g., data-turbo-frame="_top" on links). This unsupported parameter has no effect and creates confusion about the code's intent.

♻️ Suggested fix
-<%= turbo_stream.replace dom_id(`@comment`), target: "_top" do %>
+<%= turbo_stream.replace dom_id(`@comment`) do %>
📝 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
<%= turbo_stream.replace dom_id(@comment), target: "_top" do %>
<%= turbo_stream.replace dom_id(`@comment`) do %>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/comments/test1508.turbo_stream.erb` at line 1, In the turbo stream
call, remove the unsupported named argument `target: "_top"` from the
`turbo_stream.replace` invocation so the helper is called with only the
identifier (e.g., `turbo_stream.replace dom_id(`@comment`) do`), leaving
`dom_id(`@comment`)` as the replacement target; if the original intent was to
break out of nested Turbo Frames, implement that via a `data-turbo-frame="_top"`
attribute on the triggering link or form instead of passing `target: "_top"` to
`turbo_stream.replace`.

<%= render partial: "form_1508" %>
<% end %>
7 changes: 7 additions & 0 deletions app/views/pages/hotwired.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<h2>Demo React Over Hotwired</h2>

<%= react_component("HotwiredCommentScreen", props: @props, prerender: false) %>

<%= turbo_frame_tag "new_comment" do %>
<%= link_to "proof that #1508 fixed", "/test_1508", data: { turbo_stream: true } %>
<% end %>
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export default class CommentList extends BaseComponent {
<div>
{this.errorWarning()}

<TransitionGroup className="commentList" component="div">
<TransitionGroup id="comments" className="commentList" component="div">
{commentNodes}
</TransitionGroup>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// eslint-disable-next-line max-classes-per-file
import React from 'react';
import request from 'axios';
import _ from 'lodash';
import ReactOnRails from 'react-on-rails';
import { IntlProvider, injectIntl } from 'react-intl';
import BaseComponent from 'libs/components/BaseComponent';
import SelectLanguage from 'libs/i18n/selectLanguage';
import { defaultLocale } from 'libs/i18n/default';
import { translations } from 'libs/i18n/translations';

import { Turbo } from '@hotwired/turbo-rails';
import CommentForm from '../CommentBox/CommentForm/CommentForm';
import css from './HotwiredCommentScreen.module.scss';

class HotwiredCommentForm extends BaseComponent {
constructor(props) {
super(props);

this.state = {
isSaving: false,
submitCommentError: null,
};

_.bindAll(this, 'handleCommentSubmit');
}

handleCommentSubmit(comment) {
this.setState({ isSaving: true });

const requestConfig = {
responseType: 'text/vnd.turbo-stream.html',
headers: ReactOnRails.authenticityHeaders(),
};

return request
.post('/comments.turbo_stream', { comment }, requestConfig)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Axios browser adapter valid responseType values, and how to request a Turbo Stream HTML response (text/vnd.turbo-stream.html) from Axios.

💡 Result:

Axios browser adapter (XHR) valid responseType values are: '' (default), 'arraybuffer', 'blob', 'document', 'json', 'text'. Note that 'stream' is listed in docs but only works in Node.js (http adapter), not browser XHR. To request a Turbo Stream HTML response (text/vnd.turbo-stream.html) from Axios in browser: 1. Set the Accept header to include the Turbo Stream MIME type. 2. Use responseType: 'text' (or omit, defaults to text/json handling but works for HTML). 3. After receiving response, if Turbo is loaded, call Turbo.renderStreamMessage(response.data) to process the turbo-stream elements. Example: axios.get('/your-endpoint', { headers: { 'Accept': 'text/vnd.turbo-stream.html, text/html, application/xhtml+xml' }, responseType: 'text' }) .then(response => { if (response.headers['content-type']?.includes('turbo-stream')) { Turbo.renderStreamMessage(response.data); } else { // handle as regular HTML/text } }); Default Axios Accept is 'application/json, text/plain, /', so must override for server to return turbo-stream format.

Citations:


Use Accept, not responseType, for the Turbo Stream MIME.

Line 32 passes a MIME type where Axios expects transport values like text or json. That leaves the request config non-standard and hides the real content-negotiation requirement. Send Accept: text/vnd.turbo-stream.html and keep responseType: 'text'.

♻️ Suggested fix
     const requestConfig = {
-      responseType: 'text/vnd.turbo-stream.html',
-      headers: ReactOnRails.authenticityHeaders(),
+      responseType: 'text',
+      headers: {
+        Accept: 'text/vnd.turbo-stream.html',
+        ...ReactOnRails.authenticityHeaders(),
+      },
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentForm.jsx`
around lines 31 - 37, The request config is using responseType to send the Turbo
Stream MIME which is incorrect; update the requestConfig used when calling
request.post (the call to .post('/comments.turbo_stream', { comment },
requestConfig)) to set headers via ReactOnRails.authenticityHeaders() plus an
Accept header of 'text/vnd.turbo-stream.html', and keep responseType as 'text'
(or omit if default) so Axios receives a proper transport responseType while the
server gets the correct Accept content-negotiation header.

.then(r => r.data)
.then(html => {
Turbo.renderStreamMessage(html);
})
.then(() => {
this.setState({
submitCommentError: null,
isSaving: false,
});
})
.catch((error) => {
this.setState({
submitCommentError: error,
isSaving: false,
});
});
}

render() {
const { handleSetLocale, locale } = this.props;
const cssTransitionGroupClassNames = {
enter: css.elementEnter,
enterActive: css.elementEnterActive,
exit: css.elementLeave,
exitActive: css.elementLeaveActive,
};

return (
<div className="commentBox prose max-w-none prose-a:text-sky-700 prose-li:my-0">
{SelectLanguage(handleSetLocale, locale)}

<CommentForm
isSaving={this.state.isSaving}
actions={{ submitComment: this.handleCommentSubmit }}
error={{ error: this.state.submitCommentError, nodeRef: React.createRef(null) }}
cssTransitionGroupClassNames={cssTransitionGroupClassNames}
/>
</div>
);
}
}

export default class I18nWrapper extends BaseComponent {
constructor(props) {
super(props);

this.state = {
locale: defaultLocale,
};

_.bindAll(this, 'handleSetLocale');
}

handleSetLocale(locale) {
this.setState({ locale });
}

render() {
const { locale } = this.state;
const messages = translations[locale];
const InjectedHotwiredCommentForm = injectIntl(HotwiredCommentForm);

return (
<IntlProvider locale={locale} key={locale} messages={messages}>
<InjectedHotwiredCommentForm
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...this.props}
locale={locale}
handleSetLocale={this.handleSetLocale}
/>
</IntlProvider>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// eslint-disable-next-line max-classes-per-file
import React from 'react';
import Immutable from 'immutable';
import _ from 'lodash';
import { IntlProvider, injectIntl } from 'react-intl';
import BaseComponent from 'libs/components/BaseComponent';
import SelectLanguage from 'libs/i18n/selectLanguage';
import { defaultMessages, defaultLocale } from 'libs/i18n/default';
import { translations } from 'libs/i18n/translations';

import CommentList from '../CommentBox/CommentList/CommentList';
import css from './HotwiredCommentScreen.module.scss';

class HotwiredCommentScreen extends BaseComponent {
constructor(props) {
super(props);

this.state = {
$$comments: Immutable.fromJS(props.comments),
};
}

render() {
const { handleSetLocale, locale, intl } = this.props;
const { formatMessage } = intl;
const cssTransitionGroupClassNames = {
enter: css.elementEnter,
enterActive: css.elementEnterActive,
exit: css.elementLeave,
exitActive: css.elementLeaveActive,
};

return (
<div className="commentBox prose max-w-none prose-a:text-sky-700 prose-li:my-0">
<turbo-frame id="comment-box">
<h2>{formatMessage(defaultMessages.comments)}</h2>
{SelectLanguage(handleSetLocale, locale)}

<CommentList
$$comments={this.state.$$comments}
cssTransitionGroupClassNames={cssTransitionGroupClassNames}
/>

<div id="comment-form">
<a data-turbo-stream="true" href="/comments/new">New Comment</a>
</div>
</turbo-frame>
</div>
);
}
}

export default class I18nWrapper extends BaseComponent {
constructor(props) {
super(props);

this.state = {
locale: defaultLocale,
};

_.bindAll(this, 'handleSetLocale');
}

handleSetLocale(locale) {
this.setState({ locale });
}

render() {
const { locale } = this.state;
const messages = translations[locale];
const InjectedHotwiredCommentScreen = injectIntl(HotwiredCommentScreen);

return (
<IntlProvider locale={locale} key={locale} messages={messages}>
<InjectedHotwiredCommentScreen
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...this.props}
locale={locale}
handleSetLocale={this.handleSetLocale}
/>
</IntlProvider>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
.elementEnter {
opacity: 0.01;

&.elementEnterActive {
opacity: 1;
transition: opacity $animation-duration ease-in;
}
}

.elementLeave {
opacity: 1;

&.elementLeaveActive {
opacity: 0.01;
transition: opacity $animation-duration ease-in;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ function NavigationBar(props) {
Simple React
</a>
</li>
<li className={classNames({ 'bg-yellow-100': pathname === paths.HOTWIRED_PATH })}>
<a
className="px-2 py-4 w-full inline-block text-gray-500 hover:text-gray-700"
href={paths.HOTWIRED_PATH}
>
HotWired
</a>
</li>
<li className={classNames({ 'bg-yellow-100': pathname === paths.STIMULUS_PATH })}>
<a
className="px-2 py-4 w-full inline-block text-gray-500 hover:text-gray-700"
Expand Down
1 change: 1 addition & 0 deletions client/app/bundles/comments/constants/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ export const REACT_ROUTER_PATH = '/react-router';
export const NO_ROUTER_PATH = '/no-router';
export const RESCRIPT_PATH = '/rescript';
export const SIMPLE_REACT_PATH = '/simple';
export const HOTWIRED_PATH = '/hotwired';
export const STIMULUS_PATH = '/stimulus';
export const RAILS_PATH = '/comments';
6 changes: 6 additions & 0 deletions client/app/packs/client-bundle.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import ReactOnRails from 'react-on-rails';
import '@hotwired/turbo-rails';
// eslint-disable-next-line import/no-webpack-loader-syntax
import 'expose-loader?exposes=$,jQuery!jquery';
import 'jquery-ujs';
Expand All @@ -12,6 +13,9 @@ import NavigationBarApp from '../bundles/comments/startup/NavigationBarApp';
import Footer from '../bundles/comments/components/Footer/Footer';
import RescriptShow from '../bundles/comments/rescript/ReScriptShow.bs.js';

import HotwiredCommentForm from '../bundles/comments/components/HotwiredCommentScreen/HotwiredCommentForm';
import HotwiredCommentScreen from '../bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen';

import '../assets/styles/application';

ReactOnRails.setOptions({
Expand All @@ -25,6 +29,8 @@ ReactOnRails.register({
RouterApp,
NavigationBarApp,
SimpleCommentScreen,
HotwiredCommentScreen,
HotwiredCommentForm,
Footer,
RescriptShow,
});
Expand Down
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
get "simple", to: "pages#simple"
get "rescript", to: "pages#rescript"
get "no-router", to: "pages#no_router"
get "hotwired", to: "pages#hotwired"

# React Router needs a wildcard
get "react-router(/*all)", to: "pages#index"
Expand All @@ -22,4 +23,5 @@
get "comment-list", to: "comments#comment_list"
resources :comments
mount ActionCable.server => "/cable"
get "test_1508", to: "comments#test1508", format: :turbo_stream
end