Skip to content

Conversation

@ihabadham
Copy link
Contributor

@ihabadham ihabadham commented Sep 29, 2025

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/update test to cover these changes
  • Update documentation
  • Update CHANGELOG file

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 Reviewable

Summary by CodeRabbit

  • Documentation
    • Corrected a repository link in the manual installation guide to point to the appropriate source view.
    • Updated React Router integration docs to reflect current client/server app references and fix broken internal navigation links.
    • Improves clarity and reduces confusion during setup.
    • No functional or behavioral changes; all updates are documentation-only.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

Documentation updates only: adjusted one GitHub source link path and updated two internal filenames in React Router docs to reflect .client/.server naming. No code, behavior, or API changes.

Changes

Cohort / File(s) Summary of changes
Docs: Manual installation
docs/additional-details/manual-installation-overview.md
Updated ReactOnRails client link from tree/master to blob/master.
Docs: React Router integration
docs/javascript/react-router.md
Renamed internal references from ClientRouterApp.jsxRouterApp.client.jsx and ServerRouterApp.jsxRouterApp.server.jsx.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • Reduce bundle size #1697 — Introduces/standardizes split filenames (*.client.*/*.server.*), which these docs now reference.

Suggested reviewers

  • justin808
  • AbanoubGhadban
  • Judahmeek

Poem

I hop through docs with nimble cheer,
Toggling links from there to here.
Client, server—names aligned,
Breadcrumbs neat, well-defined.
With tiny taps of caret’s art,
I nibble typos, then depart. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “fix dead links” succinctly and accurately conveys the primary change in the pull request, which is updating broken documentation links; it is concise, imperative, and directly related to the main modifications described in the summary.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/dead-links-caused-by-tut649

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4a9a64 and b9d73f9.

📒 Files selected for processing (2)
  • docs/additional-details/manual-installation-overview.md (1 hunks)
  • docs/javascript/react-router.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/javascript/react-router.md
  • docs/additional-details/manual-installation-overview.md
🧠 Learnings (4)
📚 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/javascript/react-router.md
  • docs/additional-details/manual-installation-overview.md
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.

Applied to files:

  • docs/javascript/react-router.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/additional-details/manual-installation-overview.md
📚 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/additional-details/manual-installation-overview.md
⏰ 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.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
  • GitHub Check: claude-review
  • GitHub Check: markdown-link-check
  • GitHub Check: build-and-test
🔇 Additional comments (2)
docs/additional-details/manual-installation-overview.md (1)

16-16: Link fix aligns with the current package layout. Pointing the docs at packages/react-on-rails/src/ReactOnRails.client.ts matches where the client entry lives now, so readers land on the actual source instead of a dead branch view. Nicely done.

docs/javascript/react-router.md (1)

31-33: Updated filenames reflect the split client/server bundles. Swapping in RouterApp.client.jsx and RouterApp.server.jsx keeps the tutorial links in sync with the renamed files, so readers won’t hit missing pages anymore.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Sep 29, 2025

Code Review for PR #1840: Fix Dead Links

Thank you for fixing these broken documentation links! I've reviewed the changes and have the following feedback:

Positive Aspects:

  • Correctly fixes the GitHub URL pattern from /tree/master/ to /blob/master/ for the specific file link
  • Updates the react-webpack-rails-tutorial links to reflect the new file structure
  • Changes are minimal and focused on the issue at hand

📋 Review Findings:

1. Link Corrections Look Appropriate

  • Line 16 in manual-installation-overview.md: The change from tree to blob is correct for linking to a specific file
  • Lines 31 and 33 in react-router.md: The updated paths reflect the reorganized structure in the tutorial repository

⚠️ Recommendations:

  1. PR Description: Please update the PR description to provide context about which links were broken and why (e.g., "Fixed broken links due to file restructuring in react-webpack-rails-tutorial repository")

  2. Verification: While the changes appear correct, it would be good to verify all three new URLs are accessible:

  3. Checklist: Please complete the PR checklist items or mark them as N/A if they don't apply

🔒 Security: No security concerns identified

🚀 Performance: No performance impact (documentation-only changes)

🧪 Testing: No test changes required for documentation updates

Overall Assessment:

This is a straightforward and beneficial fix for broken documentation links. Once the PR description is updated with more context, this should be ready to merge.

Thank you for improving the documentation quality!

@ihabadham ihabadham closed this Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant