-
-
Notifications
You must be signed in to change notification settings - Fork 638
docs: Entry Point Consolidation and Tutorial Improvements #1860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughLarge documentation IA rework: new docs/introduction.md and trimmed docs/README.md; many outdated docs removed or reorganized; getting-started split and new using-react-on-rails.md added; Heroku/deployment and troubleshooting content updated; dummy app/specs migrated to react-router-dom v6; a process-manager help message URL updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client Browser
participant Rails as Rails App
participant ServerEntry as Server Entry (SSR)
participant ReactApp as React Router v6 App
Note left of Rails #white : Server-side render path
Browser->>Rails: GET /some/path
Rails->>ServerEntry: renderToString(StaticRouter location=/some/path)
ServerEntry->>ReactApp: Render with StaticRouter + Routes/Route
ReactApp-->>ServerEntry: HTML string
ServerEntry-->>Rails: return server HTML
Rails-->>Browser: HTML response
Browser->>ReactApp: Client hydrate with BrowserRouter (Routes/Route)
Note right of Browser #white : Client routing uses element-based Route (v6)
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)
Comment |
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.md
docs/misc/doctrine.md
docs/getting-started/using-react-on-rails.md
docs/introduction.md
docs/README.md
docs/deployment/heroku-deployment.md
docs/getting-started/tutorial.md
docs/planning/docs-improvement/04-ia-redesign-plan.md
docs/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
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: localhost Suggestion: 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! 🎉
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.md
for 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:setup
andrails db:migrate
to 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_tag
are 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.md
docs/getting-started/using-react-on-rails.md
docs/planning/docs-improvement/ia-redesign-live.md
docs/README.md
docs/getting-started/tutorial.md
docs/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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Procfile
language 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.md
docs/planning/docs-improvement/ia-redesign-live.md
docs/deployment/heroku-deployment.md
docs/getting-started/tutorial.md
docs/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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
5e71aeb
to
2cf9420
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.md
docs/deployment/heroku-deployment.md
docs/introduction.md
docs/planning/docs-improvement/ia-redesign-live.md
docs/getting-started/tutorial.md
docs/getting-started/using-react-on-rails.md
docs/planning/docs-improvement/04-ia-redesign-plan.md
docs/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: ...- Shows
react_componenthelper usage - **Route**: Added to
config/routes.rb` - *...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...enthelper usage - **Route**: Added to
config/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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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
done
Length 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.md
anddocs/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.md
docs/building-features/images.md
docs/advanced-topics/rails-engine-integration.md
docs/building-features/foreman-issues.md
docs/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-issues
found; legacy anchor optional.
We searched all.md
files 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:precompile
triggers 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
text
fences.-``` +```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-react16
directories are copied/moved to corresponding/app
directories during the conversion process (removing the-react16
suffix), 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 ensureTurbolinks
exists 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
||
### Professional Support | ||
|
||
- **[ShakaCode Support](mailto:[email protected])** - Professional React on Rails help |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure that @justin808 will want this left on the Readme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to what? do you mean now that we merged? the link on line 10 is pointing to the website not github
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
text
for 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
--redux
scaffolds 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.0
range 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.0
if 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
procfile
identifier may not be recognized by most Markdown renderers. Consider usingbash
or 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-react16
directories are copied/moved to corresponding/app
directories during the conversion process (removing the-react16
suffix), 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 -->
@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
|
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Procfile
with 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.md
docs/getting-started/using-react-on-rails.md
docs/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.
- 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"
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)
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]>
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]>
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]>
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.
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.
52d4a18
to
98ca50a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
docs/building-features/turbolinks.md (2)
16-16
: Remove or replace the TODO placeholder before publishing.Line 16 still contains the unfinished placeholder
_TODO: Walk through code changes in PR 1620._
This was flagged in a prior review and should not ship in published documentation. Either replace it with a brief summary of key changes from PR 1620 or remove the line entirely.-_TODO: Walk through code changes in PR 1620._ +_For details on the changes that enabled Turbo support, see [PR 1620](https://github.com/shakacode/react_on_rails/pull/1620)._Or simply delete the line if a summary is not needed.
26-26
: Fix GitHub branding capitalization.Line 26 uses the incorrect branding "Github". The official name is "GitHub" (capital H). This was flagged in a prior review and should be corrected.
-- See [Turbolinks on Github](https://github.com/rails/turbolinks) +- See [Turbolinks on GitHub](https://github.com/rails/turbolinks)docs/building-features/react-router.md (1)
35-78
: Redux + React Router v6 client-side setup is correct.The BrowserRouter pattern with Routes/Route and element prop correctly reflects v6 syntax. The Redux Provider wrapping is the appropriate pattern. The file path reference (
app/javascript/src/HelloWorldApp/ror_components/
) aligns with auto-bundling, which was confirmed in the PR objectives as correct. The key points section (lines 72–77) accurately summarizes v5→v6 changes. The previous review concern about this path has been addressed.docs/deployment/troubleshooting-when-using-shakapacker.md (1)
54-54
: Convert bare URL to Markdown link format.This bare URL triggers markdownlint MD034. Use a labeled link instead for both compliance and readability.
Apply this fix:
-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).Note: This issue was flagged in the previous review and remains unresolved.
docs/getting-started/tutorial.md (1)
7-7
: Fix MD036 violation: convert italicized heading to a blockquote.Using italics as a heading violates markdownlint MD036. Convert to a blockquote or callout format to maintain the visual prominence while satisfying the linter.
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 (2)
docs/getting-started/project-structure.md (1)
15-20
: Clarify file extension conventions for server-side rendering files.The structure diagram shows inconsistent file extensions:
HelloWorld.server.js
(line 16) vs.AnotherComponent.server.jsx
(line 20). This creates ambiguity about whether server-only files should use.js
or.jsx
.Consider standardizing to
.jsx
for consistency with the rest of the component setup, or add a clarifying note explaining when to use each extension (e.g., if.js
is intended for non-JSX server logic and.jsx
for JSX). Also clarify the distinction between:
- A single
.jsx
file with an optional.server.js
companion (HelloWorld pattern)- Separate
.client.jsx
and.server.jsx
files (AnotherComponent pattern)Suggested improvement:
app/javascript/ ├── src/ │ ├── HelloWorld/ │ │ ├── HelloWorld.module.css │ │ └── ror_components/ # Auto-discovered by React on Rails │ │ ├── HelloWorld.jsx # Client & server rendering -│ │ └── HelloWorld.server.js # Optional: server-only code +│ │ └── HelloWorld.server.jsx # Optional: server-only code │ └── AnotherComponent/ │ └── ror_components/ │ ├── AnotherComponent.client.jsx # Client-only rendering │ └── AnotherComponent.server.jsx # Server-only renderingAlternatively, expand the comments to explain the pattern choice for each example.
docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md (1)
17-17
: Use consistent hyphenation for compound modifiers."Server Side Rendering" should be hyphenated as "Server-Side Rendering" when used as a compound adjective.
Apply this fix:
-## Server Side Rendering with the Default shakacode/shakapacker bin/shakapacker-dev-server +## Server-Side Rendering with the Default shakacode/shakapacker bin/shakapacker-dev-server
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
spec/dummy/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (56)
docs/README.md
(3 hunks)docs/advanced-topics/manual-installation-overview.md
(0 hunks)docs/advanced-topics/rails-engine-integration.md
(2 hunks)docs/api-reference/README.md
(0 hunks)docs/api-reference/configuration.md
(1 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
(3 hunks)docs/building-features/code-splitting.md
(0 hunks)docs/building-features/foreman-issues.md
(0 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/images.md
(1 hunks)docs/building-features/process-managers.md
(1 hunks)docs/building-features/react-and-redux.md
(1 hunks)docs/building-features/react-router.md
(1 hunks)docs/building-features/rspec-configuration.md
(2 hunks)docs/building-features/turbolinks.md
(5 hunks)docs/core-concepts/react-on-rails-overview.md
(0 hunks)docs/core-concepts/webpack-configuration.md
(1 hunks)docs/deployment/heroku-deployment.md
(2 hunks)docs/deployment/troubleshooting-build-errors.md
(1 hunks)docs/deployment/troubleshooting-when-using-shakapacker.md
(2 hunks)docs/deployment/troubleshooting-when-using-webpacker.md
(1 hunks)docs/getting-started.md
(0 hunks)docs/getting-started/installation-into-an-existing-rails-app.md
(0 hunks)docs/getting-started/project-structure.md
(2 hunks)docs/getting-started/quick-start.md
(2 hunks)docs/getting-started/tutorial.md
(12 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/credits.md
(1 hunks)docs/misc/doctrine.md
(1 hunks)docs/misc/style.md
(1 hunks)docs/misc/updating-dependencies.md
(1 hunks)docs/outdated/converting-from-custom-webpack-config-to-rails-webpacker-config.md
(0 hunks)docs/outdated/deferred-rendering.md
(0 hunks)docs/outdated/rails-assets-relative-paths.md
(0 hunks)docs/outdated/rails-assets.md
(0 hunks)docs/outdated/rails3.md
(0 hunks)docs/outdated/tips-for-usage-with-sp6.md
(0 hunks)docs/outdated/upgrade-webpacker-v3-to-v4.md
(0 hunks)docs/outdated/webpack-v1-notes.md
(0 hunks)docs/outdated/webpack.md
(0 hunks)docs/planning/docs-improvement/04-ia-redesign-plan.md
(1 hunks)docs/planning/docs-improvement/ia-redesign-live.md
(3 hunks)docs/pro/react-on-rails-pro.md
(1 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.client.jsx
(1 hunks)spec/dummy/client/app/startup/RouterApp.server.jsx
(1 hunks)spec/dummy/package.json
(1 hunks)spec/react_on_rails/dev/process_manager_spec.rb
(1 hunks)
💤 Files with no reviewable changes (18)
- docs/building-features/foreman-issues.md
- docs/building-features/code-splitting.md
- docs/outdated/rails-assets.md
- docs/api-reference/README.md
- docs/guides/advanced/README.md
- docs/home.md
- docs/advanced-topics/manual-installation-overview.md
- docs/outdated/rails-assets-relative-paths.md
- docs/getting-started.md
- docs/outdated/deferred-rendering.md
- docs/outdated/upgrade-webpacker-v3-to-v4.md
- docs/outdated/rails3.md
- docs/outdated/webpack.md
- docs/outdated/converting-from-custom-webpack-config-to-rails-webpacker-config.md
- docs/outdated/webpack-v1-notes.md
- docs/getting-started/installation-into-an-existing-rails-app.md
- docs/core-concepts/react-on-rails-overview.md
- docs/outdated/tips-for-usage-with-sp6.md
✅ Files skipped from review due to trivial changes (3)
- docs/pro/react-on-rails-pro.md
- docs/building-features/react-and-redux.md
- docs/deployment/troubleshooting-when-using-webpacker.md
🚧 Files skipped from review as they are similar to previous changes (12)
- spec/react_on_rails/dev/process_manager_spec.rb
- docs/misc/updating-dependencies.md
- docs/building-features/images.md
- docs/api-reference/configuration.md
- docs/api-reference/redux-store-api.md
- docs/api-reference/view-helpers-api.md
- docs/building-features/process-managers.md
- lib/react_on_rails/dev/process_manager.rb
- docs/advanced-topics/rails-engine-integration.md
- docs/misc/credits.md
- docs/getting-started/using-react-on-rails.md
- spec/dummy/client/app/startup/RouterApp.server.jsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
spec/dummy/client/app/routes/routes.jsx
spec/dummy/client/app/components/RouterLayout.jsx
docs/api-reference/generator-details.md
docs/planning/docs-improvement/ia-redesign-live.md
docs/deployment/heroku-deployment.md
docs/README.md
docs/building-features/react-router.md
spec/dummy/package.json
docs/getting-started/quick-start.md
docs/introduction.md
docs/building-features/turbolinks.md
docs/planning/docs-improvement/04-ia-redesign-plan.md
docs/misc/doctrine.md
docs/building-features/i18n.md
docs/deployment/troubleshooting-build-errors.md
docs/getting-started/project-structure.md
spec/dummy/client/app/startup/RouterApp.client.jsx
docs/building-features/rspec-configuration.md
docs/getting-started/tutorial.md
docs/misc/style.md
docs/deployment/troubleshooting-when-using-shakapacker.md
docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md
docs/core-concepts/webpack-configuration.md
**/*.{js,jsx,ts,tsx}
📄 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/components/RouterLayout.jsx
spec/dummy/client/app/startup/RouterApp.client.jsx
🧠 Learnings (3)
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#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 under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/planning/docs-improvement/ia-redesign-live.md
docs/getting-started/project-structure.md
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
docs/building-features/turbolinks.md
📚 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/getting-started/project-structure.md
docs/building-features/rspec-configuration.md
docs/deployment/troubleshooting-when-using-shakapacker.md
🧬 Code graph analysis (2)
spec/dummy/client/app/routes/routes.jsx (1)
spec/dummy/client/app/components/RouterLayout.jsx (1)
RouterLayout
(6-29)
spec/dummy/client/app/startup/RouterApp.client.jsx (1)
spec/dummy/client/app/startup/RouterApp.server.jsx (1)
props
(6-10)
🪛 LanguageTool
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)
docs/building-features/turbolinks.md
[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)
docs/planning/docs-improvement/04-ia-redesign-plan.md
[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)
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/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md
[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)
docs/core-concepts/webpack-configuration.md
[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)
🪛 markdownlint-cli2 (0.18.1)
docs/api-reference/generator-details.md
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)
docs/planning/docs-improvement/ia-redesign-live.md
402-402: 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)
⏰ 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). (11)
- 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: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: markdown-link-check
- GitHub Check: lint-js-and-ruby
- GitHub Check: build
- GitHub Check: build-dummy-app-webpack-test-bundles
🔇 Additional comments (36)
docs/misc/style.md (2)
36-36
: Heading restructuring aligns with documentation IA redesign.The change from primary (#) to secondary (##) heading places "Git Usage" at the same level as other major sections ("Client Side JavaScript and React", "Style Guides to Follow"), creating a consistent information hierarchy. This is consistent with the PR's broader heading-level adjustments.
38-40
: Clarify the intent of the new CI/code coverage bullet.Line 40 introduces "Run hosted CI and code coverage" alongside the git flow and rebase guidance, but it's noticeably brief and semantically distinct from the preceding bullets. Confirm this bullet is complete as written—should it be expanded with context on when or how to run CI (e.g., before pushing, as part of the merge checklist), or is the terseness intentional as a standalone checklist item?
docs/building-features/i18n.md (1)
95-99
: LGTM — proper heading hierarchy and clear outdated label.Correcting the "Notes" section from h1 to h2 establishes proper semantic hierarchy under the main h1 heading. The "Outdated:" label clearly signals to readers that the referenced tutorial may not reflect current best practices, which is a valuable documentation improvement as part of the IA redesign cleanup.
docs/api-reference/generator-details.md (6)
50-58
: Add language hint to code block (MD040).Markdownlint flags this code block as missing a language specification. Per the PR comments, this should have been fixed in commit 60e9cea, but the issue persists. Add a language hint after the opening fence (e.g.,
```text
or```bash
).
70-90
: Add language hint to code block (MD040).Markdownlint flags this code block as missing a language specification. Per the PR comments, this should have been fixed in commit 60e9cea, but the issue persists. Add a language hint after the opening fence (e.g.,
```text
or```bash
).
42-118
: Content restructuring looks solid.The reorganization of generator documentation into default vs. Redux modes is clearer and more useful. Directory structures are accurate and well-annotated. The new TypeScript section is concise, and the auto-bundling reference points to the right guide. One minor observation: if components use separate
.server.jsx
files, consider adding a note that this is optional and explain the trade-offs (e.g., when to use server vs. client components).
50-58
: Add language hint to code block (MD040).Markdownlint requires a language specification on fenced code blocks. This directory tree is missing one. Per the PR comments, commit 60e9cea should have fixed this, but the issue persists. Add a language tag after the opening fence (e.g.,
```text
or```bash
).
70-90
: Add language hint to code block (MD040).Markdownlint requires a language specification on fenced code blocks. This directory tree is missing one. Per the PR comments, commit 60e9cea should have fixed this, but the issue persists. Add a language tag after the opening fence (e.g.,
```text
or```bash
).
42-118
: Content restructuring is clear and well-organized.The reorganization into default vs. Redux modes significantly improves clarity. Directory structures are accurate, annotations are helpful, and the TypeScript section is concise. The reference to the Auto-Bundling Guide keeps documentation DRY. One optional enhancement: consider adding a brief note on line 64 about when to use
.server.jsx
vs..client.jsx
or link to a reference.docs/building-features/react-router.md (4)
1-33
: Clear setup guidance and v6-focused explanation.The introduction, compatibility note, and installation section are well-structured. The distinction between Declarative Mode and Data Mode (lines 31–33) correctly explains why Data Mode doesn't fit React on Rails' server-side data loading pattern. One minor note: the statement on line 29 that "React Router v7 has merged with Remix" would benefit from verification against the latest React Router release notes to ensure the compatibility claim is current.
[web]
Has React Router v7 merged with Remix or adopted a fundamentally different architecture incompatible with SSR?
[/web]
79-127
: Server-side StaticRouter setup correctly uses v6 patterns.The StaticRouter import from
'react-router-dom/server'
(line 88) and the Routes/Route+element pattern are correct for v6. The location prop assignment and renderToString usage align with React Router v6 SSR guidance. The "Important changes" section (lines 121–126) accurately contrasts v5 and v6 approaches. One clarification: verify that the return format on line 115 ({ renderedHtml: html }
) matches React on Rails' server-side rendering expectations, as the exact return shape may depend on your Rails integration layer.
128-170
: Rails routing guidance is clear and practical.The wildcard route pattern (lines 134–143) and the Rails-to-React Router path alignment explanation (lines 161–169) correctly address the critical setup needed for SSR and direct URL visits. The concrete example showing
/hello_world
and nested routes is helpful and prevents a common integration mistake. This section provides solid operational guidance.
171-182
: External resources and tutorial references are well-contextualized.The links to the React Webpack Rails Tutorial and official React Router documentation are valuable. The note on line 177 that the tutorial uses legacy directory structure but patterns remain applicable is a helpful caveat. The v5 migration guide link supports users who are upgrading.
spec/dummy/client/app/components/RouterLayout.jsx (1)
2-2
: LGTM! Clean React Router v6 migration.The migration correctly updates to v6 conventions:
Switch
→Routes
,component
prop →element
prop with JSX, and relative paths for nested routes.Also applies to: 24-27
spec/dummy/client/app/startup/RouterApp.client.jsx (1)
8-11
: Good practice: enabling v7 future flags incrementally.The implementation correctly enables two future flags (
v7_startTransition
andv7_relativeSplatPath
) to prepare for React Router v7. Additional flags likev7_fetcherPersist
,v7_normalizeFormMethod
,v7_partialHydration
, andv7_skipActionErrorRevalidation
can be enabled and tested one at a time as needed. Based on learnings.spec/dummy/client/app/routes/routes.jsx (1)
2-2
: LGTM! Correct v6 pattern with wildcard for nested routes.The migration properly wraps the Route in Routes, uses the
element
prop, and adds the/*
wildcard to the path. The wildcard is essential here becauseRouterLayout
contains its own nested<Routes>
(in spec/dummy/client/app/components/RouterLayout.jsx lines 24-27), allowing those child routes to match relative paths.Also applies to: 6-9
spec/dummy/package.json (1)
21-22
: React 19 is compatible with react-router-dom v6 — no action needed.React-router-dom v6 specifies a permissive peerDependency (react >=16.8) and React 19 satisfies this requirement. The test suite dependencies are properly aligned.
docs/getting-started/project-structure.md (3)
103-108
: CSS Module import syntax is correct.The
import * as style from './HelloWorld.module.css'
pattern aligns correctly with React on Rails' webpack CSS Modules configuration (namedExports: true). The example and benefits section provide clear guidance on modern styling practices.
1-153
: Overall structure and guidance are comprehensive and well-organized.The dual-structure approach (Modern Auto-Bundling recommended, Traditional Manual legacy) aligns well with the PR objectives to modernize documentation. The "Choosing Your Structure" section provides clear decision criteria, and the styling guidance covers modern (CSS Modules), traditional (Rails Asset Pipeline), and advanced (global styles) approaches effectively. The file successfully consolidates entry-point guidance and removes outdated boilerplate references.
35-35
: No issues found—links are valid and resolve correctly.Both referenced documentation files exist at the specified locations. The relative paths
../core-concepts/auto-bundling-file-system-based-automated-bundle-generation.md
and../api-reference/generator-details.md
resolve correctly fromdocs/getting-started/project-structure.md
.docs/building-features/rspec-configuration.md (1)
5-5
: LGTM!Heading level adjustments are appropriate for the documentation restructuring.
Also applies to: 23-23
docs/deployment/troubleshooting-build-errors.md (1)
279-279
: LGTM!The link update to the correct webpack configuration guide is appropriate and aligns with the documentation consolidation.
docs/getting-started/quick-start.md (1)
127-127
: LGTM!Navigation updates appropriately direct users to the new consolidated documentation page. The text clarification improves readability without altering meaning.
Also applies to: 162-162
docs/misc/doctrine.md (1)
7-7
: LGTM!The updated link correctly references the new consolidated introduction.md, maintaining the doctrine's connection to the project overview.
docs/deployment/heroku-deployment.md (1)
3-3
: LGTM!The disclaimer note properly sets expectations about version currency and links to official Heroku Rails 7 guidance.
docs/introduction.md (1)
1-132
: Internal links verified—no broken references found.All 14 internal cross-references in introduction.md resolve successfully to their target documentation files. The file is ready as a consolidated entry point.
docs/getting-started/tutorial.md (2)
25-43
: Navigation structure is clear and up-to-date.The table of contents reflects the restructured content with new "Going Further" section and separate "Deployment" section. This aligns well with the PR's consolidation objectives and deployment content extraction.
157-172
: Deployment guide links verified and correct.Both target files exist at their expected locations:
docs/deployment/heroku-deployment.md
✓docs/deployment/deployment.md
✓The relative paths from
docs/getting-started/tutorial.md
resolve correctly.docs/planning/docs-improvement/ia-redesign-live.md (3)
326-396
: Phase 3 completion is well-documented with clear actions and rationale.The Actions Completed subsection provides concrete details for each of the seven major changes (file deletions, creations, transformations). Rationale is provided for each decision, making the actual vs. planned changes transparent. This aligns with the PR objectives and serves as a valuable reference for future phases.
400-478
: Reference: Final Folder Structure is comprehensive and accurate.The file tree shows the complete post-Phase-3 structure with all moves, creations, and deletions accounted for. The inline comments clarifying file status (✅ Created, ✅ Simplified, etc.) improve clarity. This serves as an effective reference artifact for validation.
480-485
: Final Decisions section appropriately documents permanent category choices.The note that
misc/
andadvanced-topics/
are now permanent categories (not temporary) clarifies the outcome of the reorganization steps. The statement "All orphaned files found homes" is a good summary of completeness.docs/core-concepts/webpack-configuration.md (1)
25-25
: Link update to external Shakapacker docs is accurate and helpful.The change from a local "Webpack Tips" reference to the upstream Shakapacker webpack customization docs improves documentation quality by directing users to the authoritative source. The anchor text is clear and descriptive.
docs/planning/docs-improvement/04-ia-redesign-plan.md (2)
799-843
: Phase 3 completion is well-documented with transparent adaptations.The status line clearly references the live tracker for details. The ADDITIONS subsection (lines 828–851) transparently documents unplanned improvements (tutorial enhancements, manual-installation-overview.md deletion, cross-reference updates). This shows adaptability and responsiveness to findings during implementation, which is good for a major reorganization.
853-860
: Markdown link checker automation already exists and is active—no implementation needed.The repository already has a fully configured GitHub Actions workflow (
.github/workflows/check-markdown-links.yml
) that automatically validates markdown links on every PR and push to master. The workflow usestcort/[email protected]
with a config file at.github/markdown-link-check-config.json
, targeting thedocs/
folder.The "TODO: before final merge" at line 858 refers to manual verification that the CI check passed, not to implementing link-checker automation. The workflow will run automatically on this PR due to markdown file changes—no manual shell script execution is required.
Likely an incorrect or invalid review comment.
docs/README.md (2)
11-54
: Navigation structure is logical and user-friendly with clear learning paths.The simplified README now provides three clear entry points (Beginner, Experienced, Migrating) instead of the previous cluttered structure. The Popular Use Cases table adds actionable quick links, and the Documentation Categories section provides a comprehensive overview. This aligns with the PR's goal of consolidating entry points and improving navigation clarity.
3-64
: All relative links in the simplified README.md resolve correctly—approved.Verification confirms all 20 unique link paths in the README exist in the documentation structure. The simplified navigation correctly references the primary user-facing categories (Getting Started, Core Concepts, API Reference, Building Features, Deployment, Upgrading, Migrating, Pro), while additional filesystem directories (advanced-topics, contributor-info, assets, images, misc, testimonials) are appropriately omitted from the main navigation for clarity.
"engines": { | ||
"node": "16.19.0", | ||
"yarn": "1.22.4" | ||
}, | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Update Node version to match documented system requirements.
The introduction.md and quick-start.md specify Node 18+ as a requirement, but this guide still pins Node to 16.19.0 (EOL since 2023). This creates version inconsistency across documentation and could lead to deployments on unsupported runtimes.
Update the engines block to reflect the current minimum requirement:
"engines": {
- "node": "16.19.0",
+ "node": "18.0.0",
"yarn": "1.22.4"
},
Please verify that Node 18+ is indeed the minimum requirement for React on Rails 16 with Shakapacker 6+, and confirm whether the Yarn version also needs updating to a version compatible with Node 18+.
I'll verify the version requirements by checking the codebase and confirming the claims made in the review comment.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check Node version references in introduction.md and quick-start.md
echo "=== Checking introduction.md for Node requirements ==="
if [ -f "docs/introduction.md" ]; then
grep -n -i "node\|version\|18\|16" docs/introduction.md | head -20
else
echo "File not found: docs/introduction.md"
fi
echo ""
echo "=== Checking quick-start.md for Node requirements ==="
if [ -f "docs/quick-start.md" ]; then
grep -n -i "node\|version\|18\|16" docs/quick-start.md | head -20
else
echo "File not found: docs/quick-start.md"
fi
echo ""
echo "=== Checking heroku-deployment.md engines section ==="
if [ -f "docs/deployment/heroku-deployment.md" ]; then
sed -n '125,140p' docs/deployment/heroku-deployment.md
else
echo "File not found: docs/deployment/heroku-deployment.md"
fi
echo ""
echo "=== Checking root package.json for Node/Yarn requirements ==="
if [ -f "package.json" ]; then
jq '.engines' package.json
fi
echo ""
echo "=== Checking Gemfile for Rails/Shakapacker versions ==="
if [ -f "Gemfile" ]; then
grep -i "rails\|shakapacker" Gemfile
fi
Length of output: 805
🌐 Web query:
Node.js 16.19.0 end of life date
💡 Result:
Node.js 16.19.0 reached end-of-life on September 11, 2023. [1][2]
Sources:
- Node.js announcement moving Node.js 16 EOL to September 11, 2023. [1]
- Node.js Releases / EOL listing showing v16 (Gallium) as EoL. [2]
🌐 Web query:
Yarn 1.22.4 compatibility Node 18
💡 Result:
Short answer: Yes — Yarn Classic 1.22.4 will run on Node 18 in practice, but 1.22.4 is old (released 2020) so you should upgrade to the latest Yarn 1.x (1.22.19) or to a maintained Yarn 2+/Berry release for best compatibility and bug/CI fixes. Test your project (install/build scripts, native modules) under Node 18 in CI before rolling out. Sources: Yarn Classic docs and release history. [1][2]
Sources
- Yarn Classic install / package.json docs. [1]
- Heroku changelog showing Yarn 1.22.4 (context on release timing). [2]
🌐 Web query:
React on Rails 16 Shakapacker 6 Node 18 requirements
💡 Result:
Short answer: Node 18 is NOT sufficient for React on Rails 16. React on Rails 16 requires Node.js 20+; Shakapacker 6 supports Node.js 14+ (so Node 18 is fine for Shakapacker). [1][2]
Sources:
- React on Rails upgrade / v16 notes (requires Node.js 20 minimum). [1]
- Shakapacker README / repo (prerequisite Node.js 14+ / 12.13+). [2]
🌐 Web query:
React on Rails v16 minimum Node version requirement documentation
💡 Result:
React on Rails v16 requires Node.js >= 20. [1][2]
Sources:
- Upgrade guide / requirements (React on Rails v16): Shakacode docs. [1]
- 16.0.0 release notes (lists Node 20 and CI-tested 20–22). [2]
🌐 Web query:
Shakapacker 6 minimum Node version supported
💡 Result:
Shakapacker v6 requires Node.js 12.13.0+ (or Node.js 14+). [1][2]
Sources:
- shakapacker npm README (v6.x). [1]
- shakapacker GitHub README / prerequisites. [2]
Update both documentation files to specify Node 20+ as minimum requirement.
The inconsistency between introduction.md and heroku-deployment.md is valid, and Node 16.19.0 is indeed EOL (September 2023). However, the suggested fix is incomplete: React on Rails v16 requires Node.js >= 20, not 18.
Additionally, introduction.md currently specifies "Node.js 18+" which is also incorrect for React on Rails 16.
Required changes:
- docs/deployment/heroku-deployment.md (lines 131-135): Update to
"node": "20.0.0"
- docs/introduction.md (line 107): Update to
Node.js 20+
Regarding Yarn: Yarn 1.22.4 will run on Node 18 in practice, but should upgrade to the latest Yarn 1.x (1.22.19) or maintained Yarn 2+/Berry releases for best compatibility.
🤖 Prompt for AI Agents
In docs/deployment/heroku-deployment.md around lines 131-135 and
docs/introduction.md at line 107, the Node minimum is outdated (16/18); update
the Heroku JSON engines node entry to "20.0.0" and change the introduction text
to state "Node.js 20+"; while here, also bump the Yarn recommendation from
1.22.4 to a maintained release (e.g., 1.22.19) or recommend Yarn 2+/Berry for
best compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we have a note about this in tutorial.md or in this file saying why we specify this version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search tutorial for Node/Yarn rationale =="
if [ -f "docs/getting-started/tutorial.md" ]; then
rg -n -C2 -i 'node\.?js|yarn|engines|version' docs/getting-started/tutorial.md || true
else
echo "Not found: docs/getting-started/tutorial.md"
fi
echo
echo "== Search heroku-deployment.md for rationale =="
rg -n -C2 -i 'node\.?js|yarn|engines|version|why' docs/deployment/heroku-deployment.md || true
echo
echo "== Check introduction and quick-start for Node minimum =="
rg -n -C2 -i 'node\.?js|node\s*[\d]+' docs/introduction.md docs/getting-started/quick-start.md 2>/dev/null || true
echo
echo "== Any global guidance about pinning versions? =="
rg -n -C2 -i 'pin|engine[s]?|"engines"|heroku|version.*why' docs || true
Length of output: 40048
@ihabadham Great question. Based on a sweep of the docs:
- docs/deployment/heroku-deployment.md: we show an engines block but don’t explain why it’s there.
- docs/getting-started/tutorial.md: we recommend “active Node LTS” and Yarn Classic and advise pinning gem/npm versions, but there’s no explicit note that engines control the deploy-time Node/Yarn (e.g., on Heroku) or why you’d pin them.
I suggest we add a brief rationale in both places. Here are small, surgical patches.
Patch 1: Add “Why specify engines?” note right after the engines JSON in docs/deployment/heroku-deployment.md
diff --git a/docs/deployment/heroku-deployment.md b/docs/deployment/heroku-deployment.md
@@
"engines": {
- "node": "16.19.0",
- "yarn": "1.22.4"
+ "node": "16.19.0",
+ "yarn": "1.22.4"
},
+> Why specify engines?
+>
+> Heroku’s Node.js buildpack reads the "engines" field in package.json to select the Node and Yarn
+> versions used at build and runtime. Pinning these avoids surprise upgrades, keeps builds
+> reproducible, and ensures your deploy uses the same toolchain as local development.
+> Choose an actively supported Node LTS and keep this in sync with the versions noted in
+> Getting Started.
Patch 2: Add a short “Note on versions” under Prerequisites in docs/getting-started/tutorial.md
```diff
diff --git a/docs/getting-started/tutorial.md b/docs/getting-started/tutorial.md
@@
- **Nodejs:** We support all [active Node versions](https://github.com/nodejs/release#release-schedule) but recommend using the latest LTS release of Nodejs for the longest support. Older inactive node versions might still work but is not guaranteed. We also recommend using [nvm](https://github.com/nvm-sh/nvm/) to ease using different node versions in different projects.
- **yarn:** We use [yarn classic](https://classic.yarnpkg.com/) as our node package manager.
- You need to have either [Overmind](https://github.com/DarthSim/overmind) or [Foreman](https://rubygems.org/gems/foreman) as a process manager.
+
+Note on versions: On platforms like Heroku, the package.json "engines" field controls the Node and
+Yarn versions used during deploy. Pin these to the same active LTS you use locally to avoid version
+drift between development and CI/deploy and to keep builds reproducible.
If you want, I can also follow up by aligning the example engines values and the “Node.js 18+” mentions across introduction.md and quick-start.md. Do you prefer we bump the example to an active LTS (e.g., Node 20/22) at the same time?
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.md
Cleanup:
docs/guides/advanced/README.md
2. 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.md
4. 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.md
docs/core-concepts/react-on-rails-overview.md
docs/guides/advanced/README.md
docs/advanced-topics/manual-installation-overview.md
Why
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.js
to useintroduction.md
as homepage/docs/planning/
from website buildFuture phases:
(Future phases are repo cleanup tasks - website filtering will hide them from users)
Review Focus
introduction.md
provide a clear, unified starting point?using-react-on-rails.md
more focused as a conceptual guide?manual-installation-overview.md
the 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