Move OSS docs to docs/oss/ for clear OSS/Pro separation#2584
Move OSS docs to docs/oss/ for clear OSS/Pro separation#2584
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDocumentation paths and in-repo doc references were updated to place OSS content under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because Privacy Mode (Legacy) is turned on. To enable Bugbot Autofix, switch your privacy mode in the Cursor dashboard.
| - 2017-04-03: 6.9.3 Released! Props rendered in JSON script tag. Page size is smaller now due to less escaping! | ||
| - 2017-03-06: Updated to Webpack v2! | ||
| - 2017-03-02: Demo of internationalization (i18n) is live at [reactrails.com](https://reactrails.com/). Docs [here](docs/building-features/i18n.md). | ||
| - 2017-03-02: Demo of internationalization (i18n) is live at [reactrails.com](https://reactrails.com/). Docs [here](docs/oss/building-features/i18n.md). |
There was a problem hiding this comment.
Missed link update in NEWS.md line 17
Medium Severity
Line 17 still references ./docs/upgrading/upgrading-react-on-rails.md (old path) but this file was moved to ./docs/oss/upgrading/upgrading-react-on-rails.md. The PR updated similar links on lines 24 and 58 but missed this one.
Greptile SummaryThis PR restructures the documentation by moving all OSS docs from Key changes:
Issue found: Six instances of broken relative links in five moved OSS files. After being relocated to Confidence Score: 3/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[docs/] --> B[README.md\nUnified entry point]
A --> C[oss/]
A --> D[pro/]
C --> C1[getting-started/]
C --> C2[core-concepts/]
C --> C3[building-features/]
C --> C4[configuration/]
C --> C5[api-reference/]
C --> C6[deployment/]
C --> C7[upgrading/]
C --> C8[migrating/]
C --> C9[misc/]
D --> D1[installation.md]
D --> D2[configuration.md]
D --> D3[react-server-components/]
D --> D4[node-renderer/]
D --> D5[caching.md / streaming / etc.]
E[internal/] --> E1[planning/]
E --> E2[testimonials/]
C4 -- "broken: ../pro/\nshould be: ../../pro/" --> D2
C5 -- "broken: ../pro/\nshould be: ../../pro/" --> D3
C2 -- "broken: ../pro/\nshould be: ../../pro/" --> D3
C1 -- "broken: ../pro/\nshould be: ../../pro/" --> D3
style C fill:#d4edda,stroke:#28a745
style D fill:#cce5ff,stroke:#004085
style E fill:#fff3cd,stroke:#856404
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Update OSS-to-Pro links after moving docs under docs/oss
This relative link now resolves to docs/oss/pro/..., which does not exist after the docs move, so readers hit a dead link when navigating from OSS docs to Pro docs on GitHub/local markdown renderers. Because the file moved from docs/configuration/... to docs/oss/configuration/... without adjusting the target, the path needs an extra .. segment (for example, ../../pro/... from this directory).
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react_on_rails/lib/react_on_rails/doctor.rb (1)
1191-1211:⚠️ Potential issue | 🟡 MinorWrap the new documentation URLs to stay within the Ruby line-length rule.
Lines 1196 and 1211 are now well past 120 chars. Please split the string or extract the URL into a constant/helper so this stays compliant with the repo's Ruby style rules. As per coding guidelines, "Ruby line length maximum 120 characters. Break long method chains across multiple lines with consistent indentation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails/lib/react_on_rails/doctor.rb` around lines 1191 - 1211, The long documentation URL in the checker.add_info messages (the string "📖 See: https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md") exceeds the 120-char Ruby line-length rule; locate the checker.add_info calls inside react_on_rails/doctor.rb (the blocks using checker.add_info for the testing-configuration.md link) and either split the literal into two concatenated strings so the source lines stay ≤120 chars or extract the URL into a named constant (e.g., TESTING_CONFIG_URL) used in the checker.add_info calls to keep lines short and readable. Ensure both occurrences (the one near the build_test_command/compile checks and the later warning block) are updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 183: The OSS docs allowlist omitted the `configuration/` subdirectory,
which conflicts with the repo layout; update the AGENTS.md line that lists OSS
subdirectories (the string listing `getting-started/`, `core-concepts/`,
`building-features/`, `api-reference/`, `deployment/`, `migrating/`,
`upgrading/`, `misc/`) to include `configuration/` (e.g., add `configuration/`
into that comma-separated list) so the documented canonical agent guidance
matches the actual `docs/oss/configuration/` directory used in the PR.
In `@docs/pro/caching.md`:
- Line 117: The example uses the non-existent method name toLocalString(locale);
update the JavaScript API to the correct locale-aware formatter by replacing any
occurrences of toLocalString(locale) with toLocaleString(locale) (and verify any
example code or prose mentions reflect toLocaleString) so readers won't
encounter a runtime error.
In `@docs/pro/home-pro.md`:
- Line 42: The "Rails Configuration" link in docs/pro/home-pro.md currently
points to ../oss/configuration/README.md while the references section links the
same text to ./configuration.md; update one of these so they consistently target
the same document or rename one link text to make the OSS vs Pro distinction
explicit. Locate the "Rails Configuration" occurrences in the file (the inline
link at line ~42 and the references section entry for configuration.md) and
either change the href to use the same path for both links or adjust one label
(e.g., "Rails Configuration (OSS)" or "Rails Configuration (Pro)") to reflect
the different targets.
In `@docs/README.md`:
- Around line 26-35: The OSS docs index under the "Categories" section in
README.md is missing the new misc section; update the list in that section by
adding an entry for the misc folder (e.g., add a bullet like "Misc" that links
to ./oss/misc/) so docs/oss/misc/ becomes discoverable from the main docs index.
In `@README.md`:
- Line 151: The README table row for "🌍 **Internationalization**" still links
to the legacy docs route; update the link target to the new OSS docs path under
docs/oss so the row matches surrounding entries (replace the URL that currently
points to https://www.shakacode.com/react-on-rails/docs/building-features/i18n
with the corresponding docs/oss/.../i18n OSS URL), keeping the link text intact.
---
Outside diff comments:
In `@react_on_rails/lib/react_on_rails/doctor.rb`:
- Around line 1191-1211: The long documentation URL in the checker.add_info
messages (the string "📖 See:
https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md")
exceeds the 120-char Ruby line-length rule; locate the checker.add_info calls
inside react_on_rails/doctor.rb (the blocks using checker.add_info for the
testing-configuration.md link) and either split the literal into two
concatenated strings so the source lines stay ≤120 chars or extract the URL into
a named constant (e.g., TESTING_CONFIG_URL) used in the checker.add_info calls
to keep lines short and readable. Ensure both occurrences (the one near the
build_test_command/compile checks and the later warning block) are updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9035b33-4696-45f3-b1a9-89d12bc26b14
📒 Files selected for processing (88)
AGENTS.mdCHANGELOG.mdNEWS.mdREADME.mddocs/README.mddocs/oss/api-reference/generator-details.mddocs/oss/api-reference/javascript-api.mddocs/oss/api-reference/rails_view_rendering_from_inline_javascript.mddocs/oss/api-reference/redux-store-api.mddocs/oss/api-reference/view-helpers-api.mddocs/oss/building-features/extensible-precompile-pattern.mddocs/oss/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.mddocs/oss/building-features/how-to-conditionally-server-render-based-on-device-type.mddocs/oss/building-features/how-to-use-different-files-for-client-and-server-rendering.mddocs/oss/building-features/i18n.mddocs/oss/building-features/images.mddocs/oss/building-features/process-managers.mddocs/oss/building-features/rails-engine-integration.mddocs/oss/building-features/rails-webpacker-react-integration-options.mddocs/oss/building-features/react-and-redux.mddocs/oss/building-features/react-helmet.mddocs/oss/building-features/react-router.mddocs/oss/building-features/streaming-server-rendering.mddocs/oss/building-features/tanstack-router.mddocs/oss/building-features/testing-configuration.mddocs/oss/building-features/turbolinks.mddocs/oss/configuration/README.mddocs/oss/configuration/configuration-deprecated.mddocs/oss/configuration/configuration-pro.mddocs/oss/core-concepts/auto-bundling-file-system-based-automated-bundle-generation.mddocs/oss/core-concepts/client-vs-server-rendering.mddocs/oss/core-concepts/how-react-on-rails-works.mddocs/oss/core-concepts/react-server-rendering.mddocs/oss/core-concepts/render-functions-and-railscontext.mddocs/oss/core-concepts/render-functions.mddocs/oss/core-concepts/webpack-configuration.mddocs/oss/deployment/README.mddocs/oss/deployment/capistrano-deployment.mddocs/oss/deployment/elastic-beanstalk.mddocs/oss/deployment/heroku-deployment.mddocs/oss/deployment/server-rendering-tips.mddocs/oss/deployment/troubleshooting-build-errors.mddocs/oss/deployment/troubleshooting-when-using-shakapacker.mddocs/oss/deployment/troubleshooting-when-using-webpacker.mddocs/oss/deployment/troubleshooting.mddocs/oss/getting-started/common-issues.mddocs/oss/getting-started/create-react-on-rails-app.mddocs/oss/getting-started/installation-into-an-existing-rails-app.mddocs/oss/getting-started/pro-quick-start.mddocs/oss/getting-started/project-structure.mddocs/oss/getting-started/quick-start.mddocs/oss/getting-started/tutorial.mddocs/oss/getting-started/using-react-on-rails.mddocs/oss/introduction.mddocs/oss/migrating/angular-js-integration-migration.mddocs/oss/migrating/babel-to-swc-migration.mddocs/oss/migrating/convert-rails-5-api-only-app.mddocs/oss/migrating/migrating-from-react-rails.mddocs/oss/migrating/migrating-from-webpack-to-rspack.mddocs/oss/misc/articles.mddocs/oss/misc/asset-pipeline.mddocs/oss/misc/code_of_conduct.mddocs/oss/misc/credits.mddocs/oss/misc/doctrine.mddocs/oss/misc/style.mddocs/oss/misc/tips.mddocs/oss/misc/updating-dependencies.mddocs/oss/upgrading/release-notes/15.0.0.mddocs/oss/upgrading/release-notes/16.0.0.mddocs/oss/upgrading/release-notes/16.1.0.mddocs/oss/upgrading/release-notes/16.2.0.mddocs/oss/upgrading/upgrading-react-on-rails.mddocs/pro/caching.mddocs/pro/home-pro.mddocs/pro/react-server-components/glossary.mdinternal/planning/JS_PRO_PACKAGE_SEPARATION_PLAN.mdinternal/planning/MONOREPO_MERGER_PLAN.mdinternal/planning/MONOREPO_MERGER_PLAN_REF.mdinternal/testimonials/README.mdinternal/testimonials/hvmn.mdinternal/testimonials/resortpass.mdreact_on_rails/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttreact_on_rails/lib/react_on_rails/doctor.rbreact_on_rails/lib/react_on_rails/test_helper.rbreact_on_rails/spec/dummy/config/initializers/react_on_rails.rbreact_on_rails_pro/spec/dummy/app/views/layouts/application.html.erbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails.rbreact_on_rails_pro/spec/execjs-compatible-dummy/config/initializers/react_on_rails.rb
Review: Move OSS docs to docs/oss/Overall the restructuring is clean and well-executed. Cross-references in Ruby source files, generator templates, Issues Found1. Stale absolute GitHub URL (broken by this PR)
# See https://github.com/shakacode/react_on_rails/blob/master/docs/configuration/README.md for the restSince # See https://github.com/shakacode/react_on_rails/blob/master/docs/oss/configuration/README.md for the rest2.
The PR description mentions a companion task for Minor Observations (pre-existing, not introduced by this PR)
Positives
The stale link in item 1 is the only issue directly introduced by this PR and should be fixed before merge. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @@ -2,8 +2,119 @@ | |||
|
|
|||
| ## View Helpers | |||
|
|
|||
There was a problem hiding this comment.
This file received a large content expansion (110+ new lines) that goes well beyond the doc restructuring scope. The view helper signatures, option descriptions, and code examples all look accurate, but a review by someone familiar with the Pro internals is worthwhile before merging — particularly the async_react_component and rsc_payload_react_component sections which describe complex behavior.
PR Review: Move OSS docs to docs/oss/Overall this is a clean, well-executed documentation reorganization. The file moves are correct and the cross-reference updates are comprehensive. One broken link to fix: Bug: Missed link update in NEWS.md line 17 This line was not part of the diff hunks so it could not receive an inline comment. It contains a broken relative link: The file was moved to docs/oss/upgrading/ by this PR but line 17 was not updated. Should be ./docs/oss/upgrading/upgrading-react-on-rails.md Observation: ruby-api.md and troubleshooting.md include substantial content additions These two Pro docs received significant new content (view helper docs with code examples, node renderer troubleshooting, fragment caching troubleshooting, license troubleshooting) well beyond the doc restructuring scope. The content looks accurate and valuable, but worth a focused review by someone familiar with the Pro internals to verify correctness (see inline comment on ruby-api.md). External website URLs intentionally deferred Several places still reference https://www.shakacode.com/react-on-rails/docs/building-features/... (CHANGELOG, README feature table, server_manager.rb). The PR description explicitly calls out website URL updates as a companion sc-website task -- that is the right approach. Link path fix in docs/pro/react-server-components/glossary.md is correct The old path ../../../docs/core-concepts/... went 3 levels up to the repo root then back into docs/, which is fragile. The new ../../oss/core-concepts/... (2 levels up to docs/, then into oss/) is cleaner and correct. |
There was a problem hiding this comment.
💡 Codex Review
Correct OSS-to-Pro relative links after moving docs under oss
After the OSS docs were moved into docs/oss, this link still uses ../pro/..., which now resolves to docs/oss/pro/... and 404s. From this directory depth it needs to go up two levels (../../pro/...) to reach the existing Pro docs, and the same pattern appears in other moved OSS pages (for example docs/oss/api-reference/generator-details.md and docs/oss/getting-started/project-structure.md).
Update moved OSS docs to use valid shared image paths
This screenshot path was valid before the move, but from docs/oss/core-concepts the relative path ../images/... now points to docs/oss/images/..., which does not exist, so the rendered documentation shows broken images. These image references need to be rebased to the shared docs/images/ location (for example ../../images/...).
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/pro/node-renderer/error-reporting-and-tracing.md`:
- Line 3: The doc line implies honeybadgerApiKey and sentryDsn are merely
deprecated, but configBuilder.ts actually causes a runtime error and exits when
those options are used in 4.0.0+, so update the sentence to state that using
honeybadgerApiKey or sentryDsn will cause the process to exit with an error in
4.0.0 and later; mention the exact option names (honeybadgerApiKey, sentryDsn)
and reference the enforcement in configBuilder.ts so readers know this is a
breaking/runtime error, not just a deprecation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77f003fa-769b-427d-99d3-5c89ce0a48b0
📒 Files selected for processing (8)
docs/pro/configuration.mddocs/pro/home-pro.mddocs/pro/node-renderer/error-reporting-and-tracing.mddocs/pro/node-renderer/heroku.mddocs/pro/node-renderer/js-configuration.mddocs/pro/node-renderer/troubleshooting.mddocs/pro/ruby-api.mddocs/pro/troubleshooting.md
✅ Files skipped from review due to trivial changes (5)
- docs/pro/node-renderer/troubleshooting.md
- docs/pro/troubleshooting.md
- docs/pro/node-renderer/heroku.md
- docs/pro/configuration.md
- docs/pro/ruby-api.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/pro/home-pro.md
|
The structural reorganization is clean and well-executed. Ruby source files, generator templates, doctor.rb, and Pro docs cross-references are all correctly updated. The docs/README.md unified entry point is a good improvement for navigation. Critical: Broken relative links to docs/pro/ in moved OSS files Six links across five files use ../pro/ which now incorrectly resolves to the non-existent docs/oss/pro/ rather than docs/pro/. After the move from docs/{subdir}/ to docs/oss/{subdir}/, the correct relative path is ../../pro/. Affected locations:
Minor: Stale link in NEWS.md line 17 ./docs/upgrading/upgrading-react-on-rails.md should be ./docs/oss/upgrading/upgrading-react-on-rails.md Minor: AGENTS.md project structure table The docs/contributor-info/ row was removed but internal/contributor-info/ is not added as its replacement, leaving contributors without guidance on where those docs now live. Generated with Claude Code |
Code ReviewOverall this is a well-executed documentation reorganization. The Issues 1. The old table had 2. The new docs say it "Automatically sets 3. Lines like Nits
|
| | `rakelib/` | Rake task definitions | | ||
| | `docs/oss/` | OSS documentation — published to the [ShakaCode website](https://www.shakacode.com/react-on-rails/docs/) | | ||
| | `docs/pro/` | Pro documentation — installation, configuration, RSC, node renderer, caching | | ||
| | `analysis/` | Investigation and analysis documents (kebab-case `.md` files) | |
There was a problem hiding this comment.
The internal/ directory (created by #2414 and extended here) is now home to contributor-info/, planning/, and testimonials/ but doesn't appear in this table. Without an entry here, agents working on the repo have no way to discover those documents.
Suggested addition below analysis/:
| | `analysis/` | Investigation and analysis documents (kebab-case `.md` files) | | |
| | `analysis/` | Investigation and analysis documents (kebab-case `.md` files) | | |
| | `internal/` | Contributor guides, planning docs, and testimonials (not published to the website) | |
|
|
||
| ### `cached_react_component_hash(component_name, options = {}, &block)` | ||
|
|
||
| Fragment-cached version of `react_component_hash`. Automatically sets `prerender: true`. Returns a hash with `"html"` and `"consoleReplayScript"` keys. |
There was a problem hiding this comment.
"Automatically sets prerender: true" is a strong behavioural claim. Please verify this against the actual helper in react_on_rails_pro_helper.rb before this ships as canonical docs. If it is true, it is worth adding a one-line rationale (e.g. "because the hash return value requires server-rendered HTML to split from the console replay script") so readers understand why the option is forced on.
|
|
||
| ## Source | ||
|
|
||
| - [View Helpers](https://github.com/shakacode/react_on_rails/blob/master/react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb) |
There was a problem hiding this comment.
These links use /blob/master/ but the repo's default branch is main. They currently resolve via GitHub's redirect, but that is fragile. Please update to /blob/main/ (or link to the directory instead of a specific file for more durability).
| - [View Helpers](https://github.com/shakacode/react_on_rails/blob/master/react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb) | |
| - [View Helpers](https://github.com/shakacode/react_on_rails/blob/main/react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb) | |
| - [Utils](https://github.com/shakacode/react_on_rails/blob/main/react_on_rails_pro/lib/react_on_rails_pro/utils.rb) |
| ### Categories | ||
|
|
||
| ### 🏗️ **Migrating from Other Solutions** | ||
| - [Getting Started](./oss/getting-started/) - Installation, quick start, tutorial |
There was a problem hiding this comment.
These category links resolve to bare directory listings on GitHub, which is a poor reading experience. Most of these directories already have a README.md — linking there directly is much better:
| - [Getting Started](./oss/getting-started/) - Installation, quick start, tutorial | |
| - [Getting Started](./oss/getting-started/README.md) - Installation, quick start, tutorial | |
| - [Core Concepts](./oss/core-concepts/) - How React on Rails works, SSR, auto-bundling | |
| - [Building Features](./oss/building-features/) - Redux, routing, i18n, testing | |
| - [Configuration](./oss/configuration/README.md) - All configuration options | |
| - [API Reference](./oss/api-reference/) - View helpers and JavaScript API | |
| - [Deployment](./oss/deployment/README.md) - Production deployment and troubleshooting | |
| - [Upgrading](./oss/upgrading/) - Version upgrade guides | |
| - [Migrating](./oss/migrating/) - From other frameworks |
(Adjust which directories have a README.md as needed — at minimum getting-started/, configuration/, and deployment/ do.)
coderabbitai
Bot
added
documentation
docs-cleanup
PR Review: Move OSS docs to docs/oss/Overall: Well-executed documentation restructuring. The separation of OSS and Pro docs is clean and most cross-references were correctly updated. Found one broken link to flag, plus a few minor observations. Bug: Broken link in
|
| ### Categories | ||
|
|
||
| ### 🏗️ **Migrating from Other Solutions** | ||
| - [Getting Started](./oss/getting-started/README.md) - Installation, quick start, tutorial |
There was a problem hiding this comment.
Broken link — docs/oss/getting-started/README.md does not exist. The getting-started/ directory contains only named files (quick-start.md, tutorial.md, etc.), no README.md.
| - [Getting Started](./oss/getting-started/README.md) - Installation, quick start, tutorial | |
| - [Getting Started](./oss/getting-started/quick-start.md) - Installation, quick start, tutorial |
|
PR Review: Move OSS Docs to docs/oss/ -- Overall assessment: This is a clean, well-executed documentation restructuring. The new docs/oss/ and docs/pro/ separation is clear, all 67+ OSS doc files are properly organized, and cross-references in Ruby source files, generator templates, and spec initializers have been correctly updated. WHAT LOOKS GOOD: All relative links in docs/README.md, docs/oss/introduction.md, and cross docs/pro/ to ../oss/ references check out against actual file paths. Ruby URL comments in doctor.rb and test_helper.rb correctly point to docs/oss/. AGENTS.md accurately documents docs/oss/ and docs/pro/ with the updated boundary rule. The unified docs/README.md entry point with learning paths and quick-reference table is a good GitHub browsing experience. ISSUES FOUND: 1) Generator template comment/code mismatch: In react_on_rails.rb.tt line 43 says 'ALTERNATIVE APPROACH: Uncomment below' but line 48 (config.build_test_command) is already uncommented. This means every newly generated app will have build_test_command active by default, which triggers the doctor.rb warning when compile: true is also set in shakapacker.yml. Should either comment out line 48 or change the comment to say 'configured below'. 2) Relative path vs full URL inconsistency: test_helper.rb:29 uses a bare relative path while doctor.rb:1193 uses a full GitHub URL. Both work but full URLs are more discoverable (clickable in IDEs). 3) Website URL updates (acknowledged out of scope): Several external URLs in README.md still reflect old URL structure, need companion updates in sc-website per the PR description. VERDICT: LGTM with a suggestion to fix the generator template comment/code mismatch before merge. |
| # ALTERNATIVE APPROACH: Uncomment below AND configure ReactOnRails::TestHelper | ||
| # - Provides explicit control over test asset compilation timing | ||
| # - Requires adding ReactOnRails::TestHelper to spec/rails_helper.rb | ||
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/building-features/testing-configuration.md | ||
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md | ||
| # | ||
| config.build_test_command = "RAILS_ENV=test bin/shakapacker" |
There was a problem hiding this comment.
The comment says "Uncomment below AND configure ReactOnRails::TestHelper" but config.build_test_command on line 48 is already uncommented. This means every app generated with this template has build_test_command set by default — which will trigger the doctor.rb conflict warning if the user also has compile: true in shakapacker.yml.
The fix is either:
| # ALTERNATIVE APPROACH: Uncomment below AND configure ReactOnRails::TestHelper | |
| # - Provides explicit control over test asset compilation timing | |
| # - Requires adding ReactOnRails::TestHelper to spec/rails_helper.rb | |
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/building-features/testing-configuration.md | |
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md | |
| # | |
| config.build_test_command = "RAILS_ENV=test bin/shakapacker" | |
| # config.build_test_command = "RAILS_ENV=test bin/shakapacker" |
...so the comment matches ("Uncomment below"), or update the comment to say "configured below" to match the current behavior where it's active by default. This appears to be a pre-existing issue, but worth resolving here.
PR Review: Move OSS docs to docs/oss/Overall this is a clean, well-scoped documentation reorganization. The structure makes the OSS/Pro split immediately obvious and the cross-reference updates are thorough. What looks good:
Issues worth addressing:
Nit: |
| - Use `config.ssr_pre_hook_js` to clear global state leaks between renders | ||
| - Enable rolling restarts as insurance against slow memory leaks | ||
|
|
||
| For additional troubleshooting, see the [main troubleshooting guide](../troubleshooting.md). |
There was a problem hiding this comment.
This file still covers Connection refused, Worker restart config, and Memory issues — the same three topics that the new docs/pro/troubleshooting.md "## Node Renderer" section now covers with expanded detail. To avoid two sources of truth, consider either:
- Converting this file to a simple pointer (remove the duplicated sections, keep only this "For additional troubleshooting..." line), or
- Keeping this file as the canonical node-renderer troubleshooting doc and removing the Node Renderer section from
docs/pro/troubleshooting.md.
| Suppose you detect that some library used in server-rendering is leaking state between calls to server render. In that case, you can set the `config.ssr_pre_hook_js` in your `config/initializers/react_on_rails_pro.rb` to run some JavaScript to clear the globally leaked state at the beginning of each call to server render. | ||
|
|
||
| For more details, see [Rails Configuration](../../docs/configuration/README.md). | ||
| For more details, see [Rails Configuration (OSS)](../oss/configuration/README.md). |
There was a problem hiding this comment.
This cross-link to ../oss/configuration/README.md looks correct for the new structure ✓. Just noting that lines 27 and 36 above still use "docs/streaming-server-rendering" and "docs/caching" as link display text (path-style) rather than human-readable titles like "Streaming Server Rendering" and "Fragment Caching". Minor readability nit since those lines weren't changed in this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 005869d9c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md | ||
| # | ||
| config.build_test_command = "RAILS_ENV=test bin/shakapacker" | ||
| # config.build_test_command = "RAILS_ENV=test bin/shakapacker" |
There was a problem hiding this comment.
Commenting out config.build_test_command leaves new installs with an empty test build command, but the generator still wires ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config) into RSpec (see base_generator.rb), and WebpackAssetsCompiler exits immediately when build_test_command is blank. In apps where the generator detects spec/rails_helper.rb, JS/system specs now abort on first run unless users manually notice and uncomment this line, which is a functional regression from the previous template default.
Useful? React with 👍 / 👎.
ruby-api.md: Document all 7 view helpers (cached_react_component, cached_react_component_hash, stream_react_component, cached_stream_react_component, rsc_payload_react_component, async_react_component, cached_async_react_component) with signatures, descriptions, and examples. Add Utils method reference table. troubleshooting.md: Add sections for node renderer issues (connection refused, memory leaks, worker restarts during streaming), fragment caching problems, license validation, and RSC debugging. Part of #2583 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace the 2-bullet stub with sections on worker restart config, connection refused debugging, and memory profiling. Cross-reference the main troubleshooting guide. Part of #2583 Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Fix NEWS.md missed link update to new docs/oss/ path - Add configuration/ to AGENTS.md OSS docs allowlist - Add internal/ directory to AGENTS.md project structure table - Fix toLocalString → toLocaleString typo in caching.md - Clarify deprecated → removed for node-renderer options in 4.0.0+ - Disambiguate Rails Configuration links in home-pro.md (OSS vs Pro) - Add misc/ section to docs/README.md categories - Link categories to README.md instead of bare directories - Update README.md i18n link to new OSS docs path - Fix all ../pro/ relative links from docs/oss/ to ../../pro/ - Fix broken ../images/ paths from docs/oss/core-concepts/ - Extract long URL in doctor.rb to stay within 120-char line limit - Add rationale for forced prerender: true in ruby-api.md Co-Authored-By: Claude Opus 4.6 <[email protected]>
The getting-started/ directory has no README.md; link to quick-start.md instead. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Comment out build_test_command in generator template so it matches the "Uncomment below" instruction (was active by default, conflicting with compile: true in shakapacker.yml) - Use full GitHub URL in test_helper.rb comment for consistency with doctor.rb and better IDE discoverability Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Use full GitHub URL in test_helper.rb comment for consistency with doctor.rb - Deduplicate node-renderer/troubleshooting.md by converting to pointer to main troubleshooting guide - Use human-readable link text in home-pro.md instead of path-style text Co-Authored-By: Claude Opus 4.6 <[email protected]>
justin808
force-pushed
the
jg/docs-consolidate-oss-pro
branch
from
8c9763c to
e99f76b
Compare
|
Review: Move OSS docs to docs/oss/ - This is a clean, well-executed documentation reorganisation. Links spot-checked and all resolve correctly. Two items worth addressing before merge: (1) In react_on_rails.rb.tt the comment says 'ALTERNATIVE APPROACH: Uncomment below' but config.build_test_command is already uncommented and active - confusing for new users. (2) The test plan still has an unchecked item for sc-website URL coordination - if merged before website redirects are set up, externally bookmarked links will 404. Also worth a follow-up: adding an automated link checker (e.g. markdown-link-check) to CI given ~90 files were touched. Overall low risk - no logic changes. |
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/building-features/testing-configuration.md | ||
| # - See: https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md | ||
| # | ||
| config.build_test_command = "RAILS_ENV=test bin/shakapacker" |
There was a problem hiding this comment.
The comment on lines 43-47 says "ALTERNATIVE APPROACH: Uncomment below" but config.build_test_command here is already active (not commented out). Developers who generate a new app will silently have build_test_command set, contradicting both the "use ONLY ONE" warning above and the "Uncomment below" instruction.
This is a pre-existing issue (not introduced by this PR's URL update), but since the file is open it's worth fixing:
| config.build_test_command = "RAILS_ENV=test bin/shakapacker" | |
| # config.build_test_command = "RAILS_ENV=test bin/shakapacker" |
|
Review: Move OSS docs to docs/oss/ Overall this is a clean, well-structured documentation reorganization with low functional risk. The separation of OSS and Pro docs into docs/oss/ and docs/pro/ is a sensible architectural decision. What is good:
Issues found: Issue 1. Stale GitHub URL inside moved file (broken link) docs/oss/building-features/how-to-conditionally-server-render-based-on-device-type.md was renamed with 100% similarity (content unchanged), but line 30 contains a hardcoded GitHub URL pointing to the old path. This link will 404 after merge. It should reference docs/oss/configuration/README.md. See inline comment. Issue 2. WARP.md:360 not updated (out of PR scope but worth noting) WARP.md still references docs/MONOREPO_MERGER_PLAN_REF.md which moved to internal/planning/MONOREPO_MERGER_PLAN_REF.md. This file is not in the PR diff - worth addressing in a follow-up. Issue 3. Relative paths in dummy app initializer comments The comments in react_on_rails/spec/dummy/config/initializers/react_on_rails.rb and the Pro equivalents now read See docs/oss/configuration/README.md (a repo-relative path). These were already relative before this PR so it is not a regression, but since test_helper.rb was upgraded to a full absolute URL in this same PR, applying the same upgrade here would be consistent. Minor. Issue 4. Website URL redirect dependency Numerous entries in README.md, CHANGELOG.md, and packages/ READMEs still link to the website with the old path structure (e.g., /docs/building-features/..., /docs/configuration/). The PR test plan mentions coordinating website URL updates - make sure those redirects are confirmed before merge to avoid breaking existing bookmarks. |
|
Inline note on docs/oss/building-features/how-to-conditionally-server-render-based-on-device-type.md line 30: This file was renamed with 100% similarity so GitHub does not show it in the diff, but the content still contains the old hardcoded URL on line 30: See https://github.com/shakacode/react_on_rails/blob/master/docs/configuration/README.md for the rest After this rename, that path no longer exists and will 404. It should be updated to: See https://github.com/shakacode/react_on_rails/blob/master/docs/oss/configuration/README.md for the rest |
| # Match test section and look for compile: true | ||
| has_compile_true = shakapacker_content.match(/^test:.*?^\s+compile:\s*true/m) | ||
|
|
||
| testing_config_url = |
There was a problem hiding this comment.
Good refactor extracting the URL to a variable to avoid repeating it twice (lines 1200 and 1215). One small suggestion: since this is a constant string that never changes within the method, you could define it as a frozen string constant at the class level (e.g., TESTING_CONFIG_URL = "...".freeze) to make it reusable if the URL is referenced elsewhere in the file in the future. That said, local variable is perfectly fine here given the current scope.
| # | ||
| # See docs/building-features/testing-configuration.md for more info | ||
| # See https://github.com/shakacode/react_on_rails/blob/master/docs/oss/building-features/testing-configuration.md | ||
| # for more info |
There was a problem hiding this comment.
Good improvement converting from a vague relative path to the full absolute GitHub URL - makes it directly navigable from any context (IDE, email, etc.).
…k cleanup (#2596) ## Summary - Added missing changelog entries for PRs #2439, #2477, #2554 - Stamped version header for 16.4.0.rc.9 (collapsed rc.8 into rc.9) - **Fixed rake task bug**: `collapse_prerelease_sections` removed section headers but left behind orphaned compare links at the bottom of CHANGELOG.md. Added `cleanup_collapsed_prerelease_links` to remove these links and update `[unreleased]` to compare from the last stable version. ## Changes 1. **CHANGELOG.md**: New entries + rc.9 stamp + removed orphaned `[16.4.0.rc.8]` link 2. **update_changelog.rake**: Added `cleanup_collapsed_prerelease_links` function, called from `prepare_changelog_for_auto_version` 3. **update_changelog_rake_helpers_spec.rb**: 3 new tests covering link cleanup (multi-prerelease chain, single prerelease, no prereleases) ## Skipped PRs (not user-visible) - #2593 — test only (lock instrumentation) - #2591 — test only (Jest clearMocks) - #2588 — internal refactoring (Thor shell output) - #2584 — docs restructuring - #2587 — internal tooling (release script) - #2589 — docs fix (JWT claim name) - #2586 — docs/CI (dead link removal) ## Test plan - [x] All 13 rake helper tests pass (3 new) - [ ] CI passes - [ ] After merge, run `rake release` to publish 16.4.0.rc.9 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed bin/setup failing in pnpm workspace member directories. * **New Features** * Added host configuration option for the Node Renderer Fastify worker. * **Improvements / Changed** * Automatic installation of react_on_rails_pro enabled for --rsc/--pro flags. * Clarified Pro-installation signaling related to immediate hydration warnings. * Updated changelog to include RC9 entries and ensure Unreleased links point to the correct base. * **Documentation** * Added startup warning and remediation guidance for unsafe compression middleware callbacks. * **Tests** * Added tests covering changelog prerelease-link cleanup behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
…k cleanup (#2596) ## Summary - Added missing changelog entries for PRs #2439, #2477, #2554 - Stamped version header for 16.4.0.rc.9 (collapsed rc.8 into rc.9) - **Fixed rake task bug**: `collapse_prerelease_sections` removed section headers but left behind orphaned compare links at the bottom of CHANGELOG.md. Added `cleanup_collapsed_prerelease_links` to remove these links and update `[unreleased]` to compare from the last stable version. ## Changes 1. **CHANGELOG.md**: New entries + rc.9 stamp + removed orphaned `[16.4.0.rc.8]` link 2. **update_changelog.rake**: Added `cleanup_collapsed_prerelease_links` function, called from `prepare_changelog_for_auto_version` 3. **update_changelog_rake_helpers_spec.rb**: 3 new tests covering link cleanup (multi-prerelease chain, single prerelease, no prereleases) ## Skipped PRs (not user-visible) - #2593 — test only (lock instrumentation) - #2591 — test only (Jest clearMocks) - #2588 — internal refactoring (Thor shell output) - #2584 — docs restructuring - #2587 — internal tooling (release script) - #2589 — docs fix (JWT claim name) - #2586 — docs/CI (dead link removal) ## Test plan - [x] All 13 rake helper tests pass (3 new) - [ ] CI passes - [ ] After merge, run `rake release` to publish 16.4.0.rc.9 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed bin/setup failing in pnpm workspace member directories. * **New Features** * Added host configuration option for the Node Renderer Fastify worker. * **Improvements / Changed** * Automatic installation of react_on_rails_pro enabled for --rsc/--pro flags. * Clarified Pro-installation signaling related to immediate hydration warnings. * Updated changelog to include RC9 entries and ensure Unreleased links point to the correct base. * **Documentation** * Added startup warning and remediation guidance for unsafe compression middleware callbacks. * **Tests** * Added tests covering changelog prerelease-link cleanup behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
…k cleanup (#2596) ## Summary - Added missing changelog entries for PRs #2439, #2477, #2554 - Stamped version header for 16.4.0.rc.9 (collapsed rc.8 into rc.9) - **Fixed rake task bug**: `collapse_prerelease_sections` removed section headers but left behind orphaned compare links at the bottom of CHANGELOG.md. Added `cleanup_collapsed_prerelease_links` to remove these links and update `[unreleased]` to compare from the last stable version. ## Changes 1. **CHANGELOG.md**: New entries + rc.9 stamp + removed orphaned `[16.4.0.rc.8]` link 2. **update_changelog.rake**: Added `cleanup_collapsed_prerelease_links` function, called from `prepare_changelog_for_auto_version` 3. **update_changelog_rake_helpers_spec.rb**: 3 new tests covering link cleanup (multi-prerelease chain, single prerelease, no prereleases) ## Skipped PRs (not user-visible) - #2593 — test only (lock instrumentation) - #2591 — test only (Jest clearMocks) - #2588 — internal refactoring (Thor shell output) - #2584 — docs restructuring - #2587 — internal tooling (release script) - #2589 — docs fix (JWT claim name) - #2586 — docs/CI (dead link removal) ## Test plan - [x] All 13 rake helper tests pass (3 new) - [ ] CI passes - [ ] After merge, run `rake release` to publish 16.4.0.rc.9 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed bin/setup failing in pnpm workspace member directories. * **New Features** * Added host configuration option for the Node Renderer Fastify worker. * **Improvements / Changed** * Automatic installation of react_on_rails_pro enabled for --rsc/--pro flags. * Clarified Pro-installation signaling related to immediate hydration warnings. * Updated changelog to include RC9 entries and ensure Unreleased links point to the correct base. * **Documentation** * Added startup warning and remediation guidance for unsafe compression middleware callbacks. * **Tests** * Added tests covering changelog prerelease-link cleanup behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 participant
Summary
docs/root intodocs/oss/, creating a clean sibling structure withdocs/pro/(already consolidated by Consolidate Pro docs into docs/pro and remove legacy GitBook config #2556)MONOREPO_MERGER_PLAN.md, etc.) and testimonials tointernal/(extends Docs: move internal-only docs out of published docs trees #2414's pattern)docs/README.mdas a unified entry point with clear navigation to both OSS and Pro docsREADME.md,AGENTS.md,CHANGELOG.md,NEWS.md, Ruby source files, generator templates, and spec initializersNew structure
Closes #2583
Test plan
./and../within the preserved subdirectory structure)docs/pro/caching.md→../oss/core-concepts/...)sc-websiterepo (companion task)bundle exec rubocopon changed Ruby files (done, passing)🤖 Generated with Claude Code
Note
Low Risk
Documentation and link updates only; no behavioral changes beyond updated help text URLs and doc navigation paths.
Overview
Moves the open-source documentation tree under
docs/oss/to cleanly separate it from the existingdocs/pro/docs, and rewritesdocs/README.mdto act as a unified navigation landing page for both OSS and Pro.Updates doc links across repository-facing files (
README.md,NEWS.md,CHANGELOG.md,AGENTS.md) and in-code pointers (generator initializer template,react_on_rails:doctoroutput,TestHelpercomment, dummy app initializers, and a few Pro docs) to reference the newdocs/oss/...paths.Written by Cursor Bugbot for commit 77c3d67. Configure here.
Summary by CodeRabbit