docs: Entry Point Consolidation and Tutorial Improvements #1860
Conversation
WalkthroughMajor documentation information‑architecture rework: new Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client browser
participant Rails as Rails app
participant ServerEntry as Server entry (Node/SSR)
participant ReactApp as React Router v6 App
Note left of Rails: Server-side render path (SSR)
Browser->>Rails: Initial HTTP GET (URL)
Rails->>ServerEntry: renderToString with StaticRouter(location)
ServerEntry->>ReactApp: Render routes (StaticRouter + Routes)
ReactApp-->>ServerEntry: HTML string
ServerEntry-->>Rails: Inject server HTML into view
Browser->>ReactApp: Hydrate with BrowserRouter (client entry)
Note right of Browser: Client routing uses Routes/Route (v6) and element-based rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (4)
Comment |
ihabadham
changed the title
PR Review: Phase 3 - Entry Point Consolidation and Tutorial ImprovementsOverall Assessment✅ Strong approval - This is excellent documentation work that significantly improves the user experience. The consolidation of entry points and modernization of the tutorial are well-executed. Strengths1. Clear Entry Point Strategy ✅
2. Improved Tutorial Structure ✅
3. Better Content Organization ✅
4. Documentation Quality ✅
Code Quality & Best PracticesExcellent ✅
Specific Review Points1. Was deleting
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/getting-started/tutorial.md (1)
125-128: Tag the environment variables code fence.Markdownlint MD040 flags this block. Add a language (e.g.,
bash) to satisfy linting.-``` +```bash EXECJS_RUNTIME=Nodedocs/planning/docs-improvement/ia-redesign-live.md (1)
402-478: Add a language tag to the folder-structure fence.Markdownlint complains about this fence lacking a language. Tag it as
text(or similar) to keep lint happy.-``` +```text docs/ ├── introduction.md # ✅ Created - new unified homepage ├── README.md # ✅ Simplified for GitHub users @@ └── advanced-topics/ (1 file - KEEPING as category) └── rails-engine-integration.md # Step 5 correction: from deployment
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/README.md(3 hunks)docs/advanced-topics/manual-installation-overview.md(0 hunks)docs/core-concepts/react-on-rails-overview.md(0 hunks)docs/deployment/heroku-deployment.md(2 hunks)docs/getting-started.md(0 hunks)docs/getting-started/installation-into-an-existing-rails-app.md(0 hunks)docs/getting-started/quick-start.md(1 hunks)docs/getting-started/tutorial.md(8 hunks)docs/getting-started/using-react-on-rails.md(1 hunks)docs/guides/advanced/README.md(0 hunks)docs/home.md(0 hunks)docs/introduction.md(1 hunks)docs/misc/doctrine.md(1 hunks)docs/planning/docs-improvement/04-ia-redesign-plan.md(1 hunks)docs/planning/docs-improvement/ia-redesign-live.md(2 hunks)
💤 Files with no reviewable changes (6)
- docs/guides/advanced/README.md
- docs/advanced-topics/manual-installation-overview.md
- docs/getting-started.md
- docs/home.md
- docs/core-concepts/react-on-rails-overview.md
- docs/getting-started/installation-into-an-existing-rails-app.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
docs/getting-started/quick-start.mddocs/misc/doctrine.mddocs/getting-started/using-react-on-rails.mddocs/introduction.mddocs/README.mddocs/deployment/heroku-deployment.mddocs/getting-started/tutorial.mddocs/planning/docs-improvement/04-ia-redesign-plan.mddocs/planning/docs-improvement/ia-redesign-live.md
🪛 markdownlint-cli2 (0.18.1)
docs/getting-started/using-react-on-rails.md
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/deployment/heroku-deployment.md
100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/getting-started/tutorial.md
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/planning/docs-improvement/ia-redesign-live.md
402-402: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: claude-review
- GitHub Check: markdown-link-check
- GitHub Check: build
- GitHub Check: build-and-test
| "engines": { | ||
| "node": "16.19.0", | ||
| "yarn": "1.22.4" | ||
| }, | ||
| ``` |
There was a problem hiding this comment.
Update the Heroku engines to match our supported runtime.
We now require Node 18+ (see Introduction/Quick Start), but this guide still pins Node 16.19.0, which is end-of-life. Following these steps will deploy on an unsupported runtime. Please bump the engines block to Node 18 (and adjust Yarn if needed) to stay consistent.
🤖 Prompt for AI Agents
In docs/deployment/heroku-deployment.md around lines 131 to 135, the engines
block pins Node to 16.19.0 which is EOL and mismatches our required Node 18+;
update the "engines" block to specify a supported Node 18+ version (e.g.,
">=18.0.0" or a specific 18.x release) and adjust the yarn entry if necessary
(either remove Yarn if irrelevant to current install flow or set a compatible
Yarn version), then run a quick lint/spell check to ensure the markdown still
renders correctly.
Pull Request Review: Entry Point Consolidation and Tutorial ImprovementsOverall Assessment ✅This is an excellent documentation improvement that successfully addresses the problem of multiple conflicting entry points. The changes are well-thought-out and significantly improve the user experience. I recommend approval with minor suggestions below. Strengths 🎯1. Clear Information Architecture
2. Content Quality
3. Documentation Hygiene
Suggestions for Improvement 💡Minor Issues1. Heroku Deployment Content (Low Priority)The merged Heroku deployment guide contains some older patterns: Lines 52-75 ( default: &default
adapter: postgresql
username:
password:
host: localhostSuggestion: Modern Rails 7+ uses Lines 86-98 (
Severity: Low - The disclaimer at the top addresses this adequately. 2. Tutorial Version ReferencesLine 113 ( **Note on Redux:** The basic installer uses React Hooks for state management...Observation: This is clear and helpful! The clarification about Redux vs Hooks is excellent. ✅ Lines 60-61 mention checking Rails 7.0+, which is good and matches the stated requirements. 3. Introduction.md Minor PolishLine 199 ( {
renderedHtml: {
componentHtml, redirectLocation, error; // ← Semicolons should be commas
}
}Issue: JavaScript object syntax error - should use commas, not semicolons between properties. Fix: {
renderedHtml: {
componentHtml,
redirectLocation,
error
}
}Severity: Medium - This is example code that won't run as written. 4. README.md SimplificationThe simplified Minor suggestion: Consider whether lines 44-50 (the "I want to..." table) duplicates content from Validation Checklist ✅Based on CLAUDE.md requirements:
Performance Considerations ⚡Not applicable - documentation changes only. No runtime performance impact. Security Concerns 🔒None identified. Documentation changes pose no security risks. Test Coverage 🧪Not applicable for documentation. However, consider:
Recommended Next Steps 🚀As noted in the PR description:
Final Recommendation✅ APPROVE with one minor fix required: Required: Fix JavaScript syntax error in Optional: Consider updating Heroku database.yml example to modern pattern (or keep with disclaimer) This PR significantly improves documentation usability and follows the Rails doctrine of "convention over configuration" by establishing clear, opinionated entry points. Excellent work! 🎉 Review Philosophy NoteFollowing the project's doctrine of professional objectivity: While this is overwhelmingly positive feedback, I've highlighted the genuine issues found (JavaScript syntax error) and areas where current best practices have evolved (Heroku config). The consolidation strategy is sound and well-executed. |
Code Review: Entry Point Consolidation and Tutorial ImprovementsI've reviewed PR #1860, which addresses Phase 3 of the IA redesign by consolidating entry points and improving the tutorial. Overall, this is excellent work that significantly improves documentation clarity and user experience. ✅ Strengths1. Entry Point Consolidation (Problem 1.2 - Solved!)Excellent execution. The new structure creates a clear, single source of truth:
2. Tutorial Improvements
3. Heroku Deployment Guide
|
| Category | Rating |
|---|---|
| Code Quality | ⭐⭐⭐⭐⭐ |
| Documentation | ⭐⭐⭐⭐⭐ |
| Best Practices | ⭐⭐⭐⭐⭐ |
| Security | ✅ |
🚀 Recommendation: APPROVE with minor suggestions
This PR successfully achieves its goals. The minor suggestions above are optional improvements.
Before merging:
- Verify all new files are committed (especially using-react-on-rails.md)
- Run rake autofix for Prettier formatting
- Consider addressing Puma duplication
Great work! 🎉
ihabadham
force-pushed
the
feature/docs-ia-redesign-1845-continue
branch
from
5388efb to
5e71aeb
Compare
PR Review: Entry Point Consolidation and Tutorial ImprovementsThis is excellent work that significantly improves documentation structure! Here is my detailed review: Strengths1. Excellent Entry Point Consolidation
2. Strong Conceptual Guide
3. Tutorial Improvements
4. README Simplification
Issues Found1. Syntax Error in using-react-on-rails.md line 192 2. Formatting Check Required
3. Link Validation Needed Deleted Files AssessmentAll deletions are appropriate:
Recommendations Before Merge
Overall AssessmentQuality: 5/5 Final Verdict: Approve with Minor Changes Once the syntax error is fixed and formatting tools are run, this is ready to merge. Great job on the thoughtful consolidation and improved information architecture! Reviewed by: Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/getting-started/tutorial.md (2)
125-127: Add a language to the fenced block (MD040).Tag this as shell/env so markdownlint passes.
-``` +```bash EXECJS_RUNTIME=Node
185-186: Fix path typo: “initializers.”Correct “config/intializers/react_on_rails.rb” to “config/initializers/react_on_rails.rb”.
-`config/intializers/react_on_rails.rb` +`config/initializers/react_on_rails.rb`
♻️ Duplicate comments (3)
docs/deployment/heroku-deployment.md (2)
100-103: Tag the Procfile code fence with a language.Add a language hint so markdownlint (MD040) passes.
-``` +```Procfile web: bundle exec puma -C config/puma.rb
131-135: Update Node/Yarn engines to supported versions (Node 18+).This still pins Node 16.19.0, which is EOL and inconsistent with prerequisites. Bump to Node 18+ (and adjust Yarn) for alignment.
- "engines": { - "node": "16.19.0", - "yarn": "1.22.4" - }, + "engines": { + "node": ">=18 <21", + "yarn": ">=1.22" + },docs/getting-started/using-react-on-rails.md (1)
126-130: Add a language to the directory tree fence (MD040).Tag this block (e.g.,
text) so markdownlint passes.-``` +```text app/javascript/ └── bundles/HelloWorld/ # or src/HelloWorld/ror_components/ with auto-bundling └── HelloWorld.jsx
🧹 Nitpick comments (7)
docs/planning/docs-improvement/ia-redesign-live.md (2)
28-38: Clarify status consistency (Step 0 vs. Completion Summary).Step 0 shows "In Progress" with unchecked items, while the Completion Summary says all steps are complete. Update Step 0 checkboxes/status to reflect completion or annotate as historical to avoid confusion.
Also applies to: 304-314
115-121: Align Step 8 task with actual changes (README.md not deleted).Step 8 lists “Delete/archive README.md,” but this PR simplified
docs/README.md. Revise the Step 8 bullet to “SimplifyREADME.mdfor GitHub users” to match what shipped.Also applies to: 359-368
docs/deployment/heroku-deployment.md (3)
168-170: Use correct language for Ruby config snippet.This is Ruby code, not bash. Tag the fence as Ruby.
-```bash +```ruby config.build_production_command = "RAILS_ENV=production NODE_ENV=production bin/shakapacker"
177-179: Use Ruby fence for Rake task clear example.This is Ruby that's added to a Rakefile/initializer, not a shell command.
-```bash +```ruby Rake::Task['shakapacker:clean'].clear
79-82: Prefer modern Rails tasks over rake aliases.Use
rails db:setupandrails db:migrateto match current Rails conventions.- rake db:setup - rake db:migrate + rails db:setup + rails db:migratedocs/getting-started/tutorial.md (1)
145-153: Clarify HMR vs full page reload; update links.The text implies HMR but describes full page refresh (live reload). Clarify that without React Refresh you get full reloads; for state‑preserving updates enable React Fast Refresh. Also replace the deprecated Webpacker link with current Webpack docs (and/or Shakapacker docs).
docs/getting-started/quick-start.md (1)
127-133: Verify helper names/necessity with auto-bundling (Shakapacker v7).Confirm
stylesheet_pack_tag/javascript_pack_tagare the intended helpers and still required when usingauto_load_bundle: true. If not, update to current helpers or note when they’re needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/README.md(3 hunks)docs/advanced-topics/manual-installation-overview.md(0 hunks)docs/core-concepts/react-on-rails-overview.md(0 hunks)docs/deployment/heroku-deployment.md(2 hunks)docs/getting-started.md(0 hunks)docs/getting-started/installation-into-an-existing-rails-app.md(0 hunks)docs/getting-started/quick-start.md(2 hunks)docs/getting-started/tutorial.md(8 hunks)docs/getting-started/using-react-on-rails.md(1 hunks)docs/guides/advanced/README.md(0 hunks)docs/home.md(0 hunks)docs/introduction.md(1 hunks)docs/misc/doctrine.md(1 hunks)docs/planning/docs-improvement/04-ia-redesign-plan.md(1 hunks)docs/planning/docs-improvement/ia-redesign-live.md(2 hunks)
💤 Files with no reviewable changes (6)
- docs/guides/advanced/README.md
- docs/core-concepts/react-on-rails-overview.md
- docs/getting-started.md
- docs/advanced-topics/manual-installation-overview.md
- docs/home.md
- docs/getting-started/installation-into-an-existing-rails-app.md
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/introduction.md
- docs/misc/doctrine.md
- docs/planning/docs-improvement/04-ia-redesign-plan.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
docs/getting-started/quick-start.mddocs/getting-started/using-react-on-rails.mddocs/planning/docs-improvement/ia-redesign-live.mddocs/README.mddocs/getting-started/tutorial.mddocs/deployment/heroku-deployment.md
🪛 markdownlint-cli2 (0.18.1)
docs/deployment/heroku-deployment.md
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: claude-review
- GitHub Check: build
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: markdown-link-check
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: build-and-test
🔇 Additional comments (2)
docs/getting-started/quick-start.md (1)
162-165: Good redirect to the new concepts guide.Pointing to “Using React on Rails” aligns with the new IA.
docs/README.md (1)
3-6: Solid simplification and IA alignment.Concise GitHub-oriented README with clear learning paths and category index. Links align with the new structure.
Also applies to: 15-26, 45-55, 60-64
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/deployment/heroku-deployment.md (2)
100-102: Restore the Procfile fence language hint.This fenced block still lacks the
Procfilelanguage tag that markdownlint flagged earlier, so CI will fail the same MD040 check unless we add it back.-``` +```Procfile web: bundle exec puma -C config/puma.rb
131-134: Align Heroku engines with our supported runtime.We still pin Heroku to Node 16.19.0, which is EOL and contradicts the Node 18+ requirement documented elsewhere in this PR. Please bump Node (and Yarn if you want to keep it) to a supported release before merging.
"engines": { - "node": "16.19.0", - "yarn": "1.22.4" + "node": ">=18.18.0", + "yarn": "1.22.22" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/README.md(3 hunks)docs/advanced-topics/manual-installation-overview.md(0 hunks)docs/core-concepts/react-on-rails-overview.md(0 hunks)docs/deployment/heroku-deployment.md(2 hunks)docs/getting-started.md(0 hunks)docs/getting-started/installation-into-an-existing-rails-app.md(0 hunks)docs/getting-started/quick-start.md(2 hunks)docs/getting-started/tutorial.md(8 hunks)docs/getting-started/using-react-on-rails.md(1 hunks)docs/guides/advanced/README.md(0 hunks)docs/home.md(0 hunks)docs/introduction.md(1 hunks)docs/misc/doctrine.md(1 hunks)docs/planning/docs-improvement/04-ia-redesign-plan.md(1 hunks)docs/planning/docs-improvement/ia-redesign-live.md(2 hunks)
💤 Files with no reviewable changes (6)
- docs/home.md
- docs/core-concepts/react-on-rails-overview.md
- docs/getting-started/installation-into-an-existing-rails-app.md
- docs/guides/advanced/README.md
- docs/getting-started.md
- docs/advanced-topics/manual-installation-overview.md
✅ Files skipped from review due to trivial changes (1)
- docs/introduction.md
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/misc/doctrine.md
- docs/getting-started/using-react-on-rails.md
- docs/getting-started/quick-start.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
docs/README.mddocs/planning/docs-improvement/ia-redesign-live.mddocs/deployment/heroku-deployment.mddocs/getting-started/tutorial.mddocs/planning/docs-improvement/04-ia-redesign-plan.md
🪛 markdownlint-cli2 (0.18.1)
docs/getting-started/tutorial.md
7-7: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
| --- | ||
|
|
||
| _Updated for Ruby 2.7, Rails 7, React on Rails v13, and Shakapacker v7_ | ||
| _Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7_ |
There was a problem hiding this comment.
Fix the markdownlint MD036 violation.
Using italics as a heading trips markdownlint (MD036). Switching this line to a callout keeps the note while satisfying lint.
-_Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7_
+> **Note:** Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7.📝 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.
| _Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7_ | |
| > **Note:** Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
7-7: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In docs/getting-started/tutorial.md around line 7, the line currently uses
italics as a heading ("_Updated for Ruby 3.0+, Rails 7, React on Rails v16, and
Shakapacker v7_"), which triggers markdownlint MD036; replace the italicized
heading with a non-heading callout/note (for example start the line with "Note:"
or a blockquote ">" or a fenced admonition) so it remains a visible update
notice but is not parsed as a heading and thus satisfies MD036.
- Add docs/introduction.md as unified homepage - Explains what React on Rails is and why to use it - Clear when to use / when not to use decision guide - Three clear paths: Quick Start, Installation, Tutorial - Built from actual docs (overview, doctrine, system requirements) - Added community stat (thousands of production sites) - Delete docs/core-concepts/react-on-rails-overview.md - Content consolidated into introduction.md - Outdated prerequisites (Rails >=5 vs current Rails 7+) - Creates confusion with two similar entry points - Update links to overview.md: - docs/home.md: Overview → Introduction - docs/misc/doctrine.md: Overview → Introduction Solves Problem 1.2 (Multiple Conflicting Entry Points) from docs improvement plan. Next: Handle remaining entry point files (home.md, README.md, getting-started.md).
- Simplify docs/README.md for GitHub users: - Direct users to website first - Keep valuable learning paths from PR #1813 - Keep popular use cases table - Add documentation categories overview - Remove duplicate content (now in introduction.md) - Reduced from 173 lines to 65 lines - Enhance introduction.md: - Add missing react_on_rails_demo_ssr_hmr example repo - Now has all 3 example repos (spec/dummy, demo SSR/HMR, live reactrails.com) Purpose: docs/README.md serves GitHub users browsing repo structure, while introduction.md serves as website homepage. Different audiences, minimal duplication.
Document completion of entry point consolidation work: - Created introduction.md - Deleted overview.md - Simplified docs/README.md - Updated folder structure to reflect changes Next: Delete home.md, handle getting-started.md
- Was the current website homepage (29 lines of links) - All valuable content already in introduction.md or README.md - No unique content lost (example repos, use cases, help links all covered) - Replaced by introduction.md as new website homepage Updated ia-redesign-live.md to reflect completion. Next: Extract content from getting-started.md then delete it.
Transform docs/getting-started.md (253 lines) into a focused conceptual guide: docs/getting-started/understanding-react-on-rails.md (238 lines). Changes: - Removed redundant sections (Choose Starting Point, System Requirements, More Reading) - Transformed installation section into brief conceptual overview with links - Kept and refined: Basic Usage, Auto-Bundling, Render-Functions, Error Handling - Added clear 'Next Steps' section with organized learning paths - Updated link in quick-start.md to point to new file - Deleted original docs/getting-started.md Purpose: This is now a conceptual journey guide explaining how React on Rails works, complementing the hands-on tutorial and quick-start guides. Addresses user feedback that tutorial was too overwhelming and mixed audiences. Next: Website config updates (keeping for last as requested)
Rationale: - "understanding" is vague and confusing next to how-react-on-rails-works.md - "using" clearly indicates practical usage guide - Better distinction: how-react-on-rails-works.md (architecture) vs using-react-on-rails.md (practical usage) Changes: - Renamed docs/getting-started/understanding-react-on-rails.md → using-react-on-rails.md - Updated link in quick-start.md - Updated references in ia-redesign-live.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
… structure Changes to tutorial.md: 1. Replaced Heroku deployment section (139 lines) with brief Deployment section linking to deployment guides 2. Updated versions: Ruby 2.7 → 3.0+, Rails 5.1.3 → 7.0+, RoR v13 → v16 3. Clarified Redux usage: tutorial demonstrates Redux, but basic installer uses Hooks (user choice) 4. Merged duplicate HMR sections into one cohesive section using ./bin/dev 5. Renamed "Other features" → "Going Further" with better organization: - Server Rendering (important) - Optional Configuration subsection (/client structure, Cloud9, RubyMine) Changes to heroku-deployment.md: 1. Merged tutorial's detailed Heroku instructions with existing guide 2. Added note about older versions with link to current Heroku Rails 7 guide 3. Organized into clear sections: Create App, Buildpacks, Postgres, Puma, Node/Yarn, Assets, Deploy Rationale: - Tutorial was overwhelming with platform-specific deployment before core features - Heroku content now in dedicated guide (reusable, maintainable) - Clear separation: tutorial teaches React on Rails, deployment guides teach platforms - Better progression: install → run → features → deploy → advanced 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Rationale: - File has been outdated since 2018 (had "TODO: Review this file" for 7 years) - Confused purpose: title says "Manual Installation" but subtitle says "summarizes what generator does" - Outdated content: references /client directory, webpacker, missing auto-bundling - No clear use case: rails generate react_on_rails:install IS the manual installation (not external CLI like create-react-app) - Content is better covered in: - how-react-on-rails-works.md (architecture) - using-react-on-rails.md (usage) - Generator source code (what files are created) Changes: - Deleted docs/advanced-topics/manual-installation-overview.md - Removed link in installation-into-an-existing-rails-app.md - Updated ia-redesign-live.md tracker Decision discussed with team in Slack. Can be reverted if use case is identified. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Mark Phase 3 as completed with notes on what actually happened vs what was planned: - ✅ Created introduction.md (as planned) - ✅ Transformed getting-started.md → using-react-on-rails.md (adapted from original plan) - ✅ Simplified README.md (kept, not deleted) - ✅ Deleted home.md (as planned) - ✅ Added tutorial improvements (discovered during work) - ✅ Deleted manual-installation-overview.md (discovered during work) - ✅ Updated cross-references manually (not scripted) Updated testing checklist to show what's complete and what's deferred to Phase 1.
Escape <head> tag in prose to prevent MDX from expecting closing tag. Changed "in the <head>:" to "in the `<head>` section:" to use inline code. This fixes gatsby-plugin-mdx compilation error: "Expected a closing tag for <head> before the end of paragraph"
ihabadham
force-pushed
the
feature/docs-ia-redesign-1845-continue
branch
from
5e71aeb to
2cf9420
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/getting-started/quick-start.md (1)
127-133: ERB example is invalid: pack helpers require names.Both helpers need at least a pack name (e.g., 'application'). Without arguments this will raise.
-```erb - <%= stylesheet_pack_tag %> - <%= javascript_pack_tag %> -``` +```erb +<%= stylesheet_pack_tag 'application', media: 'all' %> +<%= javascript_pack_tag 'application', defer: true %> +```docs/getting-started/tutorial.md (1)
184-189: Typo in path: initializers.Fix the filename to avoid confusion.
-`config/intializers/react_on_rails.rb` +`config/initializers/react_on_rails.rb`
♻️ Duplicate comments (5)
docs/deployment/heroku-deployment.md (2)
100-103: Add language to the Procfile fence (MD040).Tag the fenced block with a language to satisfy markdownlint.
-``` +```Procfile web: bundle exec puma -C config/puma.rb--- `131-135`: **Update engines to Node 18+ (aligns with system requirements).** The guide pins Node 16.19.0 (EOL) and contradicts “Node.js 18+” elsewhere. Bump Node; adjust Yarn if needed. ```diff - "engines": { - "node": "16.19.0", - "yarn": "1.22.4" - }, + "engines": { + "node": ">=18", + "yarn": ">=1.22.0" + },docs/getting-started/using-react-on-rails.md (2)
126-130: Add language to the directory tree fence (MD040).Tag the fence (e.g., text) so markdownlint passes.
-``` +```text app/javascript/ └── bundles/HelloWorld/ # or src/HelloWorld/ror_components/ with auto-bundling └── HelloWorld.jsx--- `189-195`: **Fix invalid JS example; remove stray semicolon and add language tag.** The semicolon inside the object literal is invalid; tag the fence with js. ```diff -```js -{ - renderedHtml: { - componentHtml, redirectLocation, error; - } -} +```js +{ + renderedHtml: { + componentHtml, + redirectLocation, + error + } +}</blockquote></details> <details> <summary>docs/getting-started/tutorial.md (1)</summary><blockquote> `7-7`: **Replace italics “update” line to satisfy MD036.** Use a callout instead of emphasis. ```diff -_Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7_ +> **Note:** Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7.
🧹 Nitpick comments (3)
docs/deployment/heroku-deployment.md (1)
52-75: Production DB config note for Heroku.Consider adding a note that production typically uses DATABASE_URL on Heroku, so explicit host/username/password are unnecessary (and may be ignored).
docs/getting-started/quick-start.md (1)
204-209: “Key File Locations” are out of date with auto‑bundling.This section references the older packs/bundles layout. Update to reflect the recommended auto‑bundling paths (and optionally show both).
-- **Components**: `client/app/bundles/[ComponentName]/components/` -- **Registration**: `client/app/bundles/[ComponentName]/startup/registration.js` -- **Packs**: `app/javascript/packs/` +- **Components (auto-bundling)**: `app/javascript/src/*/ror_components/` +- **Config**: `config/initializers/react_on_rails.rb` +- **Webpack (Shakapacker)**: `config/shakapacker.yml` +- (Optional legacy) **packs/startup** if using manual registrationdocs/getting-started/tutorial.md (1)
151-154: Update links away from deprecated Webpacker docs.Point to Shakapacker and Webpack docs instead of Webpacker 5.x.
-- [webpack-dev-server](https://github.com/rails/webpacker/blob/5-x-stable/docs/webpack-dev-server.md) +- [Shakapacker Dev Server](https://github.com/shakacode/shakapacker#dev-server) - [DevServer](https://webpack.js.org/configuration/dev-server/) - [Hot Module Replacement](https://webpack.js.org/concepts/hot-module-replacement/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/README.md(3 hunks)docs/advanced-topics/manual-installation-overview.md(0 hunks)docs/core-concepts/react-on-rails-overview.md(0 hunks)docs/deployment/heroku-deployment.md(2 hunks)docs/getting-started.md(0 hunks)docs/getting-started/installation-into-an-existing-rails-app.md(0 hunks)docs/getting-started/quick-start.md(2 hunks)docs/getting-started/tutorial.md(8 hunks)docs/getting-started/using-react-on-rails.md(1 hunks)docs/guides/advanced/README.md(0 hunks)docs/home.md(0 hunks)docs/introduction.md(1 hunks)docs/misc/doctrine.md(1 hunks)docs/planning/docs-improvement/04-ia-redesign-plan.md(1 hunks)docs/planning/docs-improvement/ia-redesign-live.md(2 hunks)
💤 Files with no reviewable changes (6)
- docs/getting-started.md
- docs/home.md
- docs/core-concepts/react-on-rails-overview.md
- docs/guides/advanced/README.md
- docs/advanced-topics/manual-installation-overview.md
- docs/getting-started/installation-into-an-existing-rails-app.md
✅ Files skipped from review due to trivial changes (1)
- docs/misc/doctrine.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
docs/README.mddocs/deployment/heroku-deployment.mddocs/introduction.mddocs/planning/docs-improvement/ia-redesign-live.mddocs/getting-started/tutorial.mddocs/getting-started/using-react-on-rails.mddocs/planning/docs-improvement/04-ia-redesign-plan.mddocs/getting-started/quick-start.md
🪛 LanguageTool
docs/README.md
[grammar] ~15-~15: There might be a mistake here.
Context: ...- What is React on Rails and why use it? 2. **[Quick Start](./getting-started/quick-sta...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...ls-app.md)** - Add to existing Rails app - **[API Reference](./api-reference/README.md...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...w helpers, configuration, JavaScript API - **[Core Concepts](./core-concepts/how-react...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ... | ## 📖 Documentation Categories - Getting Started -...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...** - Installation, quick start, tutorial - Core Concepts - How...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...React on Rails works, SSR, auto-bundling - **Building Features...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...ures/)** - Redux, routing, i18n, testing - API Reference - Vie...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...w helpers, configuration, JavaScript API - Deployment - Productio...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...roduction deployment and troubleshooting - Upgrading - Version upg...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ..../upgrading/)** - Version upgrade guides - Migrating - From other ...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...(./migrating/)** - From other frameworks - Pro - React on Rails Pro feat...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ... on Rails Pro features ## 🆘 Need Help? - **[Troubleshooting Guide](./deployment/trou...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...ting.md)** - Common issues and solutions - **[GitHub Discussions](https://github.com/s...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ..._on_rails/discussions)** - Ask questions - **[React on Rails Pro](https://www.shakacod...
(QB_NEW_EN)
docs/deployment/heroku-deployment.md
[grammar] ~34-~34: There might be a mistake here.
Context: ...tgresql. If you have not set up your app with Postgresql, you need to change your...
(QB_NEW_EN)
[style] ~77-~77: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... database: ror_production ``` Then you need to set up Postgres so you can run locally:...
(REP_NEED_TO_VB)
[style] ~153-~153: Reusing ‘you’ could be redundant. Try omitting the pronoun.
Context: ...``` and you will see your live app and you can share this URL with your friends. Congr...
(SUBJECT_DROP)
docs/introduction.md
[grammar] ~47-~47: There might be a mistake here.
Context: ...ituation: ### 🚀 New to React on Rails? **[15-Minute Quick Start →](./getting-start...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ... ### 📦 Adding to an Existing Rails App? **[Installation Guide →](./getting-started/...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: .... ### 📚 Want a Comprehensive Tutorial? **[Complete Tutorial →](./getting-started/t...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...nd deployment. ### 👀 Learn by Example? - **[Spec/Dummy App](https://github.com/shaka...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...c/dummy)** - Simple example in this repo - **[Tutorial Demo App](https://github.com/sh...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ... - Example with SSR, HMR, and TypeScript - **[Live Demo with Source](https://github.co...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...ails-works.md)** - Architecture overview - **[Auto-Bundling](./core-concepts/auto-bund...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...onent registration and bundle generation - **[Client vs Server Rendering](./core-conce...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...erver-rendering.md)** - When to use each - **[Webpack Configuration](./core-concepts/w...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...faults for JavaScript tooling with Rails - Optimize for Programmer Happiness - Ho...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...oading, ES6+, CSS modules, NPM ecosystem - Value Integrated Systems - Tight Rails...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...s - Rails 7+ (Rails 5.2+ supported) - Ruby 3.0+ - Node.js 18+ - **Shakap...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...+** (Rails 5.2+ supported) - Ruby 3.0+ - Node.js 18+ - Shakapacker 6+ (7+ r...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...pported) - Ruby 3.0+ - Node.js 18+ - Shakapacker 6+ (7+ recommended for Rea...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...lack](https://reactrails.slack.com)** - Real-time community help - **[GitHub Issues](...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...ver Components, Suspense SSR, streaming) - **[ShakaCode Consulting](mailto:react_on_ra...
(QB_NEW_EN)
[grammar] ~126-~126: There might be a mistake here.
Context: ...Webpack integration for Rails (required) - **React Documentation*...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...ct.dev)** - Official React documentation - **[Rails Guides](https://guides.rubyonrails...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...bove and let's build something great! 🚀
(QB_NEW_EN)
docs/planning/docs-improvement/ia-redesign-live.md
[grammar] ~361-~361: There might be a mistake here.
Context: ... - Reduced from 173 lines to 65 lines - Directs to website first - Kept valua...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ...o 65 lines - Directs to website first - Kept valuable learning paths from PR #18...
(QB_NEW_EN)
[grammar] ~363-~363: There might be a mistake here.
Context: ...pt valuable learning paths from PR #1813 - Kept popular use cases table - Added ...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ... #1813 - Kept popular use cases table - Added documentation categories overview ...
(QB_NEW_EN)
[grammar] ~365-~365: There might be a mistake here.
Context: ... Added documentation categories overview - Removed duplicate content (now in introd...
(QB_NEW_EN)
[grammar] ~366-~366: There might be a mistake here.
Context: ...plicate content (now in introduction.md) - Purpose: Serves GitHub users browsing re...
(QB_NEW_EN)
[grammar] ~380-~380: There might be a mistake here.
Context: ...) - Transformed installation section to conceptual overview with links 7. **✅ ...
(QB_NEW_EN)
[grammar] ~383-~383: There might be a mistake here.
Context: ...ad "TODO: Review this file" for 7 years) - Confused purpose: title said "Manual Ins...
(QB_NEW_EN)
[grammar] ~385-~385: There might be a mistake here.
Context: ...ent, webpacker`, missing auto-bundling - No clear use case: generator IS the manu...
(QB_NEW_EN)
[grammar] ~386-~386: There might be a mistake here.
Context: ...e manual installation (not external CLI) - Content better covered in: how-react-on-...
(QB_NEW_EN)
[grammar] ~387-~387: There might be a mistake here.
Context: ...-rails-works.md, using-react-on-rails.md - Removed link from installation-into-an-e...
(QB_NEW_EN)
[grammar] ~388-~388: There might be a mistake here.
Context: ...stallation-into-an-existing-rails-app.md - Decision discussed with team in Slack ...
(QB_NEW_EN)
[grammar] ~389-~389: There might be a mistake here.
Context: ... - Decision discussed with team in Slack - Deleted original `docs/getting-started.m...
(QB_NEW_EN)
docs/getting-started/tutorial.md
[grammar] ~36-~36: There might be a mistake here.
Context: ...mr-vs-react-hot-reloading) - Deployment - Going Further - [Turn...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...eployment](#deployment) - Going Further - [Turning on Server Rendering](#turning-on...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...urther) - Turning on Server Rendering - [Optional Configuration](#optional-config...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...r-rendering) - Optional Configuration - [Moving from the Rails default `/app/java...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...to the recommended/client` structure](#moving-from-the-rails-default-appjavascript-to-the-recommended-client-structure) - [Custom IP & PORT setup (Cloud9 example)]...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...Custom IP & PORT setup (Cloud9 example) - [RubyMine performance tip](#rubymine-perf...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...example) - RubyMine performance tip - Conclusion # Installation...
(QB_NEW_EN)
[uncategorized] ~147-~147: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...out a special React setup, will cause a full page refresh each time you save a file. If ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~151-~151: There might be a mistake here.
Context: ...k at these links: - webpack-dev-server - [DevServer](https://webpack.js.org/config...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...docs/webpack-dev-server.md) - DevServer - [Hot Module Replacement](https://webpack....
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...t.md)** - Step-by-step Heroku deployment - **[General Deployment Guide](../deployment/...
(QB_NEW_EN)
[grammar] ~168-~168: There might be a mistake here.
Context: ... guides cover: - Configuring buildpacks - Database setup (PostgreSQL) - Asset comp...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...buildpacks - Database setup (PostgreSQL) - Asset compilation - Environment variable...
(QB_NEW_EN)
[grammar] ~170-~170: There might be a mistake here.
Context: ...e setup (PostgreSQL) - Asset compilation - Environment variables - Troubleshooting ...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ...sset compilation - Environment variables - Troubleshooting common deployment issues...
(QB_NEW_EN)
docs/getting-started/using-react-on-rails.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...me back here to understand the concepts. --- ## Installation Overview When you install ...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ... app/javascript/src/*/ror_components/) - Rails integration (controllers, views, i...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ration (controllers, views, initializer) - Webpack configuration for building JavaS...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...guration for building JavaScript bundles - Development workflow with hot module rep...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...-start.md)** - Fastest path (15 minutes) - **[Installation Guide](./installation-into-...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...ails-app.md)** - For existing Rails apps - Complete Tutorial - St...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...*Complete Tutorial** - Step-by-step with Redux and routing --- ## ...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ..._rails.rb`: - Server rendering settings - Development vs production behavior - Log...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...ngs - Development vs production behavior - Logging options - Auto-bundling settings...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...vs production behavior - Logging options - Auto-bundling settings For complete con...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...o manual ReactOnRails.register() calls 5. Components are loaded on-demand per page...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ... - Eliminates boilerplate configuration - Automatic code splitting per component -...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...- Automatic code splitting per component - Smaller initial bundle sizes - Component...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...component - Smaller initial bundle sizes - Components only loaded when used For co...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: ...orld_controller.rb- Example controller - **View**:app/views/hello_world/index.htm...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...- Showsreact_componenthelper usage - **Route**: Added toconfig/routes.rb` - *...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...enthelper usage - **Route**: Added toconfig/routes.rb- **Initializer**:config/initializers/reac...
(QB_NEW_EN)
[grammar] ~149-~149: There might be a mistake here.
Context: ...arting both: - Rails server (port 3000) - Webpack dev server (for hot reloading) ...
(QB_NEW_EN)
[grammar] ~205-~205: There might be a mistake here.
Context: ...- All React on Rails errors are of type ReactOnRails::Error - Server rendering errors include context ...
(QB_NEW_EN)
[grammar] ~219-~219: There might be a mistake here.
Context: ...t-and-redux.md)** - Add state management - **[React Router](../building-features/react...
(QB_NEW_EN)
[grammar] ~220-~220: There might be a mistake here.
Context: ...react-router.md)** - Client-side routing - **[Server-Side Rendering](../core-concepts/...
(QB_NEW_EN)
[grammar] ~221-~221: There might be a mistake here.
Context: ...ver-rendering.md)** - Deep dive into SSR - **[Internationalization](../building-featur...
(QB_NEW_EN)
[grammar] ~222-~222: There might be a mistake here.
Context: ...g-features/i18n.md)** - Add i18n support - **[Testing](../building-features/rspec-conf...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ...md)** - Production deployment strategies - **[Heroku Deployment](../deployment/heroku-...
(QB_NEW_EN)
[grammar] ~228-~228: There might be a mistake here.
Context: ...roku-deployment.md)** - Deploy to Heroku - **[Troubleshooting](../deployment/troublesh...
(QB_NEW_EN)
[grammar] ~233-~233: There might be a mistake here.
Context: ...-splitting.md)** - Optimize bundle sizes - **[Webpack Configuration](../core-concepts/...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ...-configuration.md)** - Customize webpack - **[Different Client/Server Code](../buildin...
(QB_NEW_EN)
[grammar] ~239-~239: There might be a mistake here.
Context: ...)** - Complete react_component options - **[JavaScript API](../api-reference/javascr...
(QB_NEW_EN)
[grammar] ~240-~240: There might be a mistake here.
Context: ....md)** - ReactOnRails JavaScript methods - **[Configuration](../api-reference/configur...
(QB_NEW_EN)
docs/planning/docs-improvement/04-ia-redesign-plan.md
[grammar] ~813-~813: There might be a mistake here.
Context: ...it) - PLANNED: Move installation → docs/getting-started/installation.md - ACTUAL: Transformed to `docs/getting-s...
(QB_NEW_EN)
[grammar] ~814-~814: There might be a mistake here.
Context: ...ng-react-on-rails.md` (conceptual guide) - REASON: installation.md already existe...
(QB_NEW_EN)
[grammar] ~820-~820: There might be a mistake here.
Context: ...plified) - DECIDED: Option B (kept as simple TOC) - Reduced from 173 → 65 lin...
(QB_NEW_EN)
[grammar] ~820-~820: There might be a mistake here.
Context: ...DECIDED:** Option B (kept as simple TOC) - Reduced from 173 → 65 lines - Serves Git...
(QB_NEW_EN)
[grammar] ~821-~821: There might be a mistake here.
Context: ...imple TOC) - Reduced from 173 → 65 lines - Serves GitHub users browsing repo **4. ...
(QB_NEW_EN)
[grammar] ~822-~822: There might be a mistake here.
Context: ...om 173 → 65 lines - Serves GitHub users browsing repo 4. Delete docs/home.md: ✅ D...
(QB_NEW_EN)
[grammar] ~832-~832: There might be a mistake here.
Context: ...eployment (139 lines) to dedicated guide - Fixed outdated versions (Ruby 3.0+, Rail...
(QB_NEW_EN)
[grammar] ~833-~833: There might be a mistake here.
Context: ... versions (Ruby 3.0+, Rails 7+, RoR v16) - Clarified Redux vs Hooks usage - Merged ...
(QB_NEW_EN)
[grammar] ~834-~834: There might be a mistake here.
Context: ...oR v16) - Clarified Redux vs Hooks usage - Merged duplicate HMR sections - Reorgani...
(QB_NEW_EN)
[grammar] ~835-~835: There might be a mistake here.
Context: ...ks usage - Merged duplicate HMR sections - Reorganized "Other features" → "Going Fu...
(QB_NEW_EN)
[grammar] ~840-~840: There might be a mistake here.
Context: ...view.md`** ✅ DONE - Outdated since 2018 - Confused purpose - No clear use case (ge...
(QB_NEW_EN)
[grammar] ~841-~841: There might be a mistake here.
Context: ...- Outdated since 2018 - Confused purpose - No clear use case (generator IS manual i...
(QB_NEW_EN)
[grammar] ~847-~847: There might be a mistake here.
Context: ...s as they were discovered - Key changes: - overview.md → introduction.md (2 links) ...
(QB_NEW_EN)
[grammar] ~848-~848: There might be a mistake here.
Context: ... overview.md → introduction.md (2 links) - understanding-react-on-rails.md → using-...
(QB_NEW_EN)
[grammar] ~849-~849: There might be a mistake here.
Context: ...ls.md → using-react-on-rails.md (1 link) - manual-installation-overview.md link rem...
(QB_NEW_EN)
[grammar] ~850-~850: There might be a mistake here.
Context: ...lation-overview.md link removed (1 link) - NOTE: Did not use automated script - m...
(QB_NEW_EN)
[grammar] ~855-~855: There might be a mistake here.
Context: ...hecklist:** - [x] All new files created - [x] Old files deleted or transformed - [...
(QB_NEW_EN)
[grammar] ~856-~856: There might be a mistake here.
Context: ...d - [x] Old files deleted or transformed - [x] All internal links updated (manually...
(QB_NEW_EN)
[grammar] ~857-~857: There might be a mistake here.
Context: ... links updated (manually as encountered) - [ ] Run markdown link checker (TODO: bef...
(QB_NEW_EN)
[uncategorized] ~858-~858: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ted (manually as encountered) - [ ] Run markdown link checker (TODO: before final merge)...
(MARKDOWN_NNP)
[grammar] ~858-~858: There might be a mistake here.
Context: ... link checker (TODO: before final merge) - [x] Manual spot-checks of navigation - [...
(QB_NEW_EN)
[grammar] ~859-~859: There might be a mistake here.
Context: ...) - [x] Manual spot-checks of navigation - [ ] Test on local gatsby build (deferred...
(QB_NEW_EN)
docs/getting-started/quick-start.md
[grammar] ~162-~162: There might be a mistake here.
Context: ...on-rails.md)** - Core concepts explained 2. **[View Helpers API](../api-reference/view-...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
docs/deployment/heroku-deployment.md
100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/getting-started/tutorial.md
7-7: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/getting-started/using-react-on-rails.md
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: build
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
- GitHub Check: build-and-test
🔇 Additional comments (3)
docs/README.md (1)
45-55: Categories look good; ensure paths exist.Links under Documentation Categories should be validated post-merge; otherwise LGTM.
docs/planning/docs-improvement/ia-redesign-live.md (1)
326-397: Entry point consolidation summary looks solid.Good documentation of actions and remaining website config step. No changes requested.
docs/introduction.md (1)
45-63: Confirm docs homepage configuration
All linked targets exist. Verify that introduction.md is set as the docs homepage in your site configuration.
| **Testing checklist:** | ||
|
|
||
| - [ ] All new files created | ||
| - [ ] Old files deleted or split | ||
| - [ ] All internal links updated | ||
| - [ ] Run markdown link checker | ||
| - [ ] Manual spot-checks of navigation | ||
| - [ ] Test on local gatsby build | ||
| - [x] All new files created | ||
| - [x] Old files deleted or transformed | ||
| - [x] All internal links updated (manually as encountered) | ||
| - [ ] Run markdown link checker (TODO: before final merge) | ||
| - [x] Manual spot-checks of navigation | ||
| - [ ] Test on local gatsby build (deferred to Phase 1 - website config) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Run a link check before merge (open TODO).
Automate the link validation across docs.
🏁 Script executed:
#!/bin/bash
# Simple local link verifier for relative markdown links
set -euo pipefail
rg -n --type=md -o '\[[^]]+\]\((\.\.?/[^)#\s]+)' | sed -E 's/.*\((.*)/\1/' | while read -r p; do
if [ -f "$p" ]; then
echo "OK: $p"
else
echo "MISSING: $p"
fi
doneLength of output: 12064
Broken relative markdown links detected — fix and automate validation.
- Resolve all missing link targets flagged by the link-check script (e.g.,
MONOREPO_MERGER_PLAN_REF.md,docs/guides/webpack-configuration.md, etc.). - Implement an automated markdown link-checking step (e.g.,
markdown-link-check) in CI and mark the checklist item complete.
🧰 Tools
🪛 LanguageTool
[grammar] ~855-~855: There might be a mistake here.
Context: ...hecklist:** - [x] All new files created - [x] Old files deleted or transformed - [...
(QB_NEW_EN)
[grammar] ~856-~856: There might be a mistake here.
Context: ...d - [x] Old files deleted or transformed - [x] All internal links updated (manually...
(QB_NEW_EN)
[grammar] ~857-~857: There might be a mistake here.
Context: ... links updated (manually as encountered) - [ ] Run markdown link checker (TODO: bef...
(QB_NEW_EN)
[uncategorized] ~858-~858: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ted (manually as encountered) - [ ] Run markdown link checker (TODO: before final merge)...
(MARKDOWN_NNP)
[grammar] ~858-~858: There might be a mistake here.
Context: ... link checker (TODO: before final merge) - [x] Manual spot-checks of navigation - [...
(QB_NEW_EN)
[grammar] ~859-~859: There might be a mistake here.
Context: ...) - [x] Manual spot-checks of navigation - [ ] Test on local gatsby build (deferred...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/planning/docs-improvement/04-ia-redesign-plan.md around lines 853 to
861, the testing checklist calls out broken relative markdown links and a
missing automated validation step; fix every reported missing link target (e.g.,
create or correct MONOREPO_MERGER_PLAN_REF.md, fix paths like
docs/guides/webpack-configuration.md or update links to their new locations) and
add a CI job that runs a markdown link checker (for example markdown-link-check
or remark-link-check) against the repo before merge, failing the build on broken
links; update the checklist to mark the link-check item complete after adding
and validating the CI step.
Add missing H1 headings and improve H1 quality across 10 documentation files to enable proper Algolia search indexing and improve accessibility. Critical fixes (missing H1): - configuration.md: Add "React on Rails Configuration Options" - react-on-rails-pro.md: Change H2 to H1 - rails-engine-integration.md: Add "Integrating React on Rails with Rails Engines" - troubleshooting-when-using-webpacker.md: Add H1 and fix structure - troubleshooting-when-using-shakapacker.md: Add H1 and fix heading hierarchy - credits.md: Add "React on Rails Credits and Authors" Quality improvements (better searchability): - images.md: "Images" → "Configuring Images and Assets with Webpack" - foreman-issues.md: "Foreman Issues" → "Troubleshooting Foreman with React on Rails" - updating-dependencies.md: "Updating Dependencies" → "Updating Ruby and JavaScript Dependencies" - api-reference/README.md: "API Reference" → "React on Rails API Reference Guide" Why: Algolia indexes H1 as primary title. Files without H1 show as "Untitled" in search results. Generic H1s reduce searchability. Reference: docs/planning/docs-improvement/h1-headings-audit-report.md (local file, not tracked)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/advanced-topics/rails-engine-integration.md (2)
16-17: Update React on Rails version to v16 to match site-wide requirements.This page still pins to '~> 6', conflicting with the PR’s v16 baseline. Align the gem specs.
Apply:
- s.add_dependency 'react_on_rails', '~> 6' + s.add_dependency 'react_on_rails', '~> 16'- Place `gem 'react_on_rails', '~> 6'` before the gem pointing at your engine in your gemfile. + Place `gem 'react_on_rails', '~> 16'` before the gem pointing at your engine in your Gemfile.Based on PR objectives.
Also applies to: 27-27
7-11: Update helper include for v16/Rails 7+The view helper is now namespaced under
ReactOnRails::Helper, so the snippet should read:ActiveSupport.on_load(:action_view) do include ReactOnRails::Helper enddocs/building-features/images.md (2)
3-6: Remove TODO placeholders and complete the guidance.The list is incomplete and contains “todo reference”. Please finalize wording and add authoritative links.
Apply this edit for concise, actionable guidance:
-1. leading slash necessary on the - a. Option name for the file-loader and url-loader (todo reference) - b. Option publicPath for the output (todo reference) +1. Leading slash guidance + a. Prefer configuring `output.publicPath` (or `assetModuleFilename` in Webpack 5) instead of adding a leading slash in loader `name` patterns. + b. If using legacy `file-loader`/`url-loader`, avoid leading slashes in `name`; rely on `publicPath`. See: + - https://webpack.js.org/guides/asset-modules/ + - https://webpack.js.org/configuration/output/#outputpublicpath + - https://webpack.js.org/loaders/file-loader/ (deprecated), https://webpack.js.org/loaders/url-loader/ (deprecated)
15-17: Fix contradictory “Leading slash is 100% needed” and align with Webpack 5 best practices.Comments say a leading slash is required, but one rule lacks it; also this guidance is outdated. Use consistent names without a leading slash and configure
publicPath(or Asset Modules in Webpack 5).Apply this minimal fix to the snippet:
- // Leading slash is 100% needed - name: 'images/[hash].[ext]', + name: 'images/[hash].[ext]', ... - // Leading slash is 100% needed - name: '/images/[hash].[ext]', + name: 'images/[hash].[ext]',Prefer updating the doc to Webpack 5 Asset Modules (recommended):
// webpack.config.js (excerpt) module.exports = { output: { publicPath: '/assets/', // adjust to your CDN/host path assetModuleFilename: 'images/[hash][ext][query]', }, module: { rules: [ { test: /\.(png|jpe?g|gif|ico|svg)$/i, type: 'asset', // 'asset/resource' to always emit a file parser: { dataUrlCondition: { maxSize: 8 * 1024 } }, // replaces url-loader limit }, { test: /\.(woff2?|ttf|eot)$/i, type: 'asset/resource', generator: { filename: 'fonts/[hash][ext][query]' }, }, ], }, };Also applies to: 25-27
🧹 Nitpick comments (9)
docs/building-features/foreman-issues.md (2)
3-8: Tighten heading and link phrasing; use Markdown link.Minor copy polish for clarity and consistency with other docs.
-## It is not recommended to include foreman into Gemfile - -See: https://github.com/ddollar/foreman +## Do not add Foreman to your Gemfile + +See the Foreman README: [github.com/ddollar/foreman](https://github.com/ddollar/foreman) > Ruby users should take care not to install foreman in their project's Gemfile.
11-15: Re-validate “Known issues” against current baseline or mark as historical.These notes cite specific versions (foreman 0.82.0, bundler variants). Given the PR updates our recommended stack (Ruby 3.0+, Rails 7, React on Rails v16), please:
- Confirm these still reproduce; otherwise mark them as “Historical” with tested versions/dates, or remove.
- If still relevant, add “Tested with:” versions and a short workaround.
docs/deployment/troubleshooting-when-using-webpacker.md (1)
1-2: Good addition; add deprecation note and cross-link to Shakapacker troubleshooting.Webpacker is deprecated. Add a short note at the top pointing readers to the Shakapacker doc to reduce confusion for new setups.
# Troubleshooting Webpacker Manifest Errors +> Note: Webpacker is deprecated in favor of Shakapacker. For current setups, see +> [Troubleshooting Shakapacker Configuration Issues](troubleshooting-when-using-shakapacker.md).docs/deployment/troubleshooting-when-using-shakapacker.md (2)
68-76: Minor grammar tweak in the Solution section.“Tense” issue. Prefer present simple.
-As it set the `optimization.runtimeChunk` to `single`. [See its source](https://github.com/shakacode/shakapacker/blob/cdf32835d3e0949952b8b4b53063807f714f9b24/package/environments/base.js#L115-L119): +This sets `optimization.runtimeChunk` to `single`. [See source](https://github.com/shakacode/shakapacker/blob/cdf32835d3e0949952b8b4b53063807f714f9b24/package/environments/base.js#L115-L119):
5-11: Align context versions with the PR’s updated requirements or clarify scope.PR updates minimums to Ruby 3.0+, Rails 7+, React on Rails v16. This page lists React on Rails 13.3.5. Either:
- Update to v16.x if confirmed applicable, or
- Explicitly note “Observed on React on Rails 13.3.5; behavior may apply to newer versions.”
Optional diff if verified to apply to v16:
-4. React on Rails version: 13.3.5 +4. React on Rails version: 16.xdocs/advanced-topics/rails-engine-integration.md (2)
35-35: Fix heading level/capitalization.Use a subsection and proper brand capitalization.
-# Github Issues +## GitHub Issues
25-29: Add cross-link to new guides.Improve navigation by linking to the consolidated intro/usage docs.
- Add a sentence: “See Introduction and Using React on Rails for setup context” with links to
docs/introduction.mdanddocs/getting-started/using-react-on-rails.md.docs/building-features/images.md (2)
36-47: Align image paths between narrative and alias.Text references “/client/app/assets/images” but the alias targets “app/assets/images”. Pick one and keep consistent with current templates (Webpacker/Shakapacker).
Apply this if the canonical path is under client/:
- resolve: { - alias: { - images: join(process.cwd(), 'app', 'assets', 'images'), - }, - }, + resolve: { + alias: { + images: join(process.cwd(), 'client', 'app', 'assets', 'images'), + }, + },Also ensure the snippet shows importing
join:const { join } = require('path');If your current default is app/javascript or app/packs, update both the prose and alias accordingly.
34-34: Use repository-relative link for ImageExample.jsxImageExample.jsx exists at spec/dummy/client/app/startup/ImageExample.jsx — update docs/building-features/images.md:34 to link to that path (use a repo-relative path or the correct repo/branch URL instead of the external shakacode/react_on_rails link).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/advanced-topics/rails-engine-integration.md(1 hunks)docs/api-reference/README.md(1 hunks)docs/api-reference/configuration.md(1 hunks)docs/building-features/foreman-issues.md(1 hunks)docs/building-features/images.md(1 hunks)docs/deployment/troubleshooting-when-using-shakapacker.md(2 hunks)docs/deployment/troubleshooting-when-using-webpacker.md(1 hunks)docs/misc/credits.md(1 hunks)docs/misc/updating-dependencies.md(1 hunks)docs/pro/react-on-rails-pro.md(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- docs/api-reference/README.md
- docs/pro/react-on-rails-pro.md
- docs/api-reference/configuration.md
- docs/misc/credits.md
- docs/misc/updating-dependencies.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
docs/deployment/troubleshooting-when-using-webpacker.mddocs/building-features/images.mddocs/advanced-topics/rails-engine-integration.mddocs/building-features/foreman-issues.mddocs/deployment/troubleshooting-when-using-shakapacker.md
🧠 Learnings (1)
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
docs/deployment/troubleshooting-when-using-shakapacker.md
🪛 LanguageTool
docs/deployment/troubleshooting-when-using-shakapacker.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...iple` ### Context 1. Ruby version: 3.1 2. Rails version: 7.0.6 3. Shakapacker vers...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...uby version: 3.1 2. Rails version: 7.0.6 3. Shakapacker version: 6.6.0 4. React on R...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ion: 7.0.6 3. Shakapacker version: 6.6.0 4. React on Rails version: 13.3.5 ### The ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
docs/deployment/troubleshooting-when-using-shakapacker.md
54-54: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: claude-review
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: build
- GitHub Check: markdown-link-check
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build-and-test
🔇 Additional comments (5)
docs/building-features/foreman-issues.md (1)
1-1: No internal references to#foreman-issuesfound; legacy anchor optional.
We searched all.mdfiles and found no links targeting#foreman-issues. Add<a id="foreman-issues"></a>only if you must preserve external deep-links to this heading.docs/deployment/troubleshooting-when-using-shakapacker.md (1)
1-78: Reorg reads clearly and aligns with deployment docs.Sectioning, examples, and solution are clear. Nice job.
docs/advanced-topics/rails-engine-integration.md (2)
1-2: Clear, descriptive H1 added.Good improvement to page structure and findability.
31-34: Asset precompile guidance remains accurate in v16. REACT_ON_RAILS_PRECOMPILE (no|false|n|f) is still supported to disable React on Rails’s integration withrails assets:precompile; for Rails 7 use jsbundling-rails (esbuild/webpack/rollup) + Propshaft or Sprockets and configurebuild_production_command(or rely on the default) sorails assets:precompiletriggers JS bundling.docs/building-features/images.md (1)
1-1: Title update LGTM.Clearer, aligns with IA direction.
| Configuring Webpack to embed the runtime in each chunk and calling `react_component` twice in a Rails view/partial causes the client render to crash. | ||
|
|
||
| Read more at https://github.com/shakacode/react_on_rails/issues/1558. | ||
|
|
||
| ## Solution | ||
| ### Solution |
There was a problem hiding this comment.
Replace bare URL with a labeled Markdown link (fixes MD034).
Use a labeled link to satisfy markdownlint and improve readability.
-Read more at https://github.com/shakacode/react_on_rails/issues/1558.
+Read more at [shakacode/react_on_rails#1558](https://github.com/shakacode/react_on_rails/issues/1558).📝 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.
| Configuring Webpack to embed the runtime in each chunk and calling `react_component` twice in a Rails view/partial causes the client render to crash. | |
| Read more at https://github.com/shakacode/react_on_rails/issues/1558. | |
| ## Solution | |
| ### Solution | |
| Configuring Webpack to embed the runtime in each chunk and calling `react_component` twice in a Rails view/partial causes the client render to crash. | |
| Read more at [shakacode/react_on_rails#1558](https://github.com/shakacode/react_on_rails/issues/1558). | |
| ### Solution |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
54-54: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In docs/deployment/troubleshooting-when-using-shakapacker.md around lines 52 to
56 the plain URL "https://github.com/shakacode/react_on_rails/issues/1558" is
used as a bare link which triggers markdownlint MD034; replace the bare URL with
a labeled Markdown link (e.g., [react_on_rails issue
#1558](https://github.com/shakacode/react_on_rails/issues/1558)) so the
reference is readable and lint-compliant.
Continue fixing H1 heading issues by addressing files with multiple H1s. Demoted duplicate H1s to H2 throughout documentation while maintaining proper heading hierarchy (H1→H2→H3→H4). Files changed: - tutorial.md: Demoted 6 H1s (Table of Contents, Installation, HMR, Deployment, Going Further, Conclusion) - rails-engine-integration.md: Changed "Github Issues" H1 to H2 - hmr-and-hot-reloading-with-the-webpack-dev-server.md: Fixed 2 H1s with proper hierarchy - i18n.md: "Notes" H1 to H2 - react-and-redux.md: "Redux Reducers" H1 to H2 - react-router.md: "Server Rendering Using React Router V4" H1 to H2 - rspec-configuration.md: 2 H1s to H2s - generator-details.md: "Understanding the Organization..." H1 to H2 - redux-store-api.md: "More Details" H1 to H2 - view-helpers-api.md: "More details" H1 to H2 - style.md: "Git Usage" H1 to H2 Rationale: Multiple H1s per document confuse Algolia search ranking, screen readers, and violate accessibility standards. Each document should have exactly one H1 as the main title. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Reorganize turbolinks.md content to clearly separate modern Turbo (recommended) from legacy Turbolinks support. All Turbolinks v2 and v5 content is now properly nested under "Legacy Turbolinks Support" section. Key structural changes: - "Using Turbo" section remains at H2 level (modern, recommended approach) - Created "Legacy Turbolinks Support" H2 section with clear deprecation notice - Nested all Turbolinks content as H3-H5 subsections: - Why Turbolinks? (H3) - Requirements and "Why Not" sections (H3) - Installation details with checklists (H3→H4→H5) - Turbolinks 5 Specific Information (H3) - Technical Details and Troubleshooting (H3→H4) Content improvements: - Added explicit support status for Turbolinks 5.x and 2.x - Clarified auto-detection behavior for legacy versions - Moved CSRF/MIME type technical details under proper troubleshooting section - Updated messaging from "may be outdated" to "still supported, migrate when possible" Rationale: Based on codebase investigation (pageLifecycle.ts, turbolinksUtils.ts), Turbo is the recommended navigation system since 2021. All Turbolinks versions are legacy but still supported. Original flat structure mixed modern and legacy content without clear distinction, potentially confusing users about which system to use. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This commit updates documentation that was 9.5 years outdated, removing references to directory structures and patterns that haven't existed since 2016 and replacing them with accurate modern (2025) guidance. ## Changes to docs/api-reference/generator-details.md - Replace outdated structure description with accurate modern organization - Document both non-Redux and Redux generator structures separately - Add visual directory trees showing actual generated code locations - Document previously undocumented --typescript option - Remove references to non-existent app/lib folder (removed Apr 2016) - Remove incorrect path app/javascript/app/bundles (never existed - was docs typo) - Add auto-bundling explanation with cross-reference - Show real component structure: src/ComponentName/ror_components/ ## Changes to docs/building-features/react-router.md - Update path reference from legacy client/app/bundles to modern src/ structure - Clarify this example applies to --redux generator option - Make path reference generic rather than specific to outdated structure ## Changes to docs/getting-started/project-structure.md Complete rewrite to reflect modern React on Rails: ### Removed outdated content: - Old bundles/ structure presented as current (was 2015-2016 pattern) - "Moving node_modules to /client" section (tested and proven broken with Shakapacker) - References to /client/app/assets directory (generator stopped creating in Apr 2016) - Outdated CSS/assets management discussion ### Added modern content: - Modern auto-bundling structure as recommended approach - Traditional manual structure as legacy option with clear use cases - Decision guide for choosing between approaches - CSS Modules documentation (default in generator since modern versions) - Real code examples from actual generator templates - Rails Asset Pipeline as alternative approach - Advanced global styles pattern ## Historical Context Research revealed: - Oct 2015: Generator created client/app/lib/middlewares/ and client/app/bundles/ - Apr 5, 2016: Docs added describing app/lib folder - Apr 23, 2016: Generator removed these directories (18 days later!) - Apr 2016 - Oct 2025: Docs never updated - outdated for 9.5 years ## Testing Performed Created test app at /home/ihab/ihab/work/shakacode/test/default-structure-test/: - Verified default generator creates src/ structure, not bundles/ - Verified CSS modules co-located with components - Tested /client conversion: works perfectly (just move + config change) - Tested moving node_modules to /client: FAILS with Shakapacker - Confirmed SHAKAPACKER_CONFIG env var doesn't solve the issue 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/getting-started/tutorial.md (1)
52-62: Align the Rails version guidance.These lines conflict: the prerequisites claim support for Rails 6+, but the immediate instruction requires Rails 7.0 or above. Please clarify which versions this tutorial actually targets and update the wording (and section heading examples) so readers aren’t sent mixed signals.
♻️ Duplicate comments (1)
docs/getting-started/tutorial.md (1)
7-7: Resolve markdownlint MD036 by switching to a callout.Line 7 still uses italics as a faux heading, which keeps tripping markdownlint. Replace the italic line with a blockquote note (e.g.,
> **Note:** …) to satisfy the rule.
🧹 Nitpick comments (2)
docs/misc/style.md (1)
38-40: Clarify the CI/coverage instruction.Consider rephrasing to make the action explicit, e.g., “Ensure hosted CI completes successfully and review the code coverage results,” so readers know what outcome to verify rather than just “run” it.
docs/api-reference/generator-details.md (1)
50-58: Add code fence languages for tree snippets.The two directory tree examples render as plain fenced blocks without a language hint, which triggers MD040 and loses syntax highlighting. Please mark them as
textfences.-``` +```text app/javascript/ └── src/ └── HelloWorld/ └── ror_components/ # Components auto-registered by React on Rails ├── HelloWorld.jsx # Your React component ├── HelloWorld.module.css └── HelloWorld.server.js # Optional: separate server rendering logic@@
-+text
app/javascript/
└── src/
└── HelloWorldApp/
├── actions/ # Redux action creatorsAlso applies to: 70-90 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 54dc6a6808bb86604c624d96182a0349e232013c and e2333c4ed52782445afe194a351477734d337448. </details> <details> <summary>📒 Files selected for processing (13)</summary> * `docs/advanced-topics/rails-engine-integration.md` (2 hunks) * `docs/api-reference/generator-details.md` (1 hunks) * `docs/api-reference/redux-store-api.md` (1 hunks) * `docs/api-reference/view-helpers-api.md` (1 hunks) * `docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md` (2 hunks) * `docs/building-features/i18n.md` (1 hunks) * `docs/building-features/react-and-redux.md` (1 hunks) * `docs/building-features/react-router.md` (2 hunks) * `docs/building-features/rspec-configuration.md` (2 hunks) * `docs/building-features/turbolinks.md` (5 hunks) * `docs/getting-started/project-structure.md` (2 hunks) * `docs/getting-started/tutorial.md` (13 hunks) * `docs/misc/style.md` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (3)</summary> * docs/api-reference/redux-store-api.md * docs/building-features/i18n.md * docs/building-features/react-and-redux.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Prettier is the sole authority for formatting all non-Ruby files; never manually format them Files: - `docs/api-reference/view-helpers-api.md` - `docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md` - `docs/api-reference/generator-details.md` - `docs/getting-started/tutorial.md` - `docs/building-features/rspec-configuration.md` - `docs/misc/style.md` - `docs/building-features/react-router.md` - `docs/advanced-topics/rails-engine-integration.md` - `docs/getting-started/project-structure.md` - `docs/building-features/turbolinks.md` </details> </details><details> <summary>🧠 Learnings (3)</summary> <details> <summary>📚 Learning: 2025-09-16T08:01:11.146Z</summary>Learnt from: justin808
PR: #1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntaximport * as style from './file.module.css'rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.**Applied to files:** - `docs/building-features/rspec-configuration.md` - `docs/getting-started/project-structure.md` </details> <details> <summary>📚 Learning: 2025-04-26T21:55:55.874Z</summary>Learnt from: alexeyr-ci2
PR: #1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files underapp-react16directories are copied/moved to corresponding/appdirectories during the conversion process (removing the-react16suffix), which affects their relative import paths at runtime.**Applied to files:** - `docs/getting-started/project-structure.md` </details> <details> <summary>📚 Learning: 2025-02-13T16:50:26.861Z</summary>Learnt from: AbanoubGhadban
PR: #1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 usingturbolinksVersion5(), always ensureTurbolinksexists first by checkingturbolinksInstalled()to prevent TypeError when accessing properties.**Applied to files:** - `docs/building-features/turbolinks.md` </details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md</summary> [grammar] ~17-~17: Use a hyphen to join words. Context: ...v-server` during development. ## Server Side Rendering with the Default shakacod... (QB_NEW_EN_HYPHEN) --- [grammar] ~19-~19: There might be a mistake here. Context: ..., then you have a couple of options. The recommended technique is to have a diffe... (QB_NEW_EN) --- [grammar] ~20-~20: There might be a mistake here. Context: ...fferent Webpack configuration for server rendering. ### If you use the same Webp... (QB_NEW_EN) </details> <details> <summary>docs/api-reference/generator-details.md</summary> [grammar] ~60-~60: There might be a mistake here. Context: ...ource directory for all React components - **`ror_components/`**: Directory name is configurable via `co... (QB_NEW_EN) --- [grammar] ~61-~61: There might be a mistake here. Context: ...via `config.components_subdirectory` in `config/initializers/react_on_rails.rb` - **Auto-registration**: Components in `ror_... (QB_NEW_EN) --- [grammar] ~94-~94: There might be a mistake here. Context: ..." components that receive data via props - **`containers/`**: Container "smart" components connected... (QB_NEW_EN) --- [grammar] ~95-~95: There might be a mistake here. Context: ... Container "smart" components connected to Redux store - **`actions/`** and **`red... (QB_NEW_EN) --- [grammar] ~96-~96: There might be a mistake here. Context: ...**`reducers/`**: Standard Redux patterns - **`ror_components/`**: Entry point files that initialize Redu... (QB_NEW_EN) --- [grammar] ~115-~115: There might be a mistake here. Context: ...ed for use with `react_component` helper 4. Loaded on-demand when used in views For detai... (QB_NEW_EN) </details> <details> <summary>docs/getting-started/tutorial.md</summary> [grammar] ~36-~36: There might be a mistake here. Context: ...mr-vs-react-hot-reloading) - [Deployment](#deployment) - [Going Further](#going-further) - [Turn... (QB_NEW_EN) --- [grammar] ~37-~37: There might be a mistake here. Context: ...eployment](#deployment) - [Going Further](#going-further) - [Turning on Server Rendering](#turning-on... (QB_NEW_EN) --- [grammar] ~38-~38: There might be a mistake here. Context: ...urther) - [Turning on Server Rendering](#turning-on-server-rendering) - [Optional Configuration](#optional-config... (QB_NEW_EN) --- [grammar] ~39-~39: There might be a mistake here. Context: ...r-rendering) - [Optional Configuration](#optional-configuration) - [Moving from the Rails default `/app/java... (QB_NEW_EN) --- [grammar] ~40-~40: There might be a mistake here. Context: ...` to the recommended `/client` structure](#moving-from-the-rails-default-appjavascript-to-the-recommended-client-structure) - [Custom IP & PORT setup (Cloud9 example)]... (QB_NEW_EN) --- [grammar] ~41-~41: There might be a mistake here. Context: ...[Custom IP & PORT setup (Cloud9 example)](#custom-ip--port-setup-cloud9-example) - [RubyMine performance tip](#rubymine-perf... (QB_NEW_EN) --- [grammar] ~42-~42: There might be a mistake here. Context: ...example) - [RubyMine performance tip](#rubymine-performance-tip) - [Conclusion](#conclusion) ## Installatio... (QB_NEW_EN) --- [uncategorized] ~147-~147: If this is a compound adjective that modifies the following noun, use a hyphen. Context: ...out a special React setup, will cause a full page refresh each time you save a file. If ... (EN_COMPOUND_ADJECTIVE_INTERNAL) --- [grammar] ~151-~151: There might be a mistake here. Context: ...k at these links: - [webpack-dev-server](https://github.com/rails/webpacker/blob/5-x-stable/docs/webpack-dev-server.md) - [DevServer](https://webpack.js.org/config... (QB_NEW_EN) --- [grammar] ~152-~152: There might be a mistake here. Context: ...docs/webpack-dev-server.md) - [DevServer](https://webpack.js.org/configuration/dev-server/) - [Hot Module Replacement](https://webpack.... (QB_NEW_EN) --- [grammar] ~163-~163: There might be a mistake here. Context: ...t.md)** - Step-by-step Heroku deployment - **[General Deployment Guide](../deployment/... (QB_NEW_EN) --- [grammar] ~168-~168: There might be a mistake here. Context: ... guides cover: - Configuring buildpacks - Database setup (PostgreSQL) - Asset comp... (QB_NEW_EN) --- [grammar] ~169-~169: There might be a mistake here. Context: ...buildpacks - Database setup (PostgreSQL) - Asset compilation - Environment variable... (QB_NEW_EN) --- [grammar] ~170-~170: There might be a mistake here. Context: ...e setup (PostgreSQL) - Asset compilation - Environment variables - Troubleshooting ... (QB_NEW_EN) --- [grammar] ~171-~171: There might be a mistake here. Context: ...sset compilation - Environment variables - Troubleshooting common deployment issues... (QB_NEW_EN) </details> <details> <summary>docs/building-features/react-router.md</summary> [uncategorized] ~7-~7: “React” is a proper noun and needs to be capitalized. Context: ...g with the HelloWorldApp created by the react_on_rails generator (with `--redux` opti... (A_GOOGLE) </details> <details> <summary>docs/building-features/turbolinks.md</summary> [grammar] ~24-~24: There might be a mistake here. Context: ...nks 5.x** (e.g., 5.0.0+) - Auto-detected - **Turbolinks 2.x** (Classic) - Auto-detect... (QB_NEW_EN) --- [grammar] ~25-~25: There might be a mistake here. Context: ...tected - **Turbolinks 2.x** (Classic) - Auto-detected - See [Turbolinks on Github](ht... (QB_NEW_EN) --- [grammar] ~25-~25: There might be a mistake here. Context: ...rbolinks 2.x** (Classic) - Auto-detected - See [Turbolinks on Github](https://githu... (QB_NEW_EN) --- [uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”. Context: ...c) - Auto-detected - See [Turbolinks on Github](https://github.com/rails/turbolinks) ... (GITHUB) --- [grammar] ~32-~32: There might be a mistake here. Context: ...l not reload JavaScript and stylesheets. This definitely can make an app perform ... (QB_NEW_EN) --- [grammar] ~112-~112: There might be a mistake here. Context: ...ges the head element by JavaScript (not only body) on page changes with the correct ... (QB_NEW_EN) --- [grammar] ~113-~113: There might be a mistake here. Context: .../23/mime-type-resolution-in-rails.html). - **Multiple Webpack bundles**: If you're us... (QB_NEW_EN) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/api-reference/generator-details.md</summary> 50-50: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 70-70: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/getting-started/tutorial.md</summary> 7-7: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)</summary> * GitHub Check: build-dummy-app-webpack-test-bundles (3.2, 20) * GitHub Check: rspec-package-tests (3.4, latest) * GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22) * GitHub Check: rspec-package-tests (3.4, minimum) * GitHub Check: rspec-package-tests (3.2, latest) * GitHub Check: rspec-package-tests (3.2, minimum) * GitHub Check: claude-review * GitHub Check: markdown-link-check * GitHub Check: build * GitHub Check: build-and-test </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>docs/building-features/rspec-configuration.md (2)</summary><blockquote> `5-22`: **Heading promotion reads better.** Upgrading this section to an H2 makes the hierarchy consistent under the page title. Looks good. --- `23-47`: **Second heading change also looks right.** Matching the previous section with another H2 keeps the structure tidy. All good. </blockquote></details> <details> <summary>docs/advanced-topics/rails-engine-integration.md (2)</summary><blockquote> `1-2`: **Solid header upgrade.** New H1 clarifies the page scope. Looks good. --- `35-40`: **Useful issue cross-references added.** Additional links and heading tweak improve navigation without regressions. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
|
||
| Turbo is not auto-detected like older Turbolinks. | ||
|
|
||
| _TODO: Walk through code changes in PR 1620._ |
There was a problem hiding this comment.
Remove placeholder TODO before publishing
Line 16 still ships a _TODO note. Please replace it with actual guidance or drop the line so the published docs don’t contain unfinished placeholders.
- _TODO: Walk through code changes in PR 1620._📝 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.
| _TODO: Walk through code changes in PR 1620._ |
🤖 Prompt for AI Agents
In docs/building-features/turbolinks.md around lines 16 to 16, remove the
placeholder "_TODO: Walk through code changes in PR 1620._" by either replacing
it with a short, actionable guidance summarizing the PR changes (e.g., key files
changed and steps to verify) or deleting the line entirely so the published
document contains no unfinished placeholders; ensure the replacement text is
clear, concise, and consistent with surrounding docs tone.
|
|
||
| - **Turbolinks 5.x** (e.g., 5.0.0+) - Auto-detected | ||
| - **Turbolinks 2.x** (Classic) - Auto-detected | ||
| - See [Turbolinks on Github](https://github.com/rails/turbolinks) |
There was a problem hiding this comment.
Fix capitalization of GitHub
Please capitalize “GitHub” correctly in the link text so the branding is accurate.
-- See [Turbolinks on Github](https://github.com/rails/turbolinks)
+- See [Turbolinks on GitHub](https://github.com/rails/turbolinks)📝 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.
| - See [Turbolinks on Github](https://github.com/rails/turbolinks) | |
| - See [Turbolinks on GitHub](https://github.com/rails/turbolinks) |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...c) - Auto-detected - See Turbolinks on Github ...
(GITHUB)
🤖 Prompt for AI Agents
In docs/building-features/turbolinks.md around line 26, the link text uses
“Github” instead of the correct brand capitalization “GitHub”; update the link
text to read "Turbolinks on GitHub" while keeping the URL unchanged so the
branding is accurate.
|
|
||
| ### Professional Support | ||
|
|
||
| - **[ShakaCode Support](mailto:[email protected])** - Professional React on Rails help |
There was a problem hiding this comment.
I'm pretty sure that @justin808 will want this left on the Readme.
There was a problem hiding this comment.
I didn't delete it entirely, it was moved to introduction.md to keep the docs/README focused on technical content and the TOC, while referring readers to the docs website for more in-depth information. do we need to put it here as well regardless?
| @@ -1,4 +1,4 @@ | |||
| ## React on Rails Pro | |||
| # React on Rails Pro | |||
There was a problem hiding this comment.
When are we going to change the link for ReactOnRails Pro on line 10 (and probably in other places as well)?
There was a problem hiding this comment.
to what? do you mean now that we merged? the link on line 10 is pointing to the website not github
This major update modernizes React Router integration from outdated v4/v5 to current v6 API, fixing critical gaps discovered through testing. Documentation Changes (docs/building-features/react-router.md): - Remove 5-year-old "needs updating" warning - Pin React Router to v6 (^6.0.0) with explanation about v7 incompatibility - Add critical "Rails Routes Configuration" section with wildcard routing - Update all code examples to React Router v6 API: * Switch → Routes * component prop → element prop * StaticRouter import from 'react-router-dom/server' - Add path matching guidance (React Router paths must match Rails routes) - Update Turbolinks → Turbo/Turbolinks with Rails version context - Improve clarity: generator doesn't create React Router code - Add installation instructions and compatibility notes spec/dummy Updates (v5→v6 migration): - client/app/routes/routes.jsx: Add Routes wrapper, wildcard path support - client/app/components/RouterLayout.jsx: Switch→Routes, use relative paths - client/app/startup/RouterApp.server.jsx: Update StaticRouter import location - package.json: Upgrade react-router-dom from ^5.2.0 to ^6.0.0 - yarn.lock: Update dependencies (@remix-run/[email protected]) Key Discoveries from Testing: 1. React Router v7 merged with Remix, incompatible with our SSR approach 2. Rails wildcard routing is CRITICAL but was never documented 3. Documentation examples were 9+ years misleading about generator output 4. StaticRouter import location changed in v6 (breaks without this update) Testing: - Created fresh test app following documentation step-by-step - Validated client-side routing, SSR, direct URL visits, browser navigation - Confirmed spec/dummy React Router demo works with v6 API 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace outdated foreman-issues.md with comprehensive process-managers.md guide. Changes: - Create docs/building-features/process-managers.md with modern guidance - Documents both Overmind (recommended) and Foreman - Explains why React on Rails needs multiple processes - Includes authoritative explanation from Foreman's wiki about not bundling - Shows how bin/dev works under the hood - Move docs/building-features/foreman-issues.md to docs/outdated/ - Preserves 2017-era bug reports for historical reference - Removes clutter from main docs (outdated/ filtered from navigation) - Update lib/react_on_rails/dev/process_manager.rb - Change link from our old doc to official Foreman wiki - Users get explanation directly from authoritative source Addresses feedback that foreman-issues.md was outdated and didn't mention modern alternatives like Overmind (which bin/dev actually prefers).
The code-splitting.md file was explicitly marked as "(Outdated)" and directed users to contact [email protected] instead of following it. The modern solution is React on Rails Pro's loadable-components guide. Changes: - Moved docs/building-features/code-splitting.md → docs/outdated/code-splitting.md - Updated API reference to point to Pro loadable-components (appropriate for reference docs) - Removed outdated code-splitting links from beginner guides (quick-start, using-react-on-rails) - Updated planning doc to track the move Rationale: - File content from 2017 for React Router v3/v4 with ExecJS - Uses deprecated patterns (manual renderer functions, webpack v1) - Example uses abandoned react-s3-uploader package - Pro's loadable-components doc (updated Sept 2022) provides modern approach - Uses @loadable/component (React team's official recommendation) - Both docs solve the same problem: code splitting WITH SSR - Outdated doc itself says to contact Justin about loadable-components Decision: Keep Pro feature out of beginner docs to avoid confusion about whether Pro is required. Only mention in API reference where advanced users look for specific features. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Two user-facing docs were linking to docs/outdated/webpack.md which is excluded from the website, causing 404 errors. Changes: - docs/core-concepts/webpack-configuration.md: Updated to link to Shakapacker's webpack customization docs (most relevant for users customizing webpack) - docs/deployment/troubleshooting-build-errors.md: Updated to link to our own webpack-configuration.md (keeps users in our documentation) Context: webpack.md was moved to outdated/ in commit e08f91d (Oct 3, 2025) because it contained "outdated Webpack v2 content". The file references Webpack v2 patterns and links to a 2017 forum post. These two links were not updated when the file was moved, causing broken references. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/planning/docs-improvement/ia-redesign-live.md (1)
402-479: Add language hint to the folder structure code block.The fenced code block lacks a language identifier, triggering markdownlint MD040. Tag it as
textfor consistency.-``` +```text docs/ ├── introduction.md # ✅ Created - new unified homepage ├── README.md # ✅ Simplified for GitHub users
♻️ Duplicate comments (2)
docs/building-features/react-router.md (1)
37-40: Verify the file path matches generator output.A previous review noted that the React on Rails generator with
--reduxscaffolds bundles underapp/javascript/bundles/HelloWorld/...(withstartup,store,components), notsrc/HelloWorldApp/ror_components/. Ensure this path guidance matches the actual generated structure or clarify that it's for auto-bundling setups.#!/bin/bash # Verify the actual structure created by the generator with --redux rg -n "generate react_on_rails:install.*redux" docs/ -A5 -B5docs/getting-started/using-react-on-rails.md (1)
126-130: Add language hint to the code block.The fenced code block showing the directory structure lacks a language identifier, which triggers markdownlint MD040.
-``` +```text app/javascript/ └── bundles/HelloWorld/ # or src/HelloWorld/ror_components/ with auto-bundling └── HelloWorld.jsx
🧹 Nitpick comments (3)
docs/core-concepts/webpack-configuration.md (1)
25-25: Minor: Simplify verbose phrasing.The phrase "In order to" is unnecessarily wordy. Consider using "To" instead for conciseness.
-Typical Shakapacker apps have a standard directory structure as documented [here](https://github.com/shakacode/shakapacker/blob/master/README.md#configuration-and-code). If you follow [the basic tutorial](../getting-started/tutorial.md), you will see this pattern in action. In order to customize the Webpack configuration, consult the [Shakapacker webpack customization docs](https://github.com/shakacode/shakapacker#webpack-configuration). +Typical Shakapacker apps have a standard directory structure as documented [here](https://github.com/shakacode/shakapacker/blob/master/README.md#configuration-and-code). If you follow [the basic tutorial](../getting-started/tutorial.md), you will see this pattern in action. To customize the Webpack configuration, consult the [Shakapacker webpack customization docs](https://github.com/shakacode/shakapacker#webpack-configuration).spec/dummy/package.json (1)
26-26: Consider specifying a more recent React Router v6 baseline.The
^6.0.0range will install the latest v6.x (currently 6.30.1), which is fine. However, React Router v6.4+ introduced route-level loaders, actions, and RouterProvider, so you may want to set a higher baseline like^6.4.0if you plan to use those features.Based on learnings.
- "react-router-dom": "^6.0.0", + "react-router-dom": "^6.4.0",docs/building-features/process-managers.md (1)
81-85: Consider using a standard syntax identifier for the code block.The
procfileidentifier may not be recognized by most Markdown renderers. Consider usingbashor plain code blocks (no identifier) for better compatibility across documentation viewers.-```procfile +```bash rails: bundle exec rails s -p 3000 wp-client: bin/shakapacker-dev-server wp-server: SERVER_BUNDLE_ONLY=true bin/shakapacker --watch</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e2333c4ed52782445afe194a351477734d337448 and 5ed73ee57deecde2c04842619081e1f76eb63641. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `spec/dummy/yarn.lock` is excluded by `!**/yarn.lock`, `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (14)</summary> * `docs/api-reference/view-helpers-api.md` (3 hunks) * `docs/building-features/process-managers.md` (1 hunks) * `docs/building-features/react-router.md` (1 hunks) * `docs/core-concepts/webpack-configuration.md` (1 hunks) * `docs/deployment/troubleshooting-build-errors.md` (1 hunks) * `docs/getting-started/quick-start.md` (2 hunks) * `docs/getting-started/using-react-on-rails.md` (1 hunks) * `docs/outdated/foreman-issues.md` (1 hunks) * `docs/planning/docs-improvement/ia-redesign-live.md` (3 hunks) * `lib/react_on_rails/dev/process_manager.rb` (1 hunks) * `spec/dummy/client/app/components/RouterLayout.jsx` (2 hunks) * `spec/dummy/client/app/routes/routes.jsx` (1 hunks) * `spec/dummy/client/app/startup/RouterApp.server.jsx` (1 hunks) * `spec/dummy/package.json` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * docs/outdated/foreman-issues.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (3)</summary> <details> <summary>**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Prettier is the sole authority for formatting all non-Ruby files; never manually format them Files: - `docs/building-features/process-managers.md` - `docs/planning/docs-improvement/ia-redesign-live.md` - `docs/deployment/troubleshooting-build-errors.md` - `docs/building-features/react-router.md` - `docs/getting-started/quick-start.md` - `spec/dummy/client/app/routes/routes.jsx` - `spec/dummy/package.json` - `spec/dummy/client/app/startup/RouterApp.server.jsx` - `spec/dummy/client/app/components/RouterLayout.jsx` - `docs/api-reference/view-helpers-api.md` - `docs/core-concepts/webpack-configuration.md` - `docs/getting-started/using-react-on-rails.md` </details> <details> <summary>{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > `{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}`: All Ruby code must pass RuboCop with zero offenses before commit/push > RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files Files: - `lib/react_on_rails/dev/process_manager.rb` </details> <details> <summary>**/*.{js,jsx,ts,tsx}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Use ESLint for JS/TS code (lint via rake lint or yarn lint) Files: - `spec/dummy/client/app/routes/routes.jsx` - `spec/dummy/client/app/startup/RouterApp.server.jsx` - `spec/dummy/client/app/components/RouterLayout.jsx` </details> </details><details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-04-26T21:55:55.874Z</summary>Learnt from: alexeyr-ci2
PR: #1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files underapp-react16directories are copied/moved to corresponding/appdirectories during the conversion process (removing the-react16suffix), which affects their relative import paths at runtime.**Applied to files:** - `docs/planning/docs-improvement/ia-redesign-live.md` </details> </details><details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>spec/dummy/client/app/routes/routes.jsx (1)</summary><blockquote> <details> <summary>spec/dummy/client/app/components/RouterLayout.jsx (1)</summary> * `RouterLayout` (6-29) </details> </blockquote></details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/api-reference/view-helpers-api.md</summary> [grammar] ~108-~108: Use a hyphen to join words. Context: ...itting with server rendering, the server rendered code has any async code loaded ... (QB_NEW_EN_HYPHEN) --- [grammar] ~108-~108: Ensure spelling is correct Context: ...he client code must also fully load any asynch code before server rendering. Otherwise... (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) </details> <details> <summary>docs/core-concepts/webpack-configuration.md</summary> [style] ~25-~25: Consider a more concise word here. Context: ...), you will see this pattern in action. In order to customize the Webpack configuration, co... (IN_ORDER_TO_PREMIUM) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/planning/docs-improvement/ia-redesign-live.md</summary> 402-402: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/core-concepts/webpack-configuration.md</summary> 25-25: Link text should be descriptive (MD059, descriptive-link-text) </details> <details> <summary>docs/getting-started/using-react-on-rails.md</summary> 126-126: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)</summary> * GitHub Check: build * GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22) * GitHub Check: markdown-link-check * GitHub Check: rspec-package-tests (3.4, latest) * GitHub Check: build-dummy-app-webpack-test-bundles (3.2, 20) * GitHub Check: rspec-package-tests (3.4, minimum) * GitHub Check: rspec-package-tests (3.2, latest) * GitHub Check: rspec-package-tests (3.2, minimum) * GitHub Check: claude-review * GitHub Check: build-and-test </details> <details> <summary>🔇 Additional comments (18)</summary><blockquote> <details> <summary>docs/deployment/troubleshooting-build-errors.md (1)</summary><blockquote> `279-279`: **Link update is well-formed.** The relative path correctly resolves from `docs/deployment/` to `docs/core-concepts/webpack-configuration.md`, and the link text is descriptive. This update aligns with the PR's documentation restructuring objectives. </blockquote></details> <details> <summary>docs/api-reference/view-helpers-api.md (2)</summary><blockquote> `122-122`: **Approve Pro guide reference addition to React Router section.** The new link to the React on Rails Pro loadable-components guide is well-placed and aligns with the PR's broader initiative to surface Pro guidance across API references. This provides users with a clear path to modern code-splitting solutions with server-side rendering. --- `137-137`: **Verify heading level change is correctly formatted.** The heading for "More details" has been demoted from top-level to secondary-level, which is appropriate for the documentation structure reorganization. Ensure the Markdown syntax (number of `#` symbols) is correct and renders as intended in the published documentation. </blockquote></details> <details> <summary>spec/dummy/client/app/startup/RouterApp.server.jsx (1)</summary><blockquote> `2-2`: **LGTM! Correct StaticRouter import for React Router v6.** In v6, StaticRouter is imported from 'react-router-dom/server', and this change correctly updates the import path for server-side rendering. Based on learnings. </blockquote></details> <details> <summary>docs/getting-started/quick-start.md (2)</summary><blockquote> `127-127`: **Good clarification.** The HTML tag formatting makes the instruction clearer for readers. --- `162-162`: **Link correctly updated to new file location.** The link now points to the newly created `using-react-on-rails.md` file, consistent with the documentation restructuring in this PR. </blockquote></details> <details> <summary>spec/dummy/client/app/routes/routes.jsx (1)</summary><blockquote> `2-10`: **LGTM! Correct React Router v6 migration.** The changes properly implement v6 patterns: - Routes component replaces Switch - Route uses element prop instead of component - Wildcard path (`/react_router/*`) correctly supports nested routing in RouterLayout Based on learnings. </blockquote></details> <details> <summary>spec/dummy/client/app/components/RouterLayout.jsx (2)</summary><blockquote> `2-2`: **Correct imports for React Router v6.** Adding Routes to the imports is necessary for the migration. --- `24-27`: **LGTM! Proper v6 nested routing implementation.** The changes correctly implement React Router v6 patterns: - Routes replaces Switch - Route uses element prop - Nested routes use relative paths by default in v6 Based on learnings. </blockquote></details> <details> <summary>docs/building-features/react-router.md (4)</summary><blockquote> `7-17`: **Excellent Turbo/Turbolinks conflict warning.** The compatibility note clearly explains the routing conflict and provides practical solutions. This will save users significant debugging time. --- `21-29`: **Good guidance on React Router v6 vs v7.** The explanation of why to use v6 instead of v7 (Remix merger) is valuable and aligns with React on Rails' architecture. --- `86-119`: **LGTM! Correct server-side rendering implementation.** The server-side setup properly uses: - StaticRouter from 'react-router-dom/server' - Routes and Route with element prop - `location` from `railsContext` for SSR Based on learnings. --- `128-160`: **Critical Rails routing configuration guidance.** This section is essential for React Router to work correctly with server-side rendering and direct URL visits. The wildcard route pattern is properly explained with clear examples. </blockquote></details> <details> <summary>docs/planning/docs-improvement/ia-redesign-live.md (1)</summary><blockquote> `324-396`: **Well-documented Phase 3 completion.** The detailed tracking of completed actions provides excellent documentation of the restructuring process, including rationale for deletions and consolidations. </blockquote></details> <details> <summary>lib/react_on_rails/dev/process_manager.rb (1)</summary><blockquote> `150-150`: **LGTM! Documentation URL correctly updated.** The URL now points to the official Foreman wiki, which is the authoritative source for this guidance. This change is consistent with the new documentation added in `docs/building-features/process-managers.md` (line 50), which references the same URL. </blockquote></details> <details> <summary>docs/building-features/process-managers.md (3)</summary><blockquote> `1-22`: **LGTM! Clear introduction and usage instructions.** The process list and fallback order accurately reflect the implementation in `lib/react_on_rails/dev/process_manager.rb`. --- `87-91`: **The See Also section is correct.** The HMR documentation file exists at the referenced path. All links in this section have been verified and are valid. --- `23-62`: **All external documentation links are verified and accessible.** The installation instructions for both Overmind and Foreman are clear and accurate. The emphasis on not bundling Foreman is appropriate and well-supported by the official documentation. All external URLs resolve correctly (HTTP 200). </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
The docs/outdated/ folder contained deprecated content that was excluded from the website build. This content is preserved in git history and can be accessed through previous commits if needed. Deleted files: - docs/outdated/code-splitting.md - docs/outdated/converting-from-custom-webpack-config-to-rails-webpacker-config.md - docs/outdated/deferred-rendering.md - docs/outdated/foreman-issues.md - docs/outdated/rails-assets-relative-paths.md - docs/outdated/rails-assets.md - docs/outdated/rails3.md - docs/outdated/tips-for-usage-with-sp6.md - docs/outdated/upgrade-webpacker-v3-to-v4.md - docs/outdated/webpack-v1-notes.md - docs/outdated/webpack.md This cleanup follows the recommendation to rely on git history rather than maintaining hidden documentation in the main branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@claude review this PR |
|
Claude finished @justin808's task —— View job PR Review: Entry Point Consolidation and Tutorial ImprovementsReviewing PR #1860 - Phase 3 of documentation IA redesign. Tasks
Overall AssessmentThis is excellent documentation work that significantly improves the user experience. The consolidation is well-executed and addresses a real problem of multiple conflicting entry points. I have a few minor issues to address and one syntax error to fix. ✅ Strengths1. Entry Point Consolidation (Problem Solved!)The new structure successfully eliminates confusion:
2. Tutorial Improvements
3. React Router v6 MigrationCode changes look correct:
4. Documentation Quality
|
Updated test expectation to match the new Foreman documentation link. In commit 6cc93c7, we updated lib/react_on_rails/dev/process_manager.rb to link to Foreman's official wiki instead of our outdated foreman-issues.md. The test was still expecting the old link pattern. Change: - Old expectation: /foreman-issues\.md/ - New expectation: /Don't-Bundle-Foreman/ This matches the new error message which now points to: https://github.com/ddollar/foreman/wiki/Don't-Bundle-Foreman Fixes the failing spec: ReactOnRails::Dev::ProcessManager.show_process_manager_installation_help displays helpful error message with installation instructions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Updated two broken links in react-router.md that were pointing to the old location ../rails/turbolinks.md which no longer exists. The turbolinks.md file was moved from docs/rails/ to docs/building-features/ during the PR #1845 documentation reorganization, but these links were not updated at that time. Changes: - ../rails/turbolinks.md → ./turbolinks.md (same directory) This fixes the markdown-link-check CI failure: ERROR: 1 dead link found in docs/building-features/react-router.md ! [✖] ../rails/turbolinks.md → Status: 400 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Added future flags to BrowserRouter in the dummy app to enable v7-compatible behavior and eliminate console warnings that were causing test failures. Changes: - spec/dummy/client/app/startup/RouterApp.client.jsx: Added future prop with v7_startTransition and v7_relativeSplatPath flags Why this fixes the issue: The dummy app integration tests were failing because selenium_logger.rb treats JavaScript console warnings as errors. React Router v6 emits warnings about upcoming v7 changes:⚠️ React Router Future Flag Warning: React Router will begin wrapping state updates in React.startTransition in v7. You can use the v7_startTransition future flag to opt-in early.⚠️ React Router Future Flag Warning: Relative route resolution within Splat routes is changing in v7. You can use the v7_relativeSplatPath future flag to opt-in early. By enabling these flags, we: 1. Silence the warnings 2. Adopt v7-compatible behavior early 3. Demonstrate best practices for React Router v6 users 4. Align with React Router's official migration guide Reference: https://reactrouter.com/upgrading/future Fixes test failures: - React Router when rendering /react_router is expected to have text "Woohoo..." - React Router when rendering /react_router clicking links correctly renders... 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Addresses valid issues from Claude's PR review: 1. Fixed JavaScript syntax error in using-react-on-rails.md:192 - Changed semicolon to comma in object destructuring example - Was accidentally introduced during linting commit 2. Clarified Rails version requirements in tutorial.md - Changed "We support Rails 6 and later" to "This tutorial targets Rails 7.0+" - Explicitly noted Rails 6 is supported but tutorial uses Rails 7 - Removed version confusion between prerequisites and tutorial steps 3. Fixed markdownlint issues - Added language hint to code fence in using-react-on-rails.md (text) - Added language hint to Procfile code fence in heroku-deployment.md (procfile) These changes ensure code examples are syntactically correct and version requirements are clear for users following the tutorial.
|
@claude good catches! can you check the fixes i made and did i resolve the issues you stated or not? here's a report: ✅ Fixed Issues
|
|
Claude finished @ihabadham's task —— View job Verification Complete ✅I've reviewed all the fixes and here's my assessment: ✅ Issue 1: JavaScript Syntax Error - FIXEDFile: Status: ✅ Correctly fixed in commit 60e9cea The object literal now correctly uses parentheses: ({
renderedHtml: {
componentHtml,
redirectLocation,
error,
},
});This is the proper syntax for returning an object literal from an arrow function. Well done! ✅ Issue 2: Markdownlint Issues - FIXEDStatus: ✅ Correctly fixed in commit 60e9cea Both issues resolved:
These changes will prevent MD040 linting errors. ✅ Issue 3: Rails Version Confusion - FIXEDFile: Status: ✅ Correctly fixed in commit 60e9cea The clarification is perfect:
This clearly explains:
Excellent resolution!
|
| Issue | Status | Action Required |
|---|---|---|
| 1. JavaScript syntax error | ✅ Fixed | None |
| 2. Markdownlint issues | ✅ Fixed | None |
| 3. Rails version confusion | ✅ Fixed | None |
| 4. Heroku Node/Yarn versions | Optional: Add inline comment (but current disclaimer is adequate) | |
| 5. React Router path | ✅ Correct | None (automated review was mistaken) |
Overall assessment: All critical issues have been properly addressed. The fixes in commit 60e9cea are correct and resolve the identified problems. Your decisions on issues 4 and 5 are well-reasoned and appropriate.
Great work on the fixes! 🎉
Job Run • Branch: feature/docs-ia-redesign-1845-continue
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
docs/deployment/heroku-deployment.md (1)
131-135: Update Node version from EOL 16.x to 18+.Node 16.19.0 is end-of-life and does not match the Node 18+ requirement mentioned in the Quick Start. Users following this guide will deploy on an unsupported runtime.
Apply this diff to align with current requirements:
"engines": { - "node": "16.19.0", + "node": ">=18.0.0", "yarn": "1.22.4" },docs/getting-started/tutorial.md (1)
7-7: Fix MD036 linting violation: convert italics to a proper blockquote.The emphasized line is flagged by markdownlint MD036 (emphasis used instead of a heading). Convert it to a blockquote-style note while preserving the version information.
Apply this diff:
-_Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7_ +> **Note:** Updated for Ruby 3.0+, Rails 7, React on Rails v16, and Shakapacker v7.
🧹 Nitpick comments (3)
docs/deployment/heroku-deployment.md (3)
100-100: Capitalize the Procfile code fence language hint for consistency.The language hint is present (fixing MD040), but convention and the past review comment specify
Procfilewith a capital P.Apply this diff:
-```procfile +```Procfile web: bundle exec puma -C config/puma.rb
77-77: Optional: Reduce redundant phrasing for variety.The phrase "Then you need to set up Postgres" reuses phrasing from nearby sentences. Consider rephrasing for variety:
-Then you need to set up Postgres so you can run locally: +Now set up Postgres locally:Per LanguageTool style guidance.
153-153: Optional: Reduce redundant pronoun usage.The sentence reuses "you" pronouns, which could be tightened:
-and you will see your live app and you can share this URL with your friends. +and you'll see your live app—share the URL with your friends.Per LanguageTool style guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/deployment/heroku-deployment.md(2 hunks)docs/getting-started/tutorial.md(12 hunks)docs/getting-started/using-react-on-rails.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
docs/getting-started/tutorial.mddocs/getting-started/using-react-on-rails.mddocs/deployment/heroku-deployment.md
🪛 LanguageTool
docs/getting-started/tutorial.md
[uncategorized] ~147-~147: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...out a special React setup, will cause a full page refresh each time you save a file. If ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/deployment/heroku-deployment.md
[style] ~77-~77: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... database: ror_production ``` Then you need to set up Postgres so you can run locally:...
(REP_NEED_TO_VB)
[style] ~153-~153: Reusing ‘you’ could be redundant. Try omitting the pronoun.
Context: ...``` and you will see your live app and you can share this URL with your friends. Congr...
(SUBJECT_DROP)
🪛 markdownlint-cli2 (0.18.1)
docs/getting-started/tutorial.md
7-7: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.2, 20)
- GitHub Check: build
- GitHub Check: markdown-link-check
- GitHub Check: build-and-test
🔇 Additional comments (6)
docs/deployment/heroku-deployment.md (1)
1-179: Content structure and organization look solid.The consolidation of Heroku deployment guidance is well-organized with clear step-by-step sections. The introduction disclaimer appropriately notes version age and directs readers to current Heroku Rails 7 guidance. The new sections (app creation, PostgreSQL migration, Puma config, Node/Yarn specification, deployment steps) are comprehensive and follow Heroku best practices.
The main issues to address are the Node version pin (flagged separately) and optional style improvements.
docs/getting-started/using-react-on-rails.md (2)
126-130: Past review issues have been properly addressed.The code fences now include appropriate language hints (
text,js), and the JavaScript object syntax is correct with proper comma separation and no extraneous semicolons. Good attention to detail in resolving the linting violations.Also applies to: 189-197
1-246: Comprehensive documentation with clear progression and helpful examples.The document effectively scaffolds understanding from installation through advanced topics. The code examples are well-commented, all code fences have proper language hints, and the navigation callout at the top appropriately directs new users to the quick-start before returning here. Links to related documentation are consistent and comprehensive.
docs/getting-started/tutorial.md (3)
113-119: Redux vs. Hooks choice is well-clarified.The distinction between the default Hooks-based installer and the optional Redux setup is now explicit and helpful. This addresses a common point of confusion for new users following the tutorial.
157-173: Deployment section properly extracted and well-organized.The new Deployment section appropriately redirects readers to dedicated guides (Heroku and General Deployment) rather than duplicating instructions inline. This aligns with the PR's goal of consolidating deployment content and improves maintainability.
25-43: Table of Contents properly reflects restructured content.The TOC accurately maps the new organization with "Deployment" as a top-level section and "Going Further" replacing "Other features," making the document structure clearer and more logical for users seeking deployment guidance early.
This category index file was an orphaned remnant from PR #1813 that used a different navigation approach with category landing pages. During the IA redesign (PR #1845), similar category index files like guides/advanced/README.md were deleted as "redundant" because docs/README.md now serves as the single navigation hub. The api-reference/README.md survived by accident and is now the only category with such an index file, creating inconsistency. Changes: - Deleted docs/api-reference/README.md (7-line TOC) - Updated docs/README.md reference to point directly to view-helpers-api.md (the main API reference file) - Preserves existing folder-level link on line 50 that points to api-reference/ Rationale: Category folders don't need README indexes when the main docs hub already provides navigation. This matches the pattern established when we deleted guides/advanced/README.md.
3 participants
What Changed
Phase 3 of the IA redesign: consolidated multiple conflicting entry points, improved the tutorial, and cleaned up outdated documentation.
1. Entry Point Consolidation
Created unified homepage:
docs/introduction.md- New comprehensive homepage with clear learning pathsRemoved/simplified conflicting entry points:
docs/home.md- old website homepage (replaced by introduction.md)docs/README.md(173→65 lines) - now serves GitHub users onlydocs/core-concepts/react-on-rails-overview.md- content consolidated into introduction.mdTransformed getting-started guide:
docs/getting-started.md→docs/getting-started/using-react-on-rails.mdCleanup:
docs/guides/advanced/README.md2. Tutorial Improvements
Enhanced
docs/getting-started/tutorial.md:Enhanced
docs/deployment/heroku-deployment.md:3. Documentation Cleanup
Deleted outdated manual installation guide:
docs/advanced-topics/manual-installation-overview.md4. Planning Documentation Updates
Updated tracker docs:
04-ia-redesign-plan.md- marked Phase 3 complete, noted actual vs planned changesia-redesign-live.md- comprehensive Phase 3 completion documentationFiles Changed (10 commits)
Created:
docs/introduction.md(new unified homepage)docs/getting-started/using-react-on-rails.md(transformed from getting-started.md)Modified:
docs/README.md(simplified for GitHub users)docs/getting-started/tutorial.md(extracted deployment, fixed versions, reorganized)docs/deployment/heroku-deployment.md(merged tutorial content)docs/planning/docs-improvement/04-ia-redesign-plan.md(updated status)docs/planning/docs-improvement/ia-redesign-live.md(documented Phase 3)Deleted:
docs/home.mddocs/core-concepts/react-on-rails-overview.mddocs/guides/advanced/README.mddocs/advanced-topics/manual-installation-overview.mdWhy
Problem 1.2: Multiple Conflicting Entry Points
Users faced 4+ different starting points with conflicting information:
README.md(GitHub)home.md(website homepage)getting-started.md(mixed purposes)react-on-rails-overview.md(outdated prereqs)Each had different content, creating confusion about where to start.
Tutorial overwhelm:
Outdated documentation:
What's Next
Website Configuration (sc-website repo)
gatsby-node.jsto useintroduction.mdas homepage/docs/planning/from website buildFuture phases:
(Future phases are repo cleanup tasks - website filtering will hide them from users)
Review Focus
introduction.mdprovide a clear, unified starting point?using-react-on-rails.mdmore focused as a conceptual guide?manual-installation-overview.mdthe right call?Note: Planning docs in
/docs/planning/docs-improvement/are for development only and will be filtered from the website in Phase 1.This change is
Summary by CodeRabbit