Add npm READMEs, Pro quick-start guide, and llms.txt for LLM discoverability#2466
Add npm READMEs, Pro quick-start guide, and llms.txt for LLM discoverability#2466AbanoubGhadban wants to merge 2 commits intomasterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (3)
WalkthroughAdds comprehensive documentation and READMEs for React on Rails Pro (npm packages and a pro quick-start guide), an llms reference file, and small example edits in the gem README to correct the Node Renderer filename, import path, and rename Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
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 |
Greptile SummaryAdds npm READMEs, Pro quick-start guide, and llms.txt to improve LLM discoverability and correct setup guidance. Key improvements:
Critical issue:
Confidence Score: 3/5
Important Files Changed
Last reviewed commit: fdd60ac |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,106 @@ | |||
| # react-on-rails | |||
There was a problem hiding this comment.
README.md won't be published to npm. The "files" field in package.json only includes lib/** files, which excludes README.md. Add "README.md" to the files array in packages/react-on-rails/package.json:
| # react-on-rails | |
| "files": [ | |
| "README.md", | |
| "lib/**/*.js", |
| @@ -0,0 +1,97 @@ | |||
| # react-on-rails-pro | |||
There was a problem hiding this comment.
README.md won't be published to npm. The "files" field in package.json only includes lib/** files. Add "README.md" to the files array in packages/react-on-rails-pro/package.json:
| # react-on-rails-pro | |
| "files": [ | |
| "README.md", | |
| "lib/**/*.js", |
PR Review: Add npm READMEs, Pro quick-start guide, and llms.txtOverall this is a well-structured PR that addresses a real problem (LLMs generating incorrect setup code). The content is largely accurate and the motivation is sound. I found a few issues worth addressing before merge. Critical:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ### Node-side (client/node-renderer.js) | ||
|
|
||
| ```js | ||
| const { reactOnRailsProNodeRenderer } = require('react-on-rails-pro-node-renderer'); |
There was a problem hiding this comment.
The generator places node-renderer.js at client/node-renderer.js (as shown in the table above), so __dirname resolves to client/. The correct path to put the cache at the project root (matching the actual template) is '../.node-renderer-bundles', not './.node-renderer-bundles'.
| const { reactOnRailsProNodeRenderer } = require('react-on-rails-pro-node-renderer'); | |
| serverBundleCachePath: path.resolve(__dirname, '../.node-renderer-bundles'), |
| **Important:** When using the `react_on_rails_pro` Ruby gem, you **must** use this package (`react-on-rails-pro`) instead of `react-on-rails`. The base gem will reject `react-on-rails` if the Pro gem is detected. | ||
|
|
||
| ## Usage | ||
|
|
There was a problem hiding this comment.
The sentence "The base gem will reject react-on-rails if the Pro gem is detected" is confusing — "the base gem" is ambiguous and it's the Pro gem that enforces this requirement, not the base gem. Suggested rewrite:
| **Important:** When using the `react_on_rails_pro` Ruby gem, you **must** use this package (`react-on-rails-pro`) instead of `react-on-rails`. If the Pro gem detects the base `react-on-rails` npm package at runtime, it will raise an error. |
|
|
||
| - [Node Renderer Basics](https://www.shakacode.com/react-on-rails-pro/docs/node-renderer/basics/) | ||
| - [JavaScript Configuration](https://www.shakacode.com/react-on-rails-pro/docs/node-renderer/js-configuration/) | ||
| - [Rails Configuration](https://www.shakacode.com/react-on-rails-pro/docs/configuration/) |
There was a problem hiding this comment.
setupTracing is imported here but never used in the example. Either add an example usage or remove it from the import to avoid confusing copy-paste.
| - [Rails Configuration](https://www.shakacode.com/react-on-rails-pro/docs/configuration/) | |
| import { addNotifier } from 'react-on-rails-pro-node-renderer/integrations/api'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/react-on-rails-pro/README.md (1)
33-39: RSC example importsRSCRouteandwrapServerComponentRendererbut never uses themThe imports on lines 34–36 include
RSCRouteandwrapServerComponentRenderer, but onlyregisterServerComponentis exercised. Consider either removing the unused imports from the snippet or expanding the example to showRSCRouterendering.✨ Option A – trim to what the example actually demonstrates
-import { RSCRoute } from 'react-on-rails-pro/RSCRoute'; -import registerServerComponent from 'react-on-rails-pro/registerServerComponent/client'; -import { wrapServerComponentRenderer } from 'react-on-rails-pro/wrapServerComponentRenderer/client'; - -// Register a server component for client-side hydration -registerServerComponent({ MyServerComponent }); +import registerServerComponent from 'react-on-rails-pro/registerServerComponent/client'; + +// Register a server component for client-side hydration +registerServerComponent({ MyServerComponent });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-on-rails-pro/README.md` around lines 33 - 39, The README snippet imports RSCRoute and wrapServerComponentRenderer but only uses registerServerComponent; either remove the unused imports (delete RSCRoute and wrapServerComponentRenderer from the import lines) or expand the example to demonstrate their usage by showing RSCRoute rendering and invoking wrapServerComponentRenderer (e.g., reference RSCRoute and wrapServerComponentRenderer in the snippet so all imported symbols are exercised alongside registerServerComponent).packages/react-on-rails-pro-node-renderer/README.md (1)
138-144:setupTracingis imported but not used in the error-reporting exampleEither use
setupTracingin the snippet or remove it from the import.✨ Proposed fix
-import { addNotifier, setupTracing } from 'react-on-rails-pro-node-renderer/integrations/api'; +import { addNotifier } from 'react-on-rails-pro-node-renderer/integrations/api';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-on-rails-pro-node-renderer/README.md` around lines 138 - 144, The example imports setupTracing but never uses it; update the snippet to either use setupTracing or remove it—simplest fix: change the import to only bring addNotifier (remove setupTracing) so the sample shows a used import (refer to addNotifier and setupTracing in the README example).
🤖 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/getting-started/pro-quick-start.md`:
- Line 149: The markdown entry for the Code Splitting link currently points to a
404 URL; locate the line containing the link text "[Code
Splitting](https://www.shakacode.com/react-on-rails-pro/docs/code-splitting/)"
in pro-quick-start.md and either replace the broken URL with the correct working
URL for the Code Splitting docs or remove the entire bullet (the "[Code
Splitting](...)" link) until the target page is restored, ensuring the
surrounding list formatting remains valid.
- Around line 107-115: The example calling reactOnRailsProNodeRenderer uses
path.resolve but never imports path and uses the wrong env var name for
workersCount; add a require('path') at top so path is defined, and change the
workersCount expression from reading process.env.NODE_RENDERER_CONCURRENCY to
process.env.RENDERER_WORKERS_COUNT (fallback to 3 unchanged) so
serverBundleCachePath and workersCount are correct and consistent with the
README.
In `@packages/react-on-rails-pro-node-renderer/README.md`:
- Around line 114-131: Add a brief note above the Advanced example explaining it
uses ESM syntax so users must either use .mjs or set "type":"module" in
package.json or convert the imports to CommonJS; reference the example's symbols
(masterRun, run, configureFastify, cluster) so readers know the note applies to
that snippet.
- Around line 79-81: The README's fenced code block for the Procfile entry lacks
a language specifier which triggers MD040; update the Procfile code block
surrounding the line "node-renderer: node node-renderer.js" to include a
language identifier (for example use "text" or "procfile") so the block becomes
```text ... ``` (or ```procfile ... ```) to silence the markdownlint warning.
---
Nitpick comments:
In `@packages/react-on-rails-pro-node-renderer/README.md`:
- Around line 138-144: The example imports setupTracing but never uses it;
update the snippet to either use setupTracing or remove it—simplest fix: change
the import to only bring addNotifier (remove setupTracing) so the sample shows a
used import (refer to addNotifier and setupTracing in the README example).
In `@packages/react-on-rails-pro/README.md`:
- Around line 33-39: The README snippet imports RSCRoute and
wrapServerComponentRenderer but only uses registerServerComponent; either remove
the unused imports (delete RSCRoute and wrapServerComponentRenderer from the
import lines) or expand the example to demonstrate their usage by showing
RSCRoute rendering and invoking wrapServerComponentRenderer (e.g., reference
RSCRoute and wrapServerComponentRenderer in the snippet so all imported symbols
are exercised alongside registerServerComponent).
| ```js | ||
| const { reactOnRailsProNodeRenderer } = require('react-on-rails-pro-node-renderer'); | ||
|
|
||
| reactOnRailsProNodeRenderer({ | ||
| serverBundleCachePath: path.resolve(__dirname, './.node-renderer-bundles'), | ||
| port: Number(process.env.RENDERER_PORT) || 3800, | ||
| supportModules: true, | ||
| workersCount: Number(process.env.NODE_RENDERER_CONCURRENCY || 3), | ||
| }); |
There was a problem hiding this comment.
Node-side snippet: missing require('path') and mismatched env variable name
Two issues in the generated node-renderer.js example:
path.resolve(...)is called on line 111 butconst path = require('path')is never imported — the snippet as written will throwReferenceError: path is not defined.- The env variable for worker count is
NODE_RENDERER_CONCURRENCYhere, but thereact-on-rails-pro-node-rendererREADME (and configuration table) document it asRENDERER_WORKERS_COUNT. These need to be consistent.
🐛 Proposed fix
-```js
+```js
+const path = require('path');
const { reactOnRailsProNodeRenderer } = require('react-on-rails-pro-node-renderer');
reactOnRailsProNodeRenderer({
serverBundleCachePath: path.resolve(__dirname, './.node-renderer-bundles'),
port: Number(process.env.RENDERER_PORT) || 3800,
supportModules: true,
- workersCount: Number(process.env.NODE_RENDERER_CONCURRENCY || 3),
+ workersCount: Number(process.env.RENDERER_WORKERS_COUNT) || 3,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/getting-started/pro-quick-start.md` around lines 107 - 115, The example
calling reactOnRailsProNodeRenderer uses path.resolve but never imports path and
uses the wrong env var name for workersCount; add a require('path') at top so
path is defined, and change the workersCount expression from reading
process.env.NODE_RENDERER_CONCURRENCY to process.env.RENDERER_WORKERS_COUNT
(fallback to 3 unchanged) so serverBundleCachePath and workersCount are correct
and consistent with the README.
| - [Node Renderer Configuration](https://www.shakacode.com/react-on-rails-pro/docs/node-renderer/js-configuration/) — All Node Renderer options | ||
| - [Caching Guide](https://www.shakacode.com/react-on-rails-pro/docs/caching/) — Fragment and prerender caching | ||
| - [Streaming SSR](https://www.shakacode.com/react-on-rails-pro/docs/streaming-server-rendering/) — Progressive rendering | ||
| - [Code Splitting](https://www.shakacode.com/react-on-rails-pro/docs/code-splitting/) — Loadable components with SSR |
There was a problem hiding this comment.
Broken link — 404 confirmed by CI
The link https://www.shakacode.com/react-on-rails-pro/docs/code-splitting/ returns 404 (flagged by the GitHub Actions "Check Markdown Links" pipeline). Update it to the correct URL or remove it until the page exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/getting-started/pro-quick-start.md` at line 149, The markdown entry for
the Code Splitting link currently points to a 404 URL; locate the line
containing the link text "[Code
Splitting](https://www.shakacode.com/react-on-rails-pro/docs/code-splitting/)"
in pro-quick-start.md and either replace the broken URL with the correct working
URL for the Code Splitting docs or remove the entire bullet (the "[Code
Splitting](...)" link) until the target page is restored, ensuring the
surrounding list formatting remains valid.
| ``` | ||
| node-renderer: node node-renderer.js | ||
| ``` |
There was a problem hiding this comment.
Missing language specifier on Procfile code block (MD040)
The fenced code block for the Procfile entry has no language identifier, triggering the markdownlint MD040 warning. Use text (or procfile) to silence it.
🛠 Proposed fix
-```
+```text
node-renderer: node node-renderer.js</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-on-rails-pro-node-renderer/README.md` around lines 79 - 81,
The README's fenced code block for the Procfile entry lacks a language specifier
which triggers MD040; update the Procfile code block surrounding the line
"node-renderer: node node-renderer.js" to include a language identifier (for
example use "text" or "procfile") so the block becomes ```text ... ``` (or
```procfile ... ```) to silence the markdownlint warning.
| ```js | ||
| import masterRun from 'react-on-rails-pro-node-renderer/master'; | ||
| import run, { configureFastify } from 'react-on-rails-pro-node-renderer/worker'; | ||
| import cluster from 'cluster'; | ||
|
|
||
| const config = { /* your config */ }; | ||
|
|
||
| configureFastify((app) => { | ||
| app.get('/health', (request, reply) => { | ||
| reply.send({ status: 'ok' }); | ||
| }); | ||
| }); | ||
|
|
||
| if (cluster.isPrimary) { | ||
| masterRun(config); | ||
| } else { | ||
| run(config); | ||
| } |
There was a problem hiding this comment.
Advanced section uses ESM import while Quick Start uses CommonJS require — add a note
The Quick Start example uses require() (CommonJS) but the Advanced section switches to import (ESM) without explanation. Users who copy the advanced snippet into a plain .js file (the default CJS context) will get a syntax error.
Consider adding a brief note such as: "The following example uses ES Modules. Use .mjs extension or add "type": "module" to your package.json, or convert imports to require() calls."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-on-rails-pro-node-renderer/README.md` around lines 114 - 131,
Add a brief note above the Advanced example explaining it uses ESM syntax so
users must either use .mjs or set "type":"module" in package.json or convert the
imports to CommonJS; reference the example's symbols (masterRun, run,
configureFastify, cluster) so readers know the note applies to that snippet.
…ability LLMs consistently fail to generate correct React on Rails Pro setup code because the npm packages have no READMEs, there's no single quick-start page, and the correct API names are not discoverable from training data. Changes: - Add README.md to all 3 npm packages (react-on-rails, react-on-rails-pro, react-on-rails-pro-node-renderer) with correct API examples, config options, and package relationship documentation - Fix incorrect example in Pro gem README (wrong scoped package name @shakacode-tools/... and deprecated bundlePath config key) - Add docs/getting-started/pro-quick-start.md — a single self-contained guide for setting up a Pro app from scratch using the --pro generator - Add llms.txt at repo root following the llms.txt convention, providing LLMs with correct package relationships, API names, and config keys Addresses #2465 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AbanoubGhadban
force-pushed
the
abanoub/improve-llm-discoverability
branch
from
fdd60ac to
ec95e93
Compare
Review SummaryThe goal of this PR (improving LLM discoverability by adding npm READMEs) is well-motivated and the content is generally accurate. However, there is one critical defect that completely undermines the main purpose, plus a few inconsistencies worth fixing before merge. Critical: README files will not be published to npmBoth
Neither Fix: Add Inconsistency:
|
| File | Env var used |
|---|---|
docs/getting-started/pro-quick-start.md |
REACT_RENDERER_URL (matches the generator template) |
packages/react-on-rails-pro-node-renderer/README.md |
RENDERER_URL |
The generator template (lib/generators/react_on_rails/templates/pro/base/config/initializers/react_on_rails_pro.rb.tt) uses REACT_RENDERER_URL, so the node-renderer README should be updated to match.
Clarification needed: workersCount env variable
The node-renderer README config table says the env variable for workersCount is RENDERER_WORKERS_COUNT. But the generator template (client/node-renderer.js) reads NODE_RENDERER_CONCURRENCY and passes it manually as workersCount. These are different things — the renderer supports both (via its internal env and via the JS config object), but someone following the node-renderer README while also using the generator-created file will be confused about which env var to set. The table footnote or a note in the README should clarify this.
| @@ -0,0 +1,106 @@ | |||
| # react-on-rails | |||
There was a problem hiding this comment.
This README will not be included in the npm publish. packages/react-on-rails/package.json has a files field that only matches lib/**/*.js, lib/**/*.cjs, lib/**/*.mjs, and lib/**/*.d.ts — README.md does not match any of these patterns.
Fix: add "README.md" to the files array in packages/react-on-rails/package.json:
| # react-on-rails | |
| # react-on-rails |
(The README content is fine; the change needed is in package.json, not here.)
"files": [
"lib/**/*.js",
"lib/**/*.cjs",
"lib/**/*.mjs",
"lib/**/*.d.ts",
"README.md"
]
| @@ -0,0 +1,97 @@ | |||
| # react-on-rails-pro | |||
There was a problem hiding this comment.
Same issue as the base package: packages/react-on-rails-pro/package.json has:
"files": [
"lib/**/*.js",
"lib/**/*.d.ts"
]README.md is excluded. Add "README.md" to that files array to ensure it gets published to npm.
size-limit report 📦
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/getting-started/pro-quick-start.md (2)
149-149: Broken link (404) confirmed by CI pipeline — already notedThis was flagged in a previous review round and confirmed by the GitHub Actions "Check Markdown Links" pipeline failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/getting-started/pro-quick-start.md` at line 149, The markdown contains a broken external link for "Code Splitting" pointing to https://www.shakacode.com/react-on-rails-pro/docs/code-splitting/; replace the dead URL with a working target (either the current ShakaCode page for code-splitting or an alternative authoritative resource) or remove the link if no replacement exists, ensuring the link text "Code Splitting" remains accurate; update the link in the pro-quick-start document so the CI "Check Markdown Links" pipeline no longer reports a 404.
107-115:require('path')is missing and env var name is inconsistent — already notedThis was flagged in a previous review round.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/getting-started/pro-quick-start.md` around lines 107 - 115, Add the missing require('path') at the top and make the environment variable names consistent: import path (e.g., const path = require('path')) and ensure you use the same naming scheme for port and concurrency (either both RENDERER_* or both NODE_RENDERER_*); update the reactOnRailsProNodeRenderer call (reactOnRailsProNodeRenderer, serverBundleCachePath, port, workersCount) to reference the chosen env vars (e.g., process.env.RENDERER_PORT and process.env.RENDERER_CONCURRENCY or process.env.NODE_RENDERER_PORT and process.env.NODE_RENDERER_CONCURRENCY) so names are no longer inconsistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-on-rails-pro/README.md`:
- Around line 44-47: Update the unlabeled code fence in the README diagram to
include a language specifier (e.g., change the opening ``` to ```text or
```ascii) so the ASCII-art block in the README.md is labeled and satisfies
MD040; ensure the opening fence is updated and the closing ``` remains present
around the block containing "react-on-rails-pro (this package) └──
react-on-rails (base package, automatically installed as dependency)".
---
Duplicate comments:
In `@docs/getting-started/pro-quick-start.md`:
- Line 149: The markdown contains a broken external link for "Code Splitting"
pointing to https://www.shakacode.com/react-on-rails-pro/docs/code-splitting/;
replace the dead URL with a working target (either the current ShakaCode page
for code-splitting or an alternative authoritative resource) or remove the link
if no replacement exists, ensuring the link text "Code Splitting" remains
accurate; update the link in the pro-quick-start document so the CI "Check
Markdown Links" pipeline no longer reports a 404.
- Around line 107-115: Add the missing require('path') at the top and make the
environment variable names consistent: import path (e.g., const path =
require('path')) and ensure you use the same naming scheme for port and
concurrency (either both RENDERER_* or both NODE_RENDERER_*); update the
reactOnRailsProNodeRenderer call (reactOnRailsProNodeRenderer,
serverBundleCachePath, port, workersCount) to reference the chosen env vars
(e.g., process.env.RENDERER_PORT and process.env.RENDERER_CONCURRENCY or
process.env.NODE_RENDERER_PORT and process.env.NODE_RENDERER_CONCURRENCY) so
names are no longer inconsistent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/getting-started/pro-quick-start.mdllms.txtpackages/react-on-rails-pro-node-renderer/README.mdpackages/react-on-rails-pro/README.mdpackages/react-on-rails/README.mdreact_on_rails_pro/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react-on-rails-pro-node-renderer/README.md
- llms.txt
| ``` | ||
| react-on-rails-pro (this package) | ||
| └── react-on-rails (base package, automatically installed as dependency) | ||
| ``` |
There was a problem hiding this comment.
Add a language specifier to the unlabeled code fence (MD040)
The ASCII-art diagram block opens with a plain ``` fence, triggering the MD040 lint rule.
🐛 Proposed fix
-```
+```text
react-on-rails-pro (this package)
└── react-on-rails (base package, automatically installed as dependency)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-on-rails-pro/README.md` around lines 44 - 47, Update the
unlabeled code fence in the README diagram to include a language specifier
(e.g., change the opening ``` to ```text or ```ascii) so the ASCII-art block in
the README.md is labeled and satisfies MD040; ensure the opening fence is
updated and the closing ``` remains present around the block containing
"react-on-rails-pro (this package) └── react-on-rails (base package,
automatically installed as dependency)".
All React on Rails Pro gems and npm packages are now in the public monorepo. Remove "you will need access to our private repository" notes, update GitHub links from the old shakacode/react_on_rails_pro repo to shakacode/react_on_rails monorepo paths, remove obsolete GitHub Packages publishing instructions from releasing guide, and fix tense in the upgrade guide. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review: Documentation Accuracy IssuesGood effort on centralizing the LLM-discoverability content — the motivation is sound and most of the changes are accurate. However, I found several factual inconsistencies across the new docs that could mislead developers more than the current state of affairs. These should be fixed before merging. Critical: Wrong env variable name in node-renderer READMEIn config.renderer_url = ENV["RENDERER_URL"] || "http://localhost:3800"But the actual generator template ( Incorrect
|
| config.renderer_url = ENV["RENDERER_URL"] || "http://localhost:3800" | ||
| config.renderer_password = ENV["RENDERER_PASSWORD"] |
There was a problem hiding this comment.
Wrong env variable name — RENDERER_URL should be REACT_RENDERER_URL. The generator template (lib/generators/react_on_rails/templates/pro/base/config/initializers/react_on_rails_pro.rb.tt) and all other docs (pro-quick-start.md, llms.txt) consistently use REACT_RENDERER_URL. Using RENDERER_URL here will silently fall back to the hardcoded default and the env var will have no effect.
Also note the missing default for renderer_password — the generator template uses ENV.fetch("RENDERER_PASSWORD", "devPassword") for development parity.
| config.renderer_url = ENV["RENDERER_URL"] || "http://localhost:3800" | |
| config.renderer_password = ENV["RENDERER_PASSWORD"] | |
| config.renderer_url = ENV.fetch("REACT_RENDERER_URL", "http://localhost:3800") | |
| config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword") |
| const { reactOnRailsProNodeRenderer } = require('react-on-rails-pro-node-renderer'); | ||
|
|
||
| reactOnRailsProNodeRenderer({ | ||
| serverBundleCachePath: path.resolve(__dirname, './.node-renderer-bundles'), |
There was a problem hiding this comment.
Wrong relative path. Since the generator places this file at client/node-renderer.js, __dirname is the client/ directory. Using './.node-renderer-bundles' puts the cache inside client/, not the project root. The actual generator template uses '../.node-renderer-bundles' to resolve to the project root.
Also missing the const path = require('path'); import at the top of this snippet.
| serverBundleCachePath: path.resolve(__dirname, './.node-renderer-bundles'), | |
| serverBundleCachePath: path.resolve(__dirname, '../.node-renderer-bundles'), |
| ## What the generator configured | ||
|
|
||
| ### Rails-side (config/initializers/react_on_rails_pro.rb) | ||
|
|
||
| ```ruby | ||
| ReactOnRailsPro.configure do |config| | ||
| config.server_renderer = "NodeRenderer" | ||
| config.renderer_url = ENV.fetch("REACT_RENDERER_URL", "http://localhost:3800") | ||
| config.renderer_password = ENV.fetch("RENDERER_PASSWORD", "devPassword") | ||
| config.prerender_caching = true | ||
| end | ||
| ``` |
There was a problem hiding this comment.
The section title "What the generator configured" sets the expectation of accuracy, but the snippet is significantly simplified compared to the actual generated file. The real generator template also includes ssr_timeout, renderer_request_retry_limit, renderer_use_fallback_exec_js, tracing, etc. A developer who looks at their generated file will find it doesn't match.
Consider either:
- Renaming this to "Minimal configuration example" and noting the generator produces a fuller version
- Showing the complete output that matches the actual template
Otherwise developers opening their generated initializer will be confused by options not shown here.
| ```js | ||
| import masterRun from 'react-on-rails-pro-node-renderer/master'; | ||
| import run, { configureFastify } from 'react-on-rails-pro-node-renderer/worker'; | ||
| import cluster from 'cluster'; |
There was a problem hiding this comment.
The Advanced section switches to ESM import syntax without any note about it, while the entire Quick Start and the actual generator template use CommonJS require(). The package doesn't declare "type": "module" in its package.json, so CJS is the default.
Consider adding a note clarifying this is the ESM usage pattern, or converting to require() to be consistent with the Quick Start above.
| | `logHttpLevel` | `RENDERER_LOG_HTTP_LEVEL` | `'error'` | HTTP server log level | | ||
| | `serverBundleCachePath` | `RENDERER_SERVER_BUNDLE_CACHE_PATH` | Auto-detected or `/tmp/...` | Directory for cached server bundles | | ||
| | `supportModules` | `RENDERER_SUPPORT_MODULES` | `false` | Enable Node.js globals in VM context (`Buffer`, `process`, `setTimeout`, etc.) | | ||
| | `workersCount` | `RENDERER_WORKERS_COUNT` | CPU count - 1 | Number of worker processes | |
There was a problem hiding this comment.
The env variable name here (RENDERER_WORKERS_COUNT) doesn't match what the generator template uses. The actual generator template (lib/generators/react_on_rails/templates/pro/base/client/node-renderer.js) reads NODE_RENDERER_CONCURRENCY:
workersCount: env.NODE_RENDERER_CONCURRENCY != null ? Number(env.NODE_RENDERER_CONCURRENCY) : 3,If the package itself reads RENDERER_WORKERS_COUNT internally (separate from the generator config), this needs a clarifying note. If not, this documents a non-functional env var name for the most common use case (following the generator).
1 participant
Summary
LLMs consistently fail to generate correct React on Rails Pro setup code. After deep analysis (#2465), the root causes are:
@shakacode-tools/scoped name and deprecatedbundlePathThis PR addresses the low-effort, high-impact recommendations from #2465.
Changes
react-on-rails,react-on-rails-pro,react-on-rails-pro-node-renderer) with correct API examples, config option tables, and package relationship docs@shakacode-tools/react-on-rails-pro-node-renderer→react-on-rails-pro-node-renderer, deprecatedbundlePath→serverBundleCachePathdocs/getting-started/pro-quick-start.md— single self-contained guide for Pro setup from scratch using the--progeneratorllms.txtat repo root — follows the llms.txt convention providing LLMs with correct package relationships, API names, and config keysWhat this does NOT include
--pro/--rsc) — already merged in Add --pro and --rsc flags to install generator #2284react_on_rails_progem — open in Auto-install react_on_rails_pro gem when --rsc/--pro flags are used #2439Test plan
packages/react-on-rails-pro-node-renderer/lib/)rails new+rails generate react_on_rails:install --prollms.txtconfig examples match the generator templates inlib/generators/react_on_rails/templates/pro/filesfield excluding them, orfilesfield includesREADME.md)Closes #2465
🤖 Generated with Claude Code
Summary by CodeRabbit