Docs: frame Redux as legacy shared-store guidance#4278
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f939b4ffe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR reframes Redux as legacy shared-store guidance in the docs. The main changes are:
Confidence Score: 4/5The docs update is mostly safe, but the generator reference needs a fix before merging.
docs/oss/api-reference/generator-details.md Important Files Changed
Reviews (1): Last reviewed commit: "Docs: frame Redux as legacy shared-store..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/oss/api-reference/redux-store-api.md (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix hyphenation: "client-side stores" and "server-rendered".
The phrases "client side stores" and "server rendered" need hyphens when used as compound modifiers: "client-side stores" and "server-rendered". The static analysis also flags repetitive "You want to" phrasing; consider varying the sentence structure for readability.
📝 Proposed fix
- 2. You want to place the props to hydrate the client side stores at the very end of your HTML, probably server rendered, so that the browser can render all earlier HTML first. However, you're probably better off using [React on Rails Pro](../../pro/react-on-rails-pro.md) if you're at all concerned about performance. + 2. You want to place the props to hydrate the client-side stores at the very end of your HTML, probably server-rendered, so that the browser can render all earlier HTML first. However, you're probably better off using [React on Rails Pro](../../pro/react-on-rails-pro.md) if you're at all concerned about performance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/oss/api-reference/redux-store-api.md` at line 24, The markdown text in the Redux store API docs has compound modifiers written without hyphens and should be updated for readability. In the sentence describing props for hydration, change the phrases in the prose around “client side stores” and “server rendered” to “client-side stores” and “server-rendered,” and adjust the repeated “You want to” phrasing in that same paragraph to a more natural alternative while keeping the meaning intact.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/oss/api-reference/redux-store-api.md`:
- Line 24: The markdown text in the Redux store API docs has compound modifiers
written without hyphens and should be updated for readability. In the sentence
describing props for hydration, change the phrases in the prose around “client
side stores” and “server rendered” to “client-side stores” and
“server-rendered,” and adjust the repeated “You want to” phrasing in that same
paragraph to a more natural alternative while keeping the meaning intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 312f9da6-2cb7-422f-8bed-b159b99c803f
📒 Files selected for processing (12)
docs/oss/api-reference/generator-details.mddocs/oss/api-reference/javascript-api.mddocs/oss/api-reference/redux-store-api.mddocs/oss/building-features/react-and-redux.mddocs/oss/building-features/react-router.mddocs/oss/core-concepts/auto-bundling-file-system-based-automated-bundle-generation.mddocs/oss/core-concepts/how-react-on-rails-works.mddocs/oss/core-concepts/render-functions-and-railscontext.mddocs/oss/getting-started/quick-start.mddocs/oss/getting-started/tutorial.mddocs/oss/getting-started/using-react-on-rails.mddocs/oss/introduction.md
justin808
force-pushed
the
codex/redux-legacy-docs-4275
branch
from
5f939b4 to
5bed87b
Compare
justin808
changed the base branch from
main
to
codex/v17-redux-generator-policy-4272-4273
|
Updated after review:
|
💡 Codex Review Preserve explicit Redux installs in recovery guidance
When an explicit ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
justin808
force-pushed
the
codex/v17-redux-generator-policy-4272-4273
branch
from
209470d to
ba301d9
Compare
justin808
force-pushed
the
codex/redux-legacy-docs-4275
branch
from
5bed87b to
e81cf9f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e81cf9fbad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
justin808
force-pushed
the
codex/v17-redux-generator-policy-4272-4273
branch
from
ba301d9 to
55e928a
Compare
justin808
force-pushed
the
codex/redux-legacy-docs-4275
branch
from
e81cf9f to
e2d0a50
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a91e70073c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review: Docs: frame Redux as legacy shared-store guidanceThis PR's strategy is sound — demoting Redux from the default greenfield path while preserving it for existing apps and advanced multi-island shared-store use cases. The WARNING callouts, heading renames, and state-choice framing are well-executed. A few issues worth addressing before merge: [MEDIUM] Both SSR examples teach manual
|
| File | Line | Current link text |
|---|---|---|
docs/oss/getting-started/tutorial.md |
316 | "Redux integration guide" |
docs/oss/getting-started/oss-vs-pro.md |
17 | "Redux store registration" |
docs/oss/introduction.md |
88 | "Redux Integration" |
docs/oss/getting-started/using-react-on-rails.md |
240 | "Redux Integration" |
docs/oss/getting-started/quick-start.md |
193 | "Redux Integration" |
The PR updated the surrounding descriptions of these links elsewhere but these five slipped through. tutorial.md:316 is the most misleading — it sits inside the new appendix and calls the target a forward-looking "integration guide."
[LOW] State-choice three-tier taxonomy duplicated in three places
The "local island state / server state / multi-island shared client state" bullet list appears nearly verbatim in redux-store-api.md, tutorial.md, and react-and-redux.md. If the tool recommendations evolve (e.g., TanStack Query phrasing changes), three locations need independent updates with drift risk. Consider linking to one canonical location.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
+ci-run-hosted |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2af48bb8cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
justin808
added
the
ready-for-hosted-ci
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
Code Review: Docs — frame Redux as legacy shared-store guidanceThis PR cleanly reframes Redux from a recommended starter path to a legacy/advanced option across 12 docs files. The state-choice triad (local island state → server state → multi-island shared client state) is a valuable addition. Overall the direction and execution are good; a few issues need attention before merge. Findings (most-severe first)1.
Fix: run 2. The two-item use-case list now has mixed grammatical structure — item 1 is a qualifying conditional ("You want to have…") while item 2 was changed to an imperative ("Place the props…"), making it read like a required step rather than a second trigger scenario. Restore "You want to place the props…" on item 2. 3. The heading at line 26 now says "Multiple React Islands on a Page with One Store", but body prose at lines 23 and 28 still says "multiple React components". The PR updated the heading but not the surrounding prose. 4. Every file touched by this PR updates Redux descriptions to "legacy or advanced shared-store" language, but
This is the only place in the docs that describes Redux without the "legacy" qualifier, contradicting the messaging added everywhere else in this PR. 5. The WARNING block says: "New apps should start with plain "entries" is established React on Rails / webpack / Rspack terminology for bundle entry points. A developer who has not yet read the auto-bundling docs could interpret this as "create a webpack entry for each react_component call," which is the opposite of what auto-bundling provides. Suggested replacement: "plain |
Review: Docs: frame Redux as legacy shared-store guidanceThe PR cleanly reframes Redux as a legacy/advanced option throughout the docs, adds explicit state-choice guidance (local state → server state → multi-island shared store), and adds a non-Redux React Router example. The messaging is consistent and well-reasoned. Two actionable issues found. Finding 1 —
|
| llms-full.txt line | Stale anchor | Correct anchor |
|---|---|---|
| 865 | #appendix-redux-integration |
#appendix-redux-and-state-choices |
| 879 | #appendix-redux-integration |
#appendix-redux-and-state-choices |
| 2647 | redux-store-api.md#multiple-react-components-on-a-page-with-one-store |
redux-store-api.md#multiple-react-islands-on-a-page-with-one-store |
Fix: run node script/generate-llms-full.mjs and commit the updated file.
Finding 2 — New server SSR example has incompatible client/server signatures with no explanation (should fix)
The new Basic Router examples use the .client.jsx / .server.jsx auto-bundling convention, but never say so. This creates a silent trap:
RouterApp.client.jsxexports a plain React component:(props) => JSXRouterApp.server.jsxexports a React on Rails render-function:(props, railsContext) => () => JSX
These two signatures are intentionally different and work correctly only when auto-bundling is active (requires config.components_subdirectory set and nested_entries: true in shakapacker). Without auto-bundling, a developer who calls ReactOnRails.register({ RouterApp }) with one file will silently break the other: registering the client file on the server means railsContext is undefined, and const { location } = railsContext (line 124) throws TypeError: Cannot destructure property 'location' of undefined, producing a blank SSR page.
The auto-bundling docs fully document the convention, but react-router.md does not link to them or warn that the .client.jsx / .server.jsx split is a prerequisite, not just a naming convention.
Suggested fix: add a brief note before the server example (line 113) such as:
Note: The
.client.jsx/.server.jsxsuffix split relies on React on Rails auto-bundling (config.components_subdirectory). Without auto-bundling, you need separateReactOnRails.register()calls for the client and server entry points, or a single render-function that conditionally usesStaticRoutervsBrowserRouter. See the auto-bundling guide.
Code ReviewThis PR does a clean job of reframing Redux as a legacy/advanced path across the docs: renamed headings, updated anchors, new state-choice decision matrix, and re-labelled sections are all consistent. The cross-reference audit confirms the major anchors resolve correctly ( Four issues found, all in the newly added content. 1. Misleading link in The note warns that Redux inline-script registration breaks with 2. The "Basic Server-Side Rendering with React Router" section introduces the render-function thunk pattern ( 3. The new section documents 4. Misleading inline comment in The comment says "Props are passed again by React, but this example uses the props and location captured by closure." This is inaccurate: React on Rails calls |
Review: Docs — frame Redux as legacy shared-store guidanceOverall this is a well-structured docs-only PR with clear intent and good execution. A few specific observations below. What's working well
Server-side render thunk pattern: Changing the legacy Redux SSR example from manual Link repair in
Minor issues (inline comments follow)
No concerns on
|
| /** | ||
| * Allows registration of store generators to be used by multiple React components on one Rails | ||
| * view. Store generators are functions that take one arg, props, and return a store. Note that | ||
| * the `setStore` API is different in that it's the actual store hydrated with props. | ||
| * view. Store generators receive props and railsContext, then return a store. Note that the | ||
| * `setStore` API is different in that it's the actual store hydrated with props. |
There was a problem hiding this comment.
The registerStoreGenerators JSDoc here was tightened (props + railsContext) but not given the "legacy / advanced shared-store" framing that this PR applies everywhere else. Compare with the markdown copy in docs/oss/api-reference/javascript-api.md (lines 50-57 in this diff), which now reads:
Allows registration of store generators for legacy or advanced pages where multiple React roots on one Rails view share a Redux store...
vs. this JSDoc, which still says:
Allows registration of store generators to be used by multiple React components on one Rails view...
javascript-api.md itself states "The best source of docs is the interface ReactOnRails in types/index.ts" — so a developer following that pointer (or just reading IDE tooltips, which surface this JSDoc directly) lands on the old, non-legacy framing while the website doc says the opposite. Given the PR's whole purpose is consistently reframing Redux as legacy/advanced, this source-of-truth comment looks like a miss rather than an intentional choice.
ReviewThis PR reframes Redux as a legacy/advanced shared-store pattern across the docs (generator help text, tutorial, API reference, React Router guide, I traced every changed/added link anchor, verified the One inconsistency survived verification (left as an inline comment):
Everything else (cross-file anchor links, the new |
…-4272-4273' into codex/redux-legacy-docs-4275 * origin/codex/v17-redux-generator-policy-4272-4273: Preserve TypeScript in legacy Redux fallback warning
Review: Docs: frame Redux as legacy shared-store guidanceScope: Pure docs/comment changes (14 files, no runtime logic). The PR reframes Redux as a legacy/advanced-use API across the generator, tutorial, getting-started, and API-reference docs, and fixes the React Router examples so a shared route tree isn't auto-registered as a conflicting Verification performed: Cross-checked the technical claims in the new prose against the actual implementation rather than taking them at face value:
Findings: None survived verification — no inline comments posted. This PR is unusually well fact-checked against the underlying code (the base branch already shipped the generator behavior this PR documents), and I couldn't find a factual inaccuracy, broken link, or inconsistent example. |
Code ReviewScope: Docs-only PR (13 markdown/text files + Review approach: Read every changed hunk, then cross-checked the technical claims against the actual generator/runtime code and the render-function contract, specifically:
Findings: None survived verification. This is a clean, well-cross-checked documentation change — no correctness, security, or performance concerns. |
…st-surface-4274 * origin/main: Docs: frame Redux as legacy shared-store guidance (#4278) Hide legacy Redux install generator path (#4277) Add typed Rails action helper (#4260) Add Rails response type generation (#4259) Remove stale Pro RSpec encoding workaround (#4291) [Pro] Fix Gemfile loader source encoding under C locale (#4281) # Conflicts: # react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb
…ered-rsc-rendering * origin/main: Adopt agent-workflow binstubs (.agents/bin/ + AGENTS.md pointer) (#4264) [codex] Wire release harnesses into CI (#4266) Document bounded coordination audit fallback (#4283) Docs: frame Redux as legacy shared-store guidance (#4278) Hide legacy Redux install generator path (#4277) Add typed Rails action helper (#4260) Add Rails response type generation (#4259) Remove stale Pro RSpec encoding workaround (#4291) [Pro] Fix Gemfile loader source encoding under C locale (#4281) Fail fast for RSC on Rspack v1 (#4289) [codex] Document agent workflow trust boundary (#4288) Fix precompile hook forcing UTF-8 onto non-UTF-8 (national) locales (#4244) Regenerate Pro llms bundle (#4280)
1 participant
Why
The V17 new-app path should no longer teach Redux as the normal starter architecture. Redux runtime APIs remain supported, so the docs need to preserve the legitimate shared-store guidance while steering greenfield apps toward local React state, Rails/server state, and smaller patterns first.
Closes #4275.
What changed
react_on_rails:install --reduxrecommendations from generator and tutorial docs.redux_storeandregisterStoreGeneratorsas legacy or advanced multi-island shared-store APIs.ror_componentsdefinitions.Validation
.agents/bin/agent-workflow-seam-doctor.agents/skills/pr-batch/bin/pr-security-preflight --repo shakacode/react_on_rails 4275(SECURITY_PREFLIGHT_OK)node script/generate-llms-full.mjs --checkscript/check-docs-sidebarpnpm start format.listDifferentPATH="/tmp/codex-lychee-0.23.0:$PATH" bin/check-linksgit diff --checklychee 0.23.0onPATHCI recommendation
Docs-only change. No hosted CI label recommended beyond normal markdown/docs checks.