-
Notifications
You must be signed in to change notification settings - Fork 380
Migrate to react on rails auto-registration #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9721c36
d12c40d
5854552
6a2e29b
71a60b2
027bfd7
a9c10e9
0d04fa2
fe066fb
9b9bb86
d6d123e
81f84aa
2547b80
9d2bf5e
ee5bf38
ab31233
4daa2ec
351824b
58846b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# TODO: Fix Turbo Warning in Future PR | ||
|
||
## Issue | ||
There's a console warning about Turbo being loaded from within the `<body>` element instead of the `<head>`. | ||
|
||
## Root Cause | ||
Conflicting requirements between three systems: | ||
1. **Turbo** - Wants to be loaded in `<head>` to avoid re-evaluation on page changes | ||
2. **Shakapacker** - Requires all `append_javascript_pack_tag` calls to happen before the final `javascript_pack_tag` | ||
3. **React on Rails** - The `react_component` helper internally calls `append_javascript_pack_tag` when rendering components in the body | ||
|
||
## Attempted Solutions That Failed | ||
1. Moving `javascript_pack_tag` to head - Breaks because `react_component` calls come after it | ||
2. Using `data-turbo-suppress-warning` - Doesn't properly suppress the warning | ||
|
||
## Potential Future Solutions | ||
1. Extract Turbo into a separate pack from stimulus-bundle and load it in the head | ||
2. Use `prepend_javascript_pack_tag` instead of `append` for component registration | ||
3. Configure React on Rails v16 to use a different component loading strategy | ||
4. Investigate if the auto-registration feature has a different recommended pack loading pattern | ||
|
||
## Current State | ||
The application works correctly with the pack tags at the end of the body. The Turbo warning is cosmetic and doesn't affect functionality. | ||
|
||
## References | ||
- PR #649: Initial v16 migration | ||
- Shakapacker docs: https://github.com/shakacode/shakapacker#view-helper-append_javascript_pack_tag | ||
- Turbo docs: https://turbo.hotwired.dev/handbook/building#working-with-script-elements | ||
- React on Rails v16 docs: https://www.shakacode.com/react-on-rails/docs/ |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,13 +5,9 @@ | |||||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||||||
<title>RailsReactTutorial</title> | ||||||
|
||||||
<%= stylesheet_pack_tag('client-bundle', | ||||||
media: 'all', | ||||||
'data-turbolinks-track': true) %> | ||||||
|
||||||
<%= javascript_pack_tag('client-bundle', | ||||||
'data-turbolinks-track': true, | ||||||
defer: true) %> | ||||||
<%= append_stylesheet_pack_tag('stimulus-bundle') %> | ||||||
<%= append_javascript_pack_tag('stimulus-bundle') %> | ||||||
<%= append_javascript_pack_tag('stores-registration') %> | ||||||
|
||||||
<%= csrf_meta_tags %> | ||||||
</head> | ||||||
|
@@ -24,6 +20,9 @@ | |||||
|
||||||
<%= react_component "Footer" %> | ||||||
|
||||||
<%= stylesheet_pack_tag(media: 'all', 'data-turbolinks-track': true) %> | ||||||
<%= javascript_pack_tag('data-turbolinks-track': true, defer: true) %> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
|
||||||
<!-- This is a placeholder for ReactOnRails to know where to render the store props for | ||||||
client side hydration --> | ||||||
<%= redux_store_hydration_data %> | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,129 @@ | ||||||||
// Generated by ReScript, PLEASE EDIT WITH CARE | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment indicates this is a generated file, but it's being committed to version control. Generated files should typically be in .gitignore unless they're meant to be manually edited. Consider clarifying whether this file should be version controlled or generated during build.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment suggests the file is auto-generated and should not be edited, but it's being added to version control in a PR. Auto-generated files should typically be excluded from version control or the warning should be removed if manual editing is expected.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||
|
||||||||
import * as React from "react"; | ||||||||
import * as Header from "../../Header/Header.bs.js"; | ||||||||
import * as Actions from "../../Actions/Actions.bs.js"; | ||||||||
import * as AlertError from "../../CommentList/AlertError/AlertError.bs.js"; | ||||||||
import * as ActionCable from "../../bindings/ActionCable.bs.js"; | ||||||||
import * as CommentForm from "../../CommentForm/CommentForm.bs.js"; | ||||||||
import * as CommentList from "../../CommentList/CommentList.bs.js"; | ||||||||
import * as JsxRuntime from "react/jsx-runtime"; | ||||||||
|
||||||||
function reducer(param, action) { | ||||||||
if (typeof action !== "object") { | ||||||||
return { | ||||||||
commentsFetchStatus: "FetchError" | ||||||||
}; | ||||||||
} else { | ||||||||
return { | ||||||||
commentsFetchStatus: { | ||||||||
TAG: "CommentsFetched", | ||||||||
_0: action._0 | ||||||||
} | ||||||||
}; | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
function ReScriptShow$default(props) { | ||||||||
var match = React.useReducer(reducer, { | ||||||||
commentsFetchStatus: { | ||||||||
TAG: "CommentsFetched", | ||||||||
_0: [] | ||||||||
} | ||||||||
}); | ||||||||
var dispatch = match[1]; | ||||||||
var fetchData = async function () { | ||||||||
var comments = await Actions.Fetch.fetchComments(); | ||||||||
if (comments.TAG === "Ok") { | ||||||||
return dispatch({ | ||||||||
TAG: "SetComments", | ||||||||
_0: comments._0 | ||||||||
}); | ||||||||
} else { | ||||||||
return dispatch("SetFetchError"); | ||||||||
} | ||||||||
}; | ||||||||
var subscribeToCommentsChannel = function () { | ||||||||
return ActionCable.subscribeConsumer("CommentsChannel", { | ||||||||
connected: (function () { | ||||||||
console.log("Connected"); | ||||||||
}), | ||||||||
received: (function (data) { | ||||||||
dispatch({ | ||||||||
TAG: "SetComments", | ||||||||
_0: [data] | ||||||||
}); | ||||||||
}), | ||||||||
disconnected: (function () { | ||||||||
console.log("Disconnected"); | ||||||||
}) | ||||||||
}); | ||||||||
}; | ||||||||
React.useEffect((function () { | ||||||||
var scription = subscribeToCommentsChannel(); | ||||||||
return (function () { | ||||||||
scription.unsubscribe(); | ||||||||
}); | ||||||||
}), []); | ||||||||
React.useEffect((function () { | ||||||||
fetchData(); | ||||||||
}), []); | ||||||||
var comments = match[0].commentsFetchStatus; | ||||||||
var tmp; | ||||||||
tmp = typeof comments !== "object" ? JsxRuntime.jsx(AlertError.make, { | ||||||||
errorMsg: "Can't fetch the comments!" | ||||||||
}) : JsxRuntime.jsx(CommentList.make, { | ||||||||
comments: comments._0 | ||||||||
}); | ||||||||
return JsxRuntime.jsxs("div", { | ||||||||
children: [ | ||||||||
JsxRuntime.jsxs("h2", { | ||||||||
children: [ | ||||||||
"Rescript + Rails Backend (with ", | ||||||||
JsxRuntime.jsx("a", { | ||||||||
children: "react_on_rails gem", | ||||||||
href: "https://github.com/shakacode/react_on_rails" | ||||||||
}), | ||||||||
")" | ||||||||
] | ||||||||
}), | ||||||||
JsxRuntime.jsx(Header.make, {}), | ||||||||
JsxRuntime.jsxs("div", { | ||||||||
children: [ | ||||||||
JsxRuntime.jsx("h2", { | ||||||||
children: "Comments" | ||||||||
}), | ||||||||
JsxRuntime.jsxs("ul", { | ||||||||
children: [ | ||||||||
JsxRuntime.jsx("li", { | ||||||||
children: "Text supports Github Flavored Markdown." | ||||||||
}), | ||||||||
JsxRuntime.jsx("li", { | ||||||||
children: "Comments older than 24 hours are deleted." | ||||||||
}), | ||||||||
JsxRuntime.jsx("li", { | ||||||||
children: "Name is preserved. Text is reset, between submits" | ||||||||
}), | ||||||||
JsxRuntime.jsx("li", { | ||||||||
children: "To see Action Cable instantly update two browsers, open two browsers and submit a comment!" | ||||||||
}) | ||||||||
] | ||||||||
}), | ||||||||
JsxRuntime.jsx(CommentForm.make, { | ||||||||
fetchData: fetchData | ||||||||
}), | ||||||||
tmp | ||||||||
], | ||||||||
className: "prose max-w-none prose-a:text-sky-700 prose-li:my-0" | ||||||||
}) | ||||||||
] | ||||||||
}); | ||||||||
} | ||||||||
|
||||||||
var $$default = ReScriptShow$default; | ||||||||
|
||||||||
export { | ||||||||
reducer , | ||||||||
$$default as default, | ||||||||
} | ||||||||
/* react Not a pure module */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
// Compare to ../ServerRouterApp.jsx | ||
// Compare to ./RouterApp.server.jsx | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment references './RouterApp.server.jsx' but the actual file appears to be located at the same directory level. Verify that this relative path is correct for the new directory structure. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
import { Provider } from 'react-redux'; | ||
import React from 'react'; | ||
import ReactOnRails from 'react-on-rails'; | ||
import { BrowserRouter } from 'react-router-dom'; | ||
import routes from '../routes/routes.jsx'; | ||
import routes from '../../../routes/routes.jsx'; | ||
|
||
function ClientRouterApp(_props) { | ||
const store = ReactOnRails.getStore('routerCommentsStore'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
// Compare to ../ClientRouterApp.jsx | ||
// Compare to ./RouterApp.client.jsx | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment references './RouterApp.client.jsx' but the actual file appears to be located at the same directory level. Verify that this relative path is correct for the new directory structure. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
import { Provider } from 'react-redux'; | ||
import React from 'react'; | ||
import { StaticRouter } from 'react-router-dom/server'; | ||
import ReactOnRails from 'react-on-rails'; | ||
import routes from '../routes/routes.jsx'; | ||
import routes from '../../../routes/routes.jsx'; | ||
|
||
function ServerRouterApp(_props, railsContext) { | ||
const store = ReactOnRails.getStore('routerCommentsStore'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
// Example of React + Redux | ||
import ReactOnRails from 'react-on-rails'; | ||
|
||
import App from './App'; | ||
import RouterApp from './ServerRouterApp'; | ||
import SimpleCommentScreen from '../components/SimpleCommentScreen/SimpleCommentScreen'; | ||
import NavigationBarApp from './NavigationBarApp'; | ||
import App from './App/ror_components/App'; | ||
import RouterApp from './RouterApp/ror_components/RouterApp.server'; | ||
import SimpleCommentScreen from '../components/SimpleCommentScreen/ror_components/SimpleCommentScreen'; | ||
import NavigationBarApp from './NavigationBarApp/ror_components/NavigationBarApp'; | ||
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chainIf using auto-registration, this file should not manually register components/stores. With v16 generated packs, components under ror_components are auto-registered. Keeping manual ReactOnRails.register and registerStore here can cause duplicate registrations if this file is ever imported. Prefer removing manual registrations (and the store imports) or delete this file entirely. If you need server-only variants (e.g., RouterApp.server), let the generator handle it. Please run: If unused, remove this file; if used for a special case, strip store registration and rely on packs/stores-registration.js. Based on learnings. Also applies to: 10-10 🏁 Script executed: #!/bin/bash
# 1) Verify whether this file is referenced anywhere
rg -nP 'bundles/comments/startup/serverRegistration\.(jsx|tsx|js|ts)'
# 2) List all store registrations to spot duplicates with packs/stores-registration
rg -nP 'ReactOnRails\.registerStore\s*\(' -n -C2
# 3) List all component registrations to spot duplicates with generated packs
rg -nP 'ReactOnRails\.register\s*\(' -n -C2 Length of output: 1644 Remove duplicate store registrations in serverRegistration.jsx 🤖 Prompt for AI Agents
|
||
import routerCommentsStore from '../store/routerCommentsStore'; | ||
import commentsStore from '../store/commentsStore'; | ||
import Footer from '../components/Footer/Footer'; | ||
import Footer from '../components/Footer/ror_components/Footer'; | ||
|
||
ReactOnRails.register({ | ||
App, | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent startup race: generate packs before webpack processes start
With v16 auto-registration, webpack can error if wp-client/wp-server start before packs exist. Prepend generate_packs to both webpack commands (and optionally Rails) and set HOST/PORT defaults for container/devbox access.
Based on learnings.
📝 Committable suggestion
🤖 Prompt for AI Agents