diff --git a/.claude/docs/manual-dev-environment-testing.md b/.claude/docs/manual-dev-environment-testing.md index db776d6fc0..8c46be1778 100644 --- a/.claude/docs/manual-dev-environment-testing.md +++ b/.claude/docs/manual-dev-environment-testing.md @@ -2,7 +2,7 @@ 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. -**Related:** [PR Testing Guide](pr-testing-guide.md), [Testing Build Scripts](testing-build-scripts.md) +**Related:** [PR Testing Guide](pr-testing-guide.md), [Testing Build Scripts](testing-build-scripts.md), [Validating Node Renderer Changes](validating-node-renderer-changes.md) ## The Rule diff --git a/.claude/docs/validating-node-renderer-changes.md b/.claude/docs/validating-node-renderer-changes.md new file mode 100644 index 0000000000..1a17613c24 --- /dev/null +++ b/.claude/docs/validating-node-renderer-changes.md @@ -0,0 +1,132 @@ +# Validating Node Renderer Changes + +Manual validation checklist for changes under `packages/react-on-rails-pro-node-renderer/src/**`. + +**Why this exists:** the Pro dummy app consumes the _built_ renderer at +`packages/react-on-rails-pro-node-renderer/lib/**`. Editing TypeScript under `src/` does +nothing until that lib output is regenerated, so a correct fix can look broken (or worse, +a regression can look fine because the dummy is still running stale lib output). + +**Related:** [Manual Dev Environment Testing](manual-dev-environment-testing.md) + +## When This Applies + +If your PR touches **any** of these, run through the checklist below: + +- `packages/react-on-rails-pro-node-renderer/src/**` +- `packages/react-on-rails-pro-node-renderer/package.json` (especially `protocolVersion`, + `exports`, or runtime dependencies) +- Any worker pool, JWT auth, integrations (Sentry/Honeybadger), or VM-context code in the + renderer + +## Pre-flight: Toolchain + +Ruby 3.3.x is the documented baseline for the Pro dummy app, and this workflow +has also been verified on Ruby 3.4.8. On Ruby 3.5+, the dummy's Gemfile pulls in +`ostruct`, `logger`, and `benchmark` as explicit gems for stdlib compatibility. +If a newer Ruby hits unrelated incompatibilities, fall back to the documented +Ruby version. + +```bash +cd react_on_rails_pro/spec/dummy +bundle install + +# Optional fallback if your local Ruby fails: +mise shell ruby@3.3.7 # or rbenv/asdf equivalent +bundle install +``` + +## Step 1: Rebuild the Renderer Package + +Pick **one** of these depending on how you plan to iterate: + +**One-shot rebuild (recommended for a single validation pass):** + +```bash +pnpm --filter react-on-rails-pro-node-renderer run build +``` + +**Or use the dummy's convenience script:** + +```bash +cd react_on_rails_pro/spec/dummy +pnpm run node-renderer:fresh # builds, then starts the renderer standalone +``` + +This starts the renderer in the foreground on port 3800. For a full-stack dummy +run, either stop it before Step 2 and let `bin/dev` start the renderer, or leave +it running and comment out the `node-renderer:` line in `Procfile.dev` before +running `bin/dev`. + +**Watch mode (recommended when iterating on the renderer source):** + +Either uncomment the `node-renderer-build` line in `react_on_rails_pro/spec/dummy/Procfile.dev`, +or in a separate terminal run: + +```bash +pnpm --filter react-on-rails-pro-node-renderer run build-watch +``` + +> If you skip this step, the dummy app will silently keep using the previous lib build. +> Symptoms: a fix you just applied does not change behavior, or a regression you expected +> to see does not appear. + +## Step 2: Start the Dummy App + +```bash +cd react_on_rails_pro/spec/dummy +bin/dev +``` + +Verify: + +- [ ] `bin/dev` starts without `overlay.sockPort should be a number` (webpack-dev-server) +- [ ] `bin/dev` starts without `cannot load such file -- ostruct` (Rails precompile) +- [ ] All Procfile.dev processes are healthy after 30 seconds +- [ ] The `node-renderer` process logs that it bound to port 3800 + +## Step 3: Exercise the SSR Endpoints + +For PRs touching streaming, hydration, RSC, or VM-context code, hit the routes that +actually exercise the renderer (not just static pages): + +- [ ] `http://localhost:3000/stream_native_metadata` — renders without `ReferenceError` + (e.g. `performance is not defined`) in the renderer logs +- [ ] `http://localhost:3000/hybrid_metadata_streaming` — same +- [ ] Any route specifically related to your change + +For each route: + +- [ ] Page returns 200 and renders SSR content (view-source shows component markup) +- [ ] No errors in the `node-renderer` Procfile pane +- [ ] No errors in the Rails server log +- [ ] No errors in the browser console + +## Step 4: Confirm You Tested the New Code + +It is easy to validate stale lib output without realizing it. Confirm: + +- [ ] The built `lib/` file corresponding to your edit is newer than the `src/` + file you changed. Compare the specific files with `stat` or use a + per-file freshness check, for example: + `find packages/react-on-rails-pro-node-renderer/lib -newer packages/react-on-rails-pro-node-renderer/src/.ts | head` +- [ ] If you used watch mode, you saw a rebuild line in the watcher output after your + most recent edit +- [ ] Restart the `node-renderer` Procfile process after the rebuild — `node` does not + hot-reload required modules + +## Reporting Results in the PR + +```markdown +## Node Renderer Validation + +- [x] Rebuilt `react-on-rails-pro-node-renderer` package +- [x] Verified the rebuilt `lib/` file is newer than the changed `src/` file +- [x] `bin/dev` starts cleanly +- [x] `/stream_native_metadata` renders without errors +- [x] `/hybrid_metadata_streaming` renders without errors +- [x] No errors in node-renderer logs +``` + +If you cannot validate manually (e.g. no local Ruby toolchain), say so explicitly and +note that the change is type-checked / unit-tested only. diff --git a/CLAUDE.md b/CLAUDE.md index 0a02d35bbc..8fd52e6e34 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,5 +45,6 @@ Use these docs for Claude-oriented operational guidance: - `.claude/docs/docs-competitive-landscape.md` - `.claude/docs/docs-templates.md` - `.claude/docs/manual-dev-environment-testing.md` +- `.claude/docs/validating-node-renderer-changes.md` For Pro-package specifics, also read `react_on_rails_pro/CLAUDE.md`. diff --git a/docs/oss/building-features/node-renderer/debugging.md b/docs/oss/building-features/node-renderer/debugging.md index 956b2fe0e7..7aa34e7478 100644 --- a/docs/oss/building-features/node-renderer/debugging.md +++ b/docs/oss/building-features/node-renderer/debugging.md @@ -42,7 +42,7 @@ Use this when you need full control over the renderer process — different flag 1. If you want to attach a debugger instead, run: ```bash cd react_on_rails_pro/spec/dummy - pnpm run node-renderer-debug + pnpm run node-renderer:debug ``` 1. Reload the page that triggers the SSR issue and reproduce the problem. 1. If you change Ruby code in loaded gems, restart the Rails server. diff --git a/react_on_rails_pro/CLAUDE.md b/react_on_rails_pro/CLAUDE.md index 43138dc1e7..cf94b1c77c 100644 --- a/react_on_rails_pro/CLAUDE.md +++ b/react_on_rails_pro/CLAUDE.md @@ -58,6 +58,16 @@ The node renderer is a standalone Fastify HTTP server (separate Node.js process) - Integrations: Sentry, Honeybadger (optional peer deps) - Protocol versioning: `protocolVersion` in package.json must match gem expectations +**Validating source changes against the dummy app:** the dummy consumes the _built_ +`packages/react-on-rails-pro-node-renderer/lib/**`, so edits under `src/**` are not +picked up until the package is rebuilt. Use one of: + +- `pnpm --filter react-on-rails-pro-node-renderer run build` (one-shot) +- `cd react_on_rails_pro/spec/dummy && pnpm run node-renderer:fresh` (rebuild + start) +- `pnpm --filter react-on-rails-pro-node-renderer run build-watch` (watch in another shell) + +See `.claude/docs/validating-node-renderer-changes.md` for the full checklist. + ### Yalc Dependency Chain Pro dummy's preinstall builds and links packages in this order: diff --git a/react_on_rails_pro/Gemfile.development_dependencies b/react_on_rails_pro/Gemfile.development_dependencies index 4ac4ea7975..f0adf0b122 100644 --- a/react_on_rails_pro/Gemfile.development_dependencies +++ b/react_on_rails_pro/Gemfile.development_dependencies @@ -54,6 +54,12 @@ group :development, :test do gem "rbs", require: false gem "scss_lint", require: false gem "fakefs", require: "fakefs/safe" + + # Added for Ruby 3.5+ compatibility to silence warnings. + # jbuilder and other gems lazy-require these stdlib bits that became default gems in 3.5. + gem "benchmark", require: false + gem "logger", require: false + gem "ostruct", require: false end group :test do diff --git a/react_on_rails_pro/Gemfile.lock b/react_on_rails_pro/Gemfile.lock index 9e82af50a0..6ae6175ad5 100644 --- a/react_on_rails_pro/Gemfile.lock +++ b/react_on_rails_pro/Gemfile.lock @@ -247,6 +247,7 @@ GEM racc (~> 1.4) nokogiri (1.19.2-x86_64-linux-gnu) racc (~> 1.4) + ostruct (0.6.3) package_json (0.2.0) parallel (1.27.0) parser (3.3.10.0) @@ -475,6 +476,7 @@ PLATFORMS DEPENDENCIES amazing_print + benchmark bootsnap bundler capybara (>= 3.38.0) @@ -490,9 +492,11 @@ DEPENDENCIES jquery-rails launchy listen + logger net-http net-imap net-smtp + ostruct pg pry (>= 0.14.1) pry-byebug! diff --git a/react_on_rails_pro/spec/dummy/Gemfile.lock b/react_on_rails_pro/spec/dummy/Gemfile.lock index ee328c6939..7ac1251b65 100644 --- a/react_on_rails_pro/spec/dummy/Gemfile.lock +++ b/react_on_rails_pro/spec/dummy/Gemfile.lock @@ -271,6 +271,7 @@ GEM racc (~> 1.4) nokogiri (1.19.2-x86_64-linux-musl) racc (~> 1.4) + ostruct (0.6.3) package_json (0.2.0) parallel (1.27.0) parser (3.3.10.0) @@ -524,6 +525,7 @@ PLATFORMS DEPENDENCIES amazing_print + benchmark bootsnap capybara (>= 3.38.0) capybara-screenshot @@ -538,9 +540,11 @@ DEPENDENCIES jquery-rails launchy listen + logger net-http net-imap net-smtp + ostruct pg prism-rails pry (>= 0.14.1) diff --git a/react_on_rails_pro/spec/dummy/Procfile.dev b/react_on_rails_pro/spec/dummy/Procfile.dev index d6aebbda2d..b4c335c5fd 100644 --- a/react_on_rails_pro/spec/dummy/Procfile.dev +++ b/react_on_rails_pro/spec/dummy/Procfile.dev @@ -11,4 +11,11 @@ rails-server-assets: HMR=true SERVER_BUNDLE_ONLY=true bin/shakapacker --watch rails-rsc-assets: HMR=true RSC_BUNDLE_ONLY=true bin/shakapacker --watch # Start Node server for server rendering. -node-renderer: RENDERER_LOG_LEVEL=debug RENDERER_PORT=3800 node --inspect renderer/node-renderer.js +# NOTE: This consumes the *built* `packages/react-on-rails-pro-node-renderer/lib/**`. +# When validating source changes under `packages/react-on-rails-pro-node-renderer/src/**`, +# either rebuild once (`pnpm --filter react-on-rails-pro-node-renderer run build`) or +# uncomment the `node-renderer-build` line below to watch and rebuild on every save. +# Foreman/overmind starts processes concurrently, so on first use wait for the initial +# build-watch compile or restart `node-renderer` before trusting renderer output. +# node-renderer-build: pnpm --filter react-on-rails-pro-node-renderer run build-watch +node-renderer: RENDERER_PORT=3800 node renderer/node-renderer.js diff --git a/react_on_rails_pro/spec/dummy/config/webpack/development.js b/react_on_rails_pro/spec/dummy/config/webpack/development.js index 264d054f5c..46157470e8 100644 --- a/react_on_rails_pro/spec/dummy/config/webpack/development.js +++ b/react_on_rails_pro/spec/dummy/config/webpack/development.js @@ -11,7 +11,11 @@ const developmentEnvOnly = (clientWebpackConfig, _serverWebpackConfig) => { clientWebpackConfig.plugins.push( new ReactRefreshWebpackPlugin({ overlay: { - sockPort: devServer.port, + // bin/dev sets SHAKAPACKER_DEV_SERVER_PORT as a string, which Shakapacker + // surfaces unchanged on devServer.port. The plugin schema requires a number. + // `|| 3035` falls back to Shakapacker's default if devServer.port is missing, + // so a misconfiguration surfaces as a wrong port rather than silent NaN. + sockPort: parseInt(devServer.port, 10) || 3035, }, }), ); diff --git a/react_on_rails_pro/spec/dummy/package.json b/react_on_rails_pro/spec/dummy/package.json index 33095c7258..1f1ecfd4e5 100644 --- a/react_on_rails_pro/spec/dummy/package.json +++ b/react_on_rails_pro/spec/dummy/package.json @@ -105,8 +105,10 @@ "build:client": "RAILS_ENV=production NODE_ENV=production bin/shakapacker", "build:server": "RAILS_ENV=production NODE_ENV=production SERVER=true bin/shakapacker", "build:clean": "rm -rf public/webpack && rm -rf ssr-generated || true", - "node-renderer-debug": "RENDERER_PORT=3800 ndb renderer/node-renderer.js", - "node-renderer": "RENDERER_PORT=3800 node renderer/node-renderer.js" + "node-renderer:debug": "RENDERER_LOG_LEVEL=debug RENDERER_PORT=3800 node --inspect renderer/node-renderer.js", + "node-renderer": "RENDERER_PORT=3800 node renderer/node-renderer.js", + "node-renderer:fresh": "pnpm --filter react-on-rails-pro-node-renderer run build && RENDERER_LOG_LEVEL=debug RENDERER_PORT=3800 node renderer/node-renderer.js", + "node-renderer:build-watch": "pnpm --filter react-on-rails-pro-node-renderer run build-watch" }, "license": "UNLICENSED", "repository": { diff --git a/react_on_rails_pro/spec/execjs-compatible-dummy/config/webpack/development.js b/react_on_rails_pro/spec/execjs-compatible-dummy/config/webpack/development.js index b44902d015..5af6e3d727 100644 --- a/react_on_rails_pro/spec/execjs-compatible-dummy/config/webpack/development.js +++ b/react_on_rails_pro/spec/execjs-compatible-dummy/config/webpack/development.js @@ -16,7 +16,11 @@ const developmentEnvOnly = (clientWebpackConfig, _serverWebpackConfig) => { clientWebpackConfig.plugins.push( new ReactRefreshWebpackPlugin({ overlay: { - sockPort: devServer.port, + // bin/dev sets SHAKAPACKER_DEV_SERVER_PORT as a string, which Shakapacker + // surfaces unchanged on devServer.port. The plugin schema requires a number. + // `|| 3035` falls back to Shakapacker's default if devServer.port is missing, + // so a misconfiguration surfaces as a wrong port rather than silent NaN. + sockPort: parseInt(devServer.port, 10) || 3035, }, }), );