Skip to content

Commit 641b482

Browse files
justin808claude
andcommitted
fix(pro-dummy): make manual node-renderer validation reliable
Addresses the follow-up issues surfaced during review of #3158, where validating a renderer fix locally required repo-specific workarounds. Three independent breakages, one shared cause: the dummy app could make a correct fix look broken or silently validate stale build artifacts. 1. `bin/dev` failed with `overlay.sockPort should be a number` because `bin/dev` exports `SHAKAPACKER_DEV_SERVER_PORT` as a string and Shakapacker passes it through unchanged on `devServer.port`. The Pro dummy's webpack development config forwarded that string straight to `@pmmmwh/react-refresh-webpack-plugin`, whose schema requires a number. Coerce with `Number()` (applied to both Pro dummy and execjs dummy). 2. Rails precompile hook failed with `cannot load such file -- ostruct` on Ruby 3.5+ because the Pro Gemfile was missing the explicit `ostruct`, `logger`, `benchmark` declarations the open-source Gemfile already had. 3. The dummy consumes the *built* `react-on-rails-pro-node-renderer/lib/`, not the TypeScript sources, so renderer fixes weren't exercised until the package was rebuilt. Add `node-renderer:fresh` and `node-renderer:build-watch` scripts, document the rebuild requirement inline in `Procfile.dev`, expand the Pro CLAUDE.md, and add a dedicated `validating-node-renderer-changes.md` checklist for reviewers. Fixes #3177 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d3b546e commit 641b482

11 files changed

Lines changed: 162 additions & 4 deletions

File tree

.claude/docs/manual-dev-environment-testing.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Automated tests can pass while the development environment is completely broken. CI starts services explicitly and runs in a controlled environment — it does not exercise `bin/dev`, Procfile orchestration, or the actual browser experience. This guide ensures agents verify the dev environment works end-to-end before submitting a PR.
44

5-
**Related:** [PR Testing Guide](pr-testing-guide.md), [Testing Build Scripts](testing-build-scripts.md)
5+
**Related:** [PR Testing Guide](pr-testing-guide.md), [Testing Build Scripts](testing-build-scripts.md), [Validating Node Renderer Changes](validating-node-renderer-changes.md)
66

77
## The Rule
88

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# Validating Node Renderer Changes
2+
3+
Manual validation checklist for changes under `packages/react-on-rails-pro-node-renderer/src/**`.
4+
5+
**Why this exists:** the Pro dummy app consumes the _built_ renderer at
6+
`packages/react-on-rails-pro-node-renderer/lib/**`. Editing TypeScript under `src/` does
7+
nothing until that lib output is regenerated, so a correct fix can look broken (or worse,
8+
a regression can look fine because the dummy is still running stale lib output).
9+
10+
**Related:** [Manual Dev Environment Testing](manual-dev-environment-testing.md)
11+
12+
## When This Applies
13+
14+
If your PR touches **any** of these, run through the checklist below:
15+
16+
- `packages/react-on-rails-pro-node-renderer/src/**`
17+
- `packages/react-on-rails-pro-node-renderer/package.json` (especially `protocolVersion`,
18+
`exports`, or runtime dependencies)
19+
- Any worker pool, JWT auth, integrations (Sentry/Honeybadger), or VM-context code in the
20+
renderer
21+
22+
## Pre-flight: Toolchain
23+
24+
The Pro dummy app requires Ruby 3.3.x. If you are on a newer Ruby (3.5+), the dummy's
25+
Gemfile already pulls in `ostruct`, `logger`, and `benchmark` as explicit gems to silence
26+
warnings, but you may still hit unrelated incompatibilities. When in doubt, switch to the
27+
documented Ruby version:
28+
29+
```bash
30+
mise use ruby@3.3.7 # or rbenv/asdf equivalent
31+
cd react_on_rails_pro/spec/dummy
32+
bundle install
33+
```
34+
35+
## Step 1: Rebuild the Renderer Package
36+
37+
Pick **one** of these depending on how you plan to iterate:
38+
39+
**One-shot rebuild (recommended for a single validation pass):**
40+
41+
```bash
42+
pnpm --filter react-on-rails-pro-node-renderer run build
43+
```
44+
45+
**Or use the dummy's convenience script:**
46+
47+
```bash
48+
cd react_on_rails_pro/spec/dummy
49+
pnpm run node-renderer:fresh # builds the package, then starts the renderer
50+
```
51+
52+
**Watch mode (recommended when iterating on the renderer source):**
53+
54+
Either uncomment the `node-renderer-build` line in `react_on_rails_pro/spec/dummy/Procfile.dev`,
55+
or in a separate terminal run:
56+
57+
```bash
58+
pnpm --filter react-on-rails-pro-node-renderer run build-watch
59+
```
60+
61+
> If you skip this step, the dummy app will silently keep using the previous lib build.
62+
> Symptoms: a fix you just applied does not change behavior, or a regression you expected
63+
> to see does not appear.
64+
65+
## Step 2: Start the Dummy App
66+
67+
```bash
68+
cd react_on_rails_pro/spec/dummy
69+
bin/dev
70+
```
71+
72+
Verify:
73+
74+
- [ ] `bin/dev` starts without `overlay.sockPort should be a number` (webpack-dev-server)
75+
- [ ] `bin/dev` starts without `cannot load such file -- ostruct` (Rails precompile)
76+
- [ ] All Procfile.dev processes are healthy after 30 seconds
77+
- [ ] The `node-renderer` process logs that it bound to port 3800
78+
79+
## Step 3: Exercise the SSR Endpoints
80+
81+
For PRs touching streaming, hydration, RSC, or VM-context code, hit the routes that
82+
actually exercise the renderer (not just static pages):
83+
84+
- [ ] `http://localhost:3000/stream_native_metadata` — renders without `ReferenceError`
85+
(e.g. `performance is not defined`) in the renderer logs
86+
- [ ] `http://localhost:3000/hybrid_metadata_streaming` — same
87+
- [ ] Any route specifically related to your change
88+
89+
For each route:
90+
91+
- [ ] Page returns 200 and renders SSR content (view-source shows component markup)
92+
- [ ] No errors in the `node-renderer` Procfile pane
93+
- [ ] No errors in the Rails server log
94+
- [ ] No errors in the browser console
95+
96+
## Step 4: Confirm You Tested the New Code
97+
98+
It is easy to validate stale lib output without realizing it. Confirm:
99+
100+
- [ ] `lib/` mtime is newer than the `src/` file you changed (compare
101+
`ls -la packages/react-on-rails-pro-node-renderer/lib/` against
102+
`ls -la packages/react-on-rails-pro-node-renderer/src/`)
103+
- [ ] If you used watch mode, you saw a rebuild line in the watcher output after your
104+
most recent edit
105+
- [ ] Restart the `node-renderer` Procfile process after the rebuild — `node` does not
106+
hot-reload required modules
107+
108+
## Reporting Results in the PR
109+
110+
```markdown
111+
## Node Renderer Validation
112+
113+
- [x] Rebuilt `react-on-rails-pro-node-renderer` package
114+
- [x] Verified `lib/` mtime newer than `src/` after edit
115+
- [x] `bin/dev` starts cleanly
116+
- [x] `/stream_native_metadata` renders without errors
117+
- [x] `/hybrid_metadata_streaming` renders without errors
118+
- [x] No errors in node-renderer logs
119+
```
120+
121+
If you cannot validate manually (e.g. no local Ruby toolchain), say so explicitly and
122+
note that the change is type-checked / unit-tested only.

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,6 @@ Use these docs for Claude-oriented operational guidance:
4545
- `.claude/docs/docs-competitive-landscape.md`
4646
- `.claude/docs/docs-templates.md`
4747
- `.claude/docs/manual-dev-environment-testing.md`
48+
- `.claude/docs/validating-node-renderer-changes.md`
4849

4950
For Pro-package specifics, also read `react_on_rails_pro/CLAUDE.md`.

react_on_rails_pro/CLAUDE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ The node renderer is a standalone Fastify HTTP server (separate Node.js process)
5858
- Integrations: Sentry, Honeybadger (optional peer deps)
5959
- Protocol versioning: `protocolVersion` in package.json must match gem expectations
6060

61+
**Validating source changes against the dummy app:** the dummy consumes the _built_
62+
`packages/react-on-rails-pro-node-renderer/lib/**`, so edits under `src/**` are not
63+
picked up until the package is rebuilt. Use one of:
64+
65+
- `pnpm --filter react-on-rails-pro-node-renderer run build` (one-shot)
66+
- `cd react_on_rails_pro/spec/dummy && pnpm run node-renderer:fresh` (rebuild + start)
67+
- `pnpm --filter react-on-rails-pro-node-renderer run build-watch` (watch in another shell)
68+
69+
See `.claude/docs/validating-node-renderer-changes.md` for the full checklist.
70+
6171
### Yalc Dependency Chain
6272

6373
Pro dummy's preinstall builds and links packages in this order:

react_on_rails_pro/Gemfile.development_dependencies

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ group :development, :test do
5454
gem "rbs", require: false
5555
gem "scss_lint", require: false
5656
gem "fakefs", require: "fakefs/safe"
57+
58+
# Added for Ruby 3.5+ compatibility to silence warnings.
59+
# jbuilder and other gems lazy-require these stdlib bits that became default gems in 3.5.
60+
gem "benchmark", require: false
61+
gem "logger", require: false
62+
gem "ostruct", require: false
5763
end
5864

5965
group :test do

react_on_rails_pro/Gemfile.lock

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ GEM
247247
racc (~> 1.4)
248248
nokogiri (1.19.2-x86_64-linux-gnu)
249249
racc (~> 1.4)
250+
ostruct (0.6.3)
250251
package_json (0.2.0)
251252
parallel (1.27.0)
252253
parser (3.3.10.0)
@@ -475,6 +476,7 @@ PLATFORMS
475476

476477
DEPENDENCIES
477478
amazing_print
479+
benchmark
478480
bootsnap
479481
bundler
480482
capybara (>= 3.38.0)
@@ -490,9 +492,11 @@ DEPENDENCIES
490492
jquery-rails
491493
launchy
492494
listen
495+
logger
493496
net-http
494497
net-imap
495498
net-smtp
499+
ostruct
496500
pg
497501
pry (>= 0.14.1)
498502
pry-byebug!

react_on_rails_pro/spec/dummy/Gemfile.lock

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ GEM
271271
racc (~> 1.4)
272272
nokogiri (1.19.2-x86_64-linux-musl)
273273
racc (~> 1.4)
274+
ostruct (0.6.3)
274275
package_json (0.2.0)
275276
parallel (1.27.0)
276277
parser (3.3.10.0)
@@ -524,6 +525,7 @@ PLATFORMS
524525

525526
DEPENDENCIES
526527
amazing_print
528+
benchmark
527529
bootsnap
528530
capybara (>= 3.38.0)
529531
capybara-screenshot
@@ -538,9 +540,11 @@ DEPENDENCIES
538540
jquery-rails
539541
launchy
540542
listen
543+
logger
541544
net-http
542545
net-imap
543546
net-smtp
547+
ostruct
544548
pg
545549
prism-rails
546550
pry (>= 0.14.1)

react_on_rails_pro/spec/dummy/Procfile.dev

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,9 @@ rails-server-assets: HMR=true SERVER_BUNDLE_ONLY=true bin/shakapacker --watch
1111
rails-rsc-assets: HMR=true RSC_BUNDLE_ONLY=true bin/shakapacker --watch
1212

1313
# Start Node server for server rendering.
14+
# NOTE: This consumes the *built* `packages/react-on-rails-pro-node-renderer/lib/**`.
15+
# When validating source changes under `packages/react-on-rails-pro-node-renderer/src/**`,
16+
# either rebuild once (`pnpm --filter react-on-rails-pro-node-renderer run build`) or
17+
# uncomment the `node-renderer-build` line below to watch and rebuild on every save.
18+
# node-renderer-build: pnpm --filter react-on-rails-pro-node-renderer run build-watch
1419
node-renderer: RENDERER_LOG_LEVEL=debug RENDERER_PORT=3800 node --inspect renderer/node-renderer.js

react_on_rails_pro/spec/dummy/config/webpack/development.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ const developmentEnvOnly = (clientWebpackConfig, _serverWebpackConfig) => {
1111
clientWebpackConfig.plugins.push(
1212
new ReactRefreshWebpackPlugin({
1313
overlay: {
14-
sockPort: devServer.port,
14+
// bin/dev sets SHAKAPACKER_DEV_SERVER_PORT as a string, which Shakapacker
15+
// surfaces unchanged on devServer.port. The plugin schema requires a number.
16+
sockPort: Number(devServer.port),
1517
},
1618
}),
1719
);

react_on_rails_pro/spec/dummy/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@
105105
"build:server": "RAILS_ENV=production NODE_ENV=production SERVER=true bin/shakapacker",
106106
"build:clean": "rm -rf public/webpack && rm -rf ssr-generated || true",
107107
"node-renderer-debug": "RENDERER_PORT=3800 ndb renderer/node-renderer.js",
108-
"node-renderer": "RENDERER_PORT=3800 node renderer/node-renderer.js"
108+
"node-renderer": "RENDERER_PORT=3800 node renderer/node-renderer.js",
109+
"node-renderer:fresh": "pnpm --filter react-on-rails-pro-node-renderer run build && RENDERER_PORT=3800 node renderer/node-renderer.js",
110+
"node-renderer:build-watch": "pnpm --filter react-on-rails-pro-node-renderer run build-watch"
109111
},
110112
"license": "UNLICENSED",
111113
"repository": {

0 commit comments

Comments
 (0)