demo react-over-hotwired and proof #1508 fixed #592
Conversation
WalkthroughThe update introduces significant enhancements to the integration of React and Hotwired in the Rails application. Key changes include updating the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- Gemfile (1 hunks)
- app/controllers/comments_controller.rb (2 hunks)
- app/controllers/pages_controller.rb (1 hunks)
- app/views/comments/_form_1508.html.erb (1 hunks)
- app/views/comments/create.turbo_stream.erb (1 hunks)
- app/views/comments/new.turbo_stream.erb (1 hunks)
- app/views/comments/test_1508.turbo_stream.erb (1 hunks)
- app/views/pages/hotwired.html.erb (1 hunks)
- client/app/bundles/comments/components/CommentBox/CommentList/CommentList.jsx (1 hunks)
- client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentForm.jsx (1 hunks)
- client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen.jsx (1 hunks)
- client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen.module.scss (1 hunks)
- client/app/bundles/comments/components/NavigationBar/NavigationBar.jsx (1 hunks)
- client/app/bundles/comments/constants/paths.js (1 hunks)
- client/app/packs/client-bundle.js (3 hunks)
- config/routes.rb (2 hunks)
Files skipped from review due to trivial changes (8)
- app/views/comments/_form_1508.html.erb
- app/views/comments/create.turbo_stream.erb
- app/views/comments/new.turbo_stream.erb
- app/views/comments/test_1508.turbo_stream.erb
- app/views/pages/hotwired.html.erb
- client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen.module.scss
- client/app/bundles/comments/components/NavigationBar/NavigationBar.jsx
- client/app/bundles/comments/constants/paths.js
Additional comments not posted (14)
config/routes.rb (2)
14-14: The route for "hotwired" is correctly set up to point to thePagesController#hotwired.
28-28: The route for "test_1508" is correctly configured to useturbo_stream, aligning with the PR's objectives to test issue #1508.client/app/packs/client-bundle.js (4)
2-2: Import of@hotwired/turbo-railsis correctly added to support Turbo Streams integration.
16-17: Imports forHotwiredCommentFormandHotwiredCommentScreenare correctly added for component registration.
32-32: Registration ofHotwiredCommentScreenis correctly done, making it available for use in Rails views.
33-33: Registration ofHotwiredCommentFormis correctly done, making it available for use in Rails views.app/controllers/pages_controller.rb (1)
41-43: Thehotwiredmethod is correctly implemented to set@propsfor demonstrating React components over Rails 7 Hotwired.client/app/bundles/comments/components/CommentBox/CommentList/CommentList.jsx (1)
81-81: The change in theclassNameattribute from "commentList" to "comments" is appropriate and aligns with the enhanced functionality and user experience objectives.client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentScreen.jsx (2)
14-53: TheHotwiredCommentScreencomponent is correctly implemented with internationalization support and comment display functionality, aligning with the PR's objectives.
56-87: TheI18nWrappercomponent is correctly implemented to provide internationalization context forHotwiredCommentScreen, enhancing the application's internationalization strategy.app/controllers/comments_controller.rb (2)
36-36: The addition offormat.turbo_streamin thecreateaction is correctly implemented to support Turbo Streams functionality.
103-105: Thetest_1508action is correctly implemented to initialize a newCommentobject for testing the fix for issue #1508.Gemfile (1)
8-8: Ensure the local path forreact_on_railsis intended for development only.This change should be reverted before merging into production to ensure stability and version control. Please confirm that this is the intended usage.
client/app/bundles/comments/components/HotwiredCommentScreen/HotwiredCommentForm.jsx (1)
89-120: Implementation ofI18nWrapperlooks good.This component correctly sets up internationalization for the
HotwiredCommentForm. Good use of React's context API withIntlProvider.
| submitCommentError: null, | ||
| }; | ||
|
|
||
| _.bindAll(this, 'handleCommentSubmit'); |
There was a problem hiding this comment.
Consider using arrow functions for method bindings instead of lodash's bindAll.
Arrow functions can make the code cleaner and reduce the dependency on lodash for this purpose. Here's how you can refactor the handleCommentSubmit method:
handleCommentSubmit = (comment) => {
// method body remains the same
}
|
Hi @theforestvn88 can you update your branch? |
| @@ -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' | |||
There was a problem hiding this comment.
You have this change locally only
There was a problem hiding this comment.
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.
|
|
||
| <%= turbo_stream.update "comment-form" do %> | ||
| <%= link_to "New Comment", new_comment_path, data: {turbo_stream: true} %> | ||
| <% end %> |
There was a problem hiding this comment.
You are missing EOL on new files.
| @@ -22,4 +23,7 @@ | |||
| get "comment-list", to: "comments#comment_list" | |||
| resources :comments | |||
| mount ActionCable.server => "/cable" | |||
|
|
|||
|
|
|||
| get "test_1508", to: "comments#test_1508", format: :turbo_stream | |||
This is a demo for my pull request react_over_hotwired and also proof that the react_on_rails issue 1508 is fixed.
to run this pr:
demo on new tab
Hotwiredtab, with a link at the bottom to show the proof of 1508.about the issue 1508: i setup the same as the reproduction, the car model replaced by the comment model. The react-component inside the form will show with the flag
force_load = truethat proof my solution could solve this issue. (you can turn-off the flagforce_loadto see that the nested react-component will not show).the main purpose of this pull request is to demo how to work with react-components over rails 7 hotwired, but what i have done so far is only sending new react-components to client side over hotwired. How about the existed react-components ?
For example, in this demo i setup 2 components: comment-list and comment-form, comment-form will send over hotwired and it looks good, but when a comment is created, the created comment will be sent to client over hotwired and prepend
directlyto comments list on comment-list component, this is a normal hotwired flow --> but with react, it very smell, in my opinion. We should not inject a view data into dom-elements hierarchy under the react-component control, right ?of course with json response or set up ActionCable (which has it's cost), we could do that by dispatching new data (as you guys already did on Stimulus page demo), but what i try to do here is with turbo-stream response.
the only solution come to my mind (so far) is sending a script that dispatching append/prepend/delete/modify... data through the redux store.
so what do you think ?
This change is