-
-
Couldn't load subscription status.
- Fork 638
Improve React on Rails documentation for v16 and coding agents #1789
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
Conversation
## New Documentation - **troubleshooting-build-errors.md**: Comprehensive guide for webpack/build issues - Missing routes file error patterns and solutions - ProvidePlugin module resolution troubleshooting - Environment setup dependencies and workarounds - Agent-friendly diagnostic scripts and auto-fixes - **coding-agents-guide.md**: Structured workflows for AI coding agents - Version compatibility matrix - Installation and upgrade workflows with error detection - Auto-fix strategies for common issues - Emergency procedures and rollback guidance ## Updated Documentation - **upgrading-react-on-rails.md**: - Removed js:export from upgrade steps (it's a setup issue, not upgrade-specific) - Focused troubleshooting on actual upgrade-related issues - Added reference to comprehensive build errors guide - **getting-started.md**: Updated to use react_on_rails v16.0.0 - **home.md**: Added troubleshooting guide to navigation ## Key Improvements - Separated setup/installation issues from upgrade-specific issues - Provided agent-friendly automation and error detection - Enhanced troubleshooting with actionable solutions - Improved documentation discoverability These improvements make react_on_rails documentation more comprehensive and accessible for both human developers and AI coding agents. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. WalkthroughAdds two new documentation pages (coding agents guide; JS build troubleshooting), updates the getting-started install snippet to react_on_rails v16.0.0, inserts a “Common Upgrade Issues” section into the v16 upgrade guide, and updates the docs home links to include the new troubleshooting entry. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Agent as Developer / Coding Agent
participant Docs as Documentation
participant App as Rails App
participant WP as Shakapacker / Webpack
Agent->>Docs: Read install / upgrade / troubleshooting guidance
Agent->>App: Update Gemfile / package.json, run generators, export routes
Agent->>WP: Run build / dev server
alt Build succeeds
WP-->>Agent: Build completes
else Build fails (module resolution / missing routes)
WP-->>Agent: Error output
Agent->>Docs: Follow troubleshooting steps
par Quick fixes
Agent->>WP: Clear webpack cache
Agent->>App: Ensure `app/javascript/utils/routes.js` exists
Agent->>WP: Verify ProvidePlugin modules & aliases
end
Agent->>WP: Re-run build
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review for PR #1789: Improve React on Rails documentationOverviewThis PR significantly enhances the React on Rails documentation with a focus on troubleshooting build errors and providing clear guidance for AI coding agents. The changes are well-structured and add valuable content for both human developers and automated tools. ✅ Strengths
🔍 Suggestions for Improvement1. PR Description CleanupThe PR description still contains template text that should be removed:
2. Documentation ConsistencyIn getting-started.md, consider adding a note about when to run 'rails js:export' since this is a critical step mentioned throughout the new troubleshooting docs. 3. Code Safety in ScriptsThe diagnostic scripts could benefit from more defensive programming. For example, always checking if commands exist before piping to grep, and providing fallback messages when items are not found. 4. Error Handling in Auto-fix ScriptsThe auto-fix scripts should check for success and provide fallback options when the primary approach fails. This is especially important for the database-dependent 'rails js:export' command. 🚨 Potential Issues
📚 Best Practices Alignment✅ Good adherence to project conventions:
🔒 Security Considerations✅ No security concerns identified. The scripts appropriately:
💡 Additional Recommendations
ConclusionThis is a valuable addition to the React on Rails documentation that will significantly improve the developer experience, especially for those using AI coding assistants. The documentation is thorough, practical, and well-organized. With minor improvements to error handling and cleanup of the PR description, this will be an excellent contribution to the project. Recommendation: Approve with minor suggestions ✅ The documentation improvements are substantial and valuable. The suggested improvements are minor and could be addressed in this PR or as follow-up improvements. |
## Problem Documentation gave the impression that js-routes was a core react_on_rails requirement, when it's actually an optional integration used only in specific scenarios. ## Changes Made ### troubleshooting-build-errors.md - Added clear note that missing routes error only occurs with js-routes gem - Explained when js-routes IS and ISN'T needed - Provided alternative solution: remove ProvidePlugin if not using js-routes - Distinguished modern SPA patterns from Rails-heavy integration patterns ### coding-agents-guide.md - Added "(if using js-routes gem)" qualifier to all js:export commands - Updated auto-fix functions to clarify js-routes context - Modified diagnostic scripts to indicate js-routes dependency ### Environment dependencies - Clarified that rails js:export is only needed for js-routes gem ## Result - Documentation now accurately reflects js-routes as optional - Developers won't be confused into thinking it's required for react_on_rails - Clear guidance on when to use js-routes vs modern alternatives - Better separation of concerns between react_on_rails core and optional integrations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
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
🧹 Nitpick comments (8)
docs/getting-started.md (1)
72-73: Fix link text/path mismatch for Configuration doc.Anchor text says docs/basics/configuration.md but href points to guides. Make both “guides.”
Apply this diff:
-- Configure `config/initializers/react_on_rails.rb`. You can adjust some necessary settings and defaults. See file [docs/basics/configuration.md](./guides/configuration.md) for documentation of all configuration options. +- Configure `config/initializers/react_on_rails.rb`. You can adjust necessary settings and defaults. See [docs/guides/configuration.md](./guides/configuration.md) for all configuration options.docs/javascript/troubleshooting-build-errors.md (4)
16-21: Add language to fenced block (markdownlint MD040).Specify a language for this error snippet.
-``` +```text Cannot read properties of undefined (reading 'module') TypeError: Cannot read properties of undefined (reading 'module') at ProvidedDependencyTemplate.apply--- `116-124`: **Add language to fenced block (markdownlint MD040).** This is plain output; mark as text. ```diff - ``` + ```text MONGODB | Error checking localhost:27017: Connection refused--- `127-132`: **Add language to fenced block (markdownlint MD040).** Mark optional gem warning as text. ```diff - ``` + ```text sidekiq-pro is not installed ```
25-31: Clarify prerequisite for rails js:export.Call out that js:export exists only if the project uses a routes‑export task; suggest the common alt command.
1. **Generate JavaScript routes file:** ```bash bundle exec rails js:export ``` + Note: Ensure your app includes a routes export task. If you use the `js-routes` gem, the command is typically: + ```bash + bundle exec rails js:routes + ```docs/guides/upgrading-react-on-rails.md (1)
84-96: Nice addition; add cache clear step.Cache often causes stale resolution errors; add tmp/cache to the checklist.
**Solutions:** -1. Clear webpack cache: `rm -rf node_modules/.cache` +1. Clear caches: `rm -rf node_modules/.cache tmp/cache` 2. Verify all ProvidePlugin modules exist 3. Check webpack alias configurationdocs/contributor-info/coding-agents-guide.md (2)
81-82: Use grep on file directly (avoid ls piping).Simpler and more reliable.
-ls Gemfile | grep -q shakapacker || echo "⚠️ Shakapacker required" +grep -q 'shakapacker' Gemfile || echo "⚠️ Shakapacker required"
254-304: Diagnostics: guard npm absence and surface non‑zero exit.Improve robustness when npm isn’t present or build script missing.
-echo "🔨 Testing build..." -if npm run build >/dev/null 2>&1; then +echo "🔨 Testing build..." +if command -v npm >/dev/null 2>&1 && npm run build >/dev/null 2>&1; then echo "✓ Build successful" else echo "❌ Build failed"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/contributor-info/coding-agents-guide.md(1 hunks)docs/getting-started.md(1 hunks)docs/guides/upgrading-react-on-rails.md(1 hunks)docs/home.md(1 hunks)docs/javascript/troubleshooting-build-errors.md(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/getting-started.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.mddocs/guides/upgrading-react-on-rails.md
📚 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/guides/upgrading-react-on-rails.mddocs/home.md
🪛 GitHub Actions: Lint JS and Ruby
docs/guides/upgrading-react-on-rails.md
[warning] Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
docs/javascript/troubleshooting-build-errors.md
[warning] Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
docs/contributor-info/coding-agents-guide.md
[warning] Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
🪛 markdownlint-cli2 (0.17.2)
docs/javascript/troubleshooting-build-errors.md
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
116-116: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
127-127: 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). (6)
- 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.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
🔇 Additional comments (6)
docs/getting-started.md (1)
29-29: Version bump looks good.Aligns with v16 docs across the PR.
docs/home.md (1)
16-17: LGTM: Navigation updates improve discoverability.docs/contributor-info/coding-agents-guide.md (2)
32-47: Table and quick commands look solid.Good, concise reference for agents.
1-406: Prettier formatting applied — no further changes needed.
Rannpx prettier --write docs/contributor-info/coding-agents-guide.md; file was formatted.docs/guides/upgrading-react-on-rails.md (1)
1-99: Prettier formatting applied — docs/guides/upgrading-react-on-rails.md updated. Rannpx prettier --write docs/guides/upgrading-react-on-rails.md; the file was formatted and should satisfy CI.docs/javascript/troubleshooting-build-errors.md (1)
181-199: Prettier applied — formatting normalized.
Prettier was run on docs/javascript/troubleshooting-build-errors.md and formatting changes were written.
| # 1. Update versions | ||
| sed -i 's/react_on_rails.*~> 14\.0/react_on_rails", "~> 16.0/' Gemfile | ||
| sed -i 's/"react-on-rails": "^14\./"react-on-rails": "^16./' package.json | ||
|
|
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.
Broken sed replacement for Gemfile; produces malformed line.
Current command drops quotes and gem keyword. Provide a safe, anchored edit.
-# 1. Update versions
-sed -i 's/react_on_rails.*~> 14\.0/react_on_rails", "~> 16.0/' Gemfile
-sed -i 's/"react-on-rails": "^14\./"react-on-rails": "^16./' package.json
+# 1. Update versions
+# Gemfile: replace react_on_rails constraint to "~> 16.0"
+sed -i -E 's/^(\s*gem\s+"react_on_rails",\s*")[^"]+(")/\1~> 16.0\2/' Gemfile
+# package.json: bump to ^16.0.0
+sed -i -E 's/("react-on-rails":\s*")[^"]+(")/\1^16.0.0\2/' package.json📝 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.
| # 1. Update versions | |
| sed -i 's/react_on_rails.*~> 14\.0/react_on_rails", "~> 16.0/' Gemfile | |
| sed -i 's/"react-on-rails": "^14\./"react-on-rails": "^16./' package.json | |
| # 1. Update versions | |
| # Gemfile: replace react_on_rails constraint to "~> 16.0" | |
| sed -i -E 's/^([[:space:]]*gem[[:space:]]+"react_on_rails",[[:space:]]*")[^"]+(")/\1~> 16.0\2/' Gemfile | |
| # package.json: bump to ^16.0.0 | |
| sed -i -E 's/("react-on-rails":[[:space:]]*")[^"]+(")/\1^16.0.0\2/' package.json |
🧰 Tools
🪛 GitHub Actions: Lint JS and Ruby
[warning] Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
Code Review for PR #1789: Improve React on Rails documentation for v16 and coding agentsI've completed a thorough review of this PR. Here's my analysis: ✅ Strengths
🎯 Areas for Improvement1. Clarification on js-routes DependencyThe documentation should more prominently clarify that
2. PR Description CleanupThe PR description still contains the template boilerplate ("Remove this paragraph..."). Consider cleaning this up for clarity. 3. Documentation Consistency
4. Error Recovery GuidanceWhile the emergency procedures are good, consider adding:
📋 Specific Suggestions
🔒 Security Considerations
⚡ Performance Considerations
🧪 Test CoverageWhile this is a documentation-only PR, the diagnostic scripts and auto-fix commands have been well-thought-out with proper error handling and fallback strategies. 📝 Final VerdictThis is a high-quality documentation improvement that will significantly help both human developers and AI coding agents work with React on Rails. The structured workflows, clear error patterns, and actionable solutions are particularly valuable. Recommendation: ✅ Approve with minor suggestions The documentation additions are valuable and well-structured. The minor improvements suggested above would make this excellent documentation even better, but they're not blocking issues. Great work on making React on Rails more accessible and easier to troubleshoot! 🎉 |
Code Review for PR #1789: Improve React on Rails documentation for v16 and coding agents🎯 Overall AssessmentThis PR significantly improves the documentation for React on Rails v16, particularly for AI coding agents and troubleshooting common build issues. The changes are well-structured and provide valuable guidance. ✅ Strengths
🔍 Code Quality & Best PracticesPositive Aspects:
Minor Improvements Suggested:
|
New Documentation
troubleshooting-build-errors.md: Comprehensive guide for webpack/build issues
coding-agents-guide.md: Structured workflows for AI coding agents
Updated Documentation
upgrading-react-on-rails.md:
getting-started.md: Updated to use react_on_rails v16.0.0
home.md: Added troubleshooting guide to navigation
Key Improvements
These improvements make react_on_rails documentation more comprehensive and accessible for both human developers and AI coding agents.
🤖 Generated with Claude Code
Summary
Remove this paragraph and provide a general description of the code changes in your pull
request... were there any bugs you had fixed? If so, mention them. If
these bugs have open GitHub issues, be sure to tag them here as well,
to keep the conversation linked together.
Pull Request checklist
Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by
~.Add the CHANGELOG entry at the top of the file.
Other Information
Remove this paragraph and mention any other important and relevant information such as benchmarks.
This change is
Summary by CodeRabbit