[mirror] Move Node Renderer entry point to renderer/ directory#6
Open
yashwant86 wants to merge 10 commits into
Open
[mirror] Move Node Renderer entry point to renderer/ directory#6yashwant86 wants to merge 10 commits into
yashwant86 wants to merge 10 commits into
Conversation
Establishes `renderer/node-renderer.js` as the canonical location for the Pro Node Renderer entry point. Previously the path was inconsistent across docs (`node-renderer.js`, `client/node-renderer.js`, or `react-on-rails-pro-node-renderer.js`). Using a dedicated top-level `renderer/` directory makes it trivial to exclude from production Docker builds that strip JS sources after bundling — the Node Renderer process still needs its entry file at runtime. Changes: - Generator (`pro_setup.rb`, `base_generator.rb`, `demo_page_config.rb`, USAGE, initializer template) now creates the entry at `renderer/node-renderer.js` - Template file moved to `templates/pro/base/renderer/node-renderer.js` - Pro `spec/dummy` renderer file, Procfile.dev, Procfile.prod, package.json scripts, .controlplane/rails.yml, and benchmark reference updated - All 13 docs now use `renderer/node-renderer.js` consistently - Container deployment doc explains the rationale for the dedicated directory - Generator specs updated; 13 renderer-related specs pass Existing apps are unaffected — the generator skips files that already exist, so upgrades keep their current `client/node-renderer.js`. Fixes shakacode#3073 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous link to main-branch GitHub URL fails link-check on the PR branch because the file path has changed. Relative link works across all branches and avoids the chicken-and-egg check on the renamed path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…og polish - pro_setup.rb: skip `create_node_renderer` when the legacy `client/node-renderer.js` exists, emitting a migration hint so existing Pro users don't silently end up with two renderer files after rerunning the generator. Added a spec covering the legacy-path case. - docs/oss/.../js-configuration.md: revert the testing-example link to the absolute GitHub URL so hosted docs platforms don't 404 on a relative path that leaves the docs tree. - CHANGELOG.md: split `**[Pro]**` from the entry title and add the PR attribution to match the project's changelog guidelines. - packages/.../react-on-rails-pro-node-renderer/README.md: update the quickstart to reference `renderer/node-renderer.js` so the README matches the new generator default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The absolute URL 404s on main until this PR merges, so the pre-push lychee check rejects it. Keep the relative link which resolves correctly on GitHub file browsing; hosted docs are served via GitHub, so the reviewer's concern about Docusaurus-style resolution does not apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…irectory Addresses PR shakacode#3165 review (claude[bot]): - setup_pro now captures the return value of create_node_renderer. When a legacy client/node-renderer.js is present, create_node_renderer returns true, and setup_pro skips add_pro_to_procfile. Previously, legacy-renderer users could end up with a Procfile.dev entry pointing at renderer/node-renderer.js (a file deliberately not created), leaving the renderer process unable to start. - create_node_renderer now uses Thor's empty_directory instead of FileUtils.mkdir_p, so it participates in --pretend / verbose output like the rest of the generator. - Added a Procfile.dev assertion to the legacy-renderer spec to lock in the correct behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The legacy-renderer branch of create_node_renderer returns early, which also bypasses add_pro_to_procfile's "Procfile.dev not found" warning that spells out the exact node-renderer: command. A user with client/node-renderer.js and no Procfile.dev would get no Procfile guidance at all. Echo the full command inline so it's always visible. Also adds a spec covering the case where renderer/node-renderer.js exists but Procfile.dev lacks node-renderer: — create_node_renderer returns false, add_pro_to_procfile runs, entry is appended. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- pro_setup.rb: broaden legacy-renderer migration hint to mention Procfile.prod and Docker CMD / command: alongside Procfile.dev so production configs aren't left pointing at the old client/ path. - pro_setup.rb: correct create_node_renderer @return doc — method now always returns an explicit Boolean, never nil. - renderer/node-renderer.js template: document that serverBundleCachePath resolves to <project-root>/.node-renderer-bundles. - pro_generator_spec.rb: add coverage for mid-migration state where both renderer/node-renderer.js and client/node-renderer.js exist; asserts renderer/ guard fires first and Procfile.dev gets exactly one renderer/ entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Tighten `add_pro_to_procfile` guard so it only skips when Procfile.dev already references `renderer/node-renderer.js`. Stale legacy entries (e.g. `node client/node-renderer.js`) now surface an explicit warning instead of silently skipping, and are not duplicated with a new entry. - Fix accidental `:):` punctuation in the legacy-renderer migration hint. - Add specs: stale legacy Procfile entry path (warn + no duplicate) and idempotent no-op when Procfile.dev already has the new entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Match renderer command with regex so `node ./renderer/node-renderer.js` is recognized as already present (idempotency was too strict). - When a legacy `client/node-renderer.js` is detected and Procfile.dev still launches it, surface a pointed warning pinpointing the stale line instead of leaving users to diff the generic migration hint. - Add specs for both the `./`-prefixed command and the stale legacy Procfile entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
size-limit report 📦
|
⚡ Risk Assessment —
|
| Files | Summary |
|---|---|
Node Renderer File Relocationreact_on_rails/lib/generators/react_on_rails/templates/pro/base/renderer/node-renderer.js |
Moved Node Renderer bootstrap template from client/ to renderer/ directory with updated path comment. |
Generator Logic & Migration Supportreact_on_rails/lib/generators/react_on_rails/pro_setup.rb |
Refactored create_node_renderer to target renderer/ directory, added legacy client/node-renderer.js detection with user-friendly migration hints, implemented regex-based Procfile.dev entry detection to handle new/legacy/stale renderer paths, and conditionally skip Procfile updates when legacy renderer detected. |
Test Suite Updatesreact_on_rails/spec/react_on_rails/generators/install_generator_spec.rbreact_on_rails/spec/react_on_rails/generators/pro_generator_spec.rbreact_on_rails/spec/react_on_rails/support/shared_examples/pro_generator_examples.rb |
Updated all test assertions to expect renderer/node-renderer.js path; added 10 new test contexts covering legacy detection, stale Procfile entries, ./ prefix handling, commented entries, and coexistence scenarios. |
Generator Hints & Configurationreact_on_rails/lib/generators/react_on_rails/base_generator.rbreact_on_rails/lib/generators/react_on_rails/demo_page_config.rbreact_on_rails/lib/generators/react_on_rails/pro/USAGEreact_on_rails/lib/generators/react_on_rails/templates/pro/base/config/initializers/react_on_rails_pro.rb.tt |
Updated generator hints, USAGE documentation, and initializer template to reference renderer/node-renderer.js path. |
Dummy App Configurationreact_on_rails_pro/spec/dummy/Procfile.devreact_on_rails_pro/spec/dummy/Procfile.prodreact_on_rails_pro/spec/dummy/package.jsonbenchmarks/bench-node-renderer.rb |
Updated dummy app and benchmark configuration files to use renderer/node-renderer.js path in Procfile entries and npm scripts. |
Documentation Updatesdocs/oss/api-reference/generator-details.mddocs/oss/building-features/code-splitting.mddocs/oss/building-features/node-renderer/basics.mddocs/oss/building-features/node-renderer/container-deployment.mddocs/oss/building-features/node-renderer/debugging.mddocs/oss/building-features/node-renderer/heroku.mddocs/oss/building-features/node-renderer/js-configuration.mddocs/oss/deployment/docker-deployment.mddocs/oss/migrating/rsc-preparing-app.mddocs/oss/migrating/rsc-troubleshooting.mddocs/pro/installation.mddocs/pro/js-memory-leaks.mddocs/pro/profiling-server-side-rendering-code.mdpackages/react-on-rails-pro-node-renderer/README.mdCHANGELOG.md |
Updated all documentation and changelog to reflect renderer/node-renderer.js path in code examples, command references, and configuration snippets. |
Sequence Diagram
sequenceDiagram
participant User
participant Generator
participant FileSystem
participant Procfile
User->>Generator: Run Pro setup
Generator->>FileSystem: Check renderer/node-renderer.js exists?
alt New setup
FileSystem-->>Generator: No
Generator->>FileSystem: Check legacy client/node-renderer.js?
alt Legacy exists
FileSystem-->>Generator: Yes
Generator->>User: Display migration hint
Generator->>Procfile: Skip Procfile update
else No legacy
FileSystem-->>Generator: No
Generator->>FileSystem: Create renderer/ directory
Generator->>FileSystem: Copy template to renderer/node-renderer.js
Generator->>Procfile: Add new node-renderer entry
Generator->>User: Confirm creation
end
else Already exists
FileSystem-->>Generator: Yes
Generator->>User: Skip, already present
end
alt Procfile management
Generator->>Procfile: Read Procfile.dev
Generator->>Procfile: Check for NEW_RENDERER_COMMAND_REGEX
alt Matches new pattern
Procfile-->>Generator: Found
Generator->>User: Skip, already configured
else Check legacy pattern
Procfile-->>Generator: Not found
Generator->>Procfile: Check LEGACY_RENDERER_COMMAND_REGEX
alt Matches legacy
Procfile-->>Generator: Found
Generator->>User: Warn about stale entry
else No match
Procfile-->>Generator: Not found
Generator->>Procfile: Append new entry
end
end
end
Dig Deeper With Commands
/review <file-path> <function-optional>/chat <file-path> "<question>"/roast <file-path>
Runs only when explicitly triggered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of upstream shakacode#3165 for benchmark. Do not merge.
Summary by MergeMonkey