Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }

ruby "3.1.2"

gem "react_on_rails", "14.0.0"
gem "react_on_rails", path: '../react_on_rails'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You have this change locally only

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

it's just a demo to prove 1508, and it depends on the branch [react_on_hotwired](theforestvn88:react_over_hotwired) which has not been merged so it linked to the locally build of that branch.

anyway, i don't think this PR should be merged so we don't need to be serious about this.

gem "shakapacker", "7.2.1"

# Bundle edge Rails instead: gem "rails", github: "rails/rails"
Expand Down
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 test_1508
@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 %>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You are missing EOL on new files.

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/test_1508.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 %>
<%= render partial: 'form_1508' %>
<% end %>
8 changes: 8 additions & 0 deletions app/views/pages/hotwired.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<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,120 @@
// eslint-disable-next-line max-classes-per-file
import React from 'react';
import request from 'axios';
import Immutable from 'immutable';
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 { defaultMessages, 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');
}

componentDidMount() {
}

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)
.then(r => r.data)
.then(html => {
Turbo.renderStreamMessage(html)
})
.then(() => {
const { $$comments } = this.state;
const $$comment = Immutable.fromJS(comment);

this.setState({
$$comments: $$comments.unshift($$comment),
submitCommentError: null,
isSaving: false,
});
})
.catch((error) => {
this.setState({
submitCommentError: error,
isSaving: false,
});
});
}

render() {
const { handleSetLocale, locale, intl } = 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,87 @@
// eslint-disable-next-line max-classes-per-file
import React from 'react';
import Immutable from 'immutable';
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 { 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),
};
}

componentDidMount() {
}

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
Loading