feat: align renderer cache staging with bundle-hash layout#3124
feat: align renderer cache staging with bundle-hash layout#3124
Conversation
Adds `react_on_rails_pro:pre_seed_renderer_cache` rake task that copies compiled server bundles into the Node Renderer's cache directory structure during Docker image builds. This eliminates the 410→retry cold-start latency (200ms–1s+) on the first SSR request after deployment. Unlike the existing `pre_stage_bundle_for_node_renderer` (which creates symlinks for local/CI use), this task copies files into the subdirectory structure the renderer expects: <cache>/<bundleHash>/<bundleHash>.js Supports RENDERER_SERVER_BUNDLE_CACHE_PATH env var, RSC bundles, and assets_to_copy configuration. Closes #3122 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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:
WalkthroughAdds a Docker/CI-oriented rake task and supporting utilities to pre-seed the React On Rails Pro Node Renderer on-disk bundle cache by copying compiled server (and optional RSC) bundles plus configured assets into per-hash cache directories; includes cache-path resolution with a deprecated env-var fallback, docs, and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Builder as Docker Build / CI
participant Image as Docker Image FS
participant Container as App Container (startup)
participant Renderer as Node Renderer
participant Rails as Rails App
Builder->>Image: copy compiled server bundle & assets into\n<cache_dir>/<bundle_hash>/<bundle_hash>.js
Note right of Image: optionally copy previous bundle hash from artifact store
Image->>Container: container created with pre-seeded cache
Container->>Renderer: renderer reads cache dir (<cache_dir>/<bundle_hash>/...)
Renderer-->>Container: bundle available (no 410)
Rails->>Renderer: SSR request referencing bundle_hash
Renderer-->>Rails: rendered HTML (fast)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 553d4c5ef5
ℹ️ 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".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review SummaryOverall the approach is clean and well-motivated — copying bundles at image-build time to avoid the 410→retry cold-start round-trip is a solid pattern. A few issues to address: Must fix
Should fix
Nice to have
|
Greptile SummaryAdds the Confidence Score: 5/5Safe to merge — the implementation is correct, consistent with existing conventions, and all P2 findings are minor style/documentation improvements. No P0 or P1 issues found. The three findings are all P2: a wrong PR link in the changelog, a missing explicit CHANGELOG.md (wrong PR number), react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb (missing require), react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb (no RSC test) Important Files Changed
Sequence DiagramsequenceDiagram
participant D as Docker Build
participant Rake as rake pre_seed_renderer_cache
participant PSR as PreSeedRendererCache
participant NRP as NodeRenderingPool
participant FS as Filesystem
D->>Rake: bundle exec rake react_on_rails_pro:pre_seed_renderer_cache
Rake->>PSR: PreSeedRendererCache.call
PSR->>PSR: resolve_cache_dir (ENV or Rails.root default)
PSR->>NRP: server_bundle_hash
NRP-->>PSR: "abc123.js"
PSR->>FS: mkdir_p cache/abc123.js/
PSR->>FS: cp server-bundle.js → cache/abc123.js/abc123.js.js
PSR->>PSR: collect_assets (assets_to_copy + RSC manifests if RSC enabled)
PSR->>FS: cp each asset → cache/abc123.js/
alt RSC support enabled
PSR->>NRP: rsc_bundle_hash
NRP-->>PSR: "rsc-xyz789.js"
PSR->>FS: mkdir_p cache/rsc-xyz789.js/
PSR->>FS: cp rsc-bundle.js → cache/rsc-xyz789.js/rsc-xyz789.js.js
PSR->>FS: cp each asset → cache/rsc-xyz789.js/
end
D->>D: Container starts — Node Renderer finds pre-seeded cache on startup
Reviews (1): Last reviewed commit: "feat: add pre_seed_renderer_cache rake t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Fix wrong PR number in CHANGELOG (3123→3124) - Guard missing bundle before calling server_bundle_hash to prevent Errno::ENOENT from masking the user-friendly error message - Add require "fileutils" for explicit dependency - Add RENDERER_BUNDLE_PATH deprecation warning to PrepareNodeRenderBundles for consistency, and support RENDERER_SERVER_BUNDLE_CACHE_PATH there too - Add RSC test coverage (enable_rsc_support: true) verifying both server and RSC bundles plus manifest assets land in correct subdirectories Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Review Feedback Addressed (e18004c)All review comments have been addressed and threads resolved: Fixed (5 items):
Declined (1 item):
All 8 tests pass locally. Rationale replies posted on duplicate/declined items. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Review:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e18004c70a
ℹ️ 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".
… warning Moves the renderer cache directory resolution logic (env var priority + deprecation warning) into Utils.resolve_renderer_cache_dir. Both PrepareNodeRenderBundles and PreSeedRendererCache now delegate to it, eliminating duplication and fixing the bug where resolve_dest_path warned about RENDERER_BUNDLE_PATH even when RENDERER_SERVER_BUNDLE_CACHE_PATH was set and being used. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review Feedback Addressed (ab3262f)Fixed (2 items):
All 8 pre_seed_renderer_cache specs pass. All review threads resolved. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab3262fdf4
ℹ️ 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".
…cache Add test verifying preferred env var wins when both RENDERER_SERVER_BUNDLE_CACHE_PATH and RENDERER_BUNDLE_PATH are set, with no spurious deprecation warning. Fix after block to clean up server_bundle_path test artifact. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review Feedback Addressed (9f79213)Fixed (2 items):
Skipped with rationale (3 items):
All 9 tests pass. All review threads replied to and resolved. |
ReviewClean, well-motivated feature — pre-seeding the renderer cache at image-build time to eliminate 410→retry cold-start latency is a solid pattern. RSC support, the deprecation path, and the test coverage (including the RSC-enabled paths) all look good. Four things to address before merge: Should fix
Nice to have
|
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 `@react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb`:
- Around line 3-4: The file uses FileUtils (see extend FileUtils and subsequent
FileUtils method calls in PrepareNodeRendererBundles) but never requires it; add
require "fileutils" at the top of
react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb so
the FileUtils constant is always available before using extend FileUtils and any
FileUtils.* calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 342b7d49-a244-4346-92ef-ae50c7cffe97
📒 Files selected for processing (4)
react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rbreact_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rbreact_on_rails_pro/lib/react_on_rails_pro/utils.rbreact_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb
- react_on_rails_pro/lib/react_on_rails_pro/utils.rb
…ion guard Addresses review feedback on PR #3124: - Add missing `require "fileutils"` to prepare_node_renderer_bundles.rb so FileUtils is never resolved via implicit loading. - Extract duplicated helpers (collect_assets, required_rsc_asset_paths, validate_bundle_exists!, bundle_sources) into RendererCacheHelpers. PreSeedRendererCache and PrepareNodeRenderBundles now delegate there, keeping only their copy-vs-symlink distinction local. - bundle_sources now resolves hash values at construction time so the caller iterates (path, hash) pairs directly — no more indirect public_send on method-name symbols. - Wrap the RENDERER_BUNDLE_PATH deprecation guard in a Mutex so the one-time warning holds under multi-threaded callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review feedback addressed — 89d1d52Thanks everyone for the thorough reviews. I (Claude Code) worked through all 16 unresolved threads — below is the per-category summary. Fixed in 89d1d52
While refactoring I also fully hoisted Already addressed in earlier commits on this branchTen threads were flagged against older commits but the code had already moved on:
All 16 threads have been replied to inline and resolved. Green on:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me take a look at the new 🏁 Script executed: Length of output: 18151
Thread-safe deprecation guard —
Copy-vs-symlink as the only leaf distinction — The shared 18 + 31 examples all green with RuboCop clean — happy to approve. 🐇 [approve] 🧠 Learnings used |
Code ReviewOverall this is well-structured work — clear separation of copy vs symlink semantics, good shared helper extraction, solid test coverage across the new scenarios, and the deprecation-warning guard is correctly thread-safe. A few things worth addressing before merge: Bugs / Correctness
The class-level
Both Minor Code Quality
def collect_assets
assets = Array(ReactOnRailsPro.configuration.assets_to_copy).dup
if ReactOnRailsPro.configuration.enable_rsc_supportCache the configuration object in a local variable to avoid the double lookup and make it obvious both accesses read the same snapshot.
The method is documented as test-only ( Missing Test Coverage
Docs / ChangelogBoth are clear and accurate. The rolling-deploy "seed current and previous bundle hashes" guidance is a useful addition that wasn't there before. |
- Move validate_bundle_exists! inside RendererCacheHelpers.bundle_sources so the guard runs before pool.server_bundle_hash / pool.rsc_bundle_hash are invoked. Hash computation eventually calls Digest::MD5.file / File.mtime on the bundle path, which raises raw Errno::ENOENT if the file is missing — bypassing the friendly ReactOnRailsPro::Error. Drop the now-redundant validate_bundle_exists! calls in both callers. Add a regression spec that un-stubs server_bundle_hash so the real hash-computation path runs on a missing bundle and asserts ReactOnRailsPro::Error (not Errno::ENOENT). - Expand asset paths against Rails.root rather than Dir.pwd in both required_rsc_asset_paths and the per-asset lookups in copy_assets / symlink_assets. This keeps the Set comparison consistent when the rake task runs from a different cwd (e.g. inside a Docker RUN step). - Snapshot ReactOnRailsPro.configuration once at the top of collect_assets so both reads see the same value and the method makes one dispatch. - Remove dead `extend FileUtils` in PrepareNodeRenderBundles (no longer used after the refactor to explicit FileUtils. prefixes). - Add symmetric RENDERER_BUNDLE_PATH deprecation spec in prepare_node_renderer_bundles_spec.rb. All 20 dummy specs pass; rubocop clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addressed review feedback in 3a4bcc5Fixed (6 items):
Declined with rationale (1 item):
All 20 dummy specs pass; rubocop clean. I (Claude Code) addressed the feedback on behalf of @justin808. |
|
Code Review Overview: Clean implementation. RendererCacheHelpers extraction, thread-safe one-time deprecation warning, and pre-validation of bundle paths before hash-computation are all good design decisions. Test coverage is thorough. Issue 1 - Duplicated asset validation logic: copy_assets in PreSeedRendererCache and symlink_assets in PrepareNodeRenderBundles are structurally identical (expand path, check existence, test rsc_required_paths, raise/warn, then perform the file operation). Only the final step differs. Since RendererCacheHelpers was introduced to eliminate this duplication, the per-asset validation belongs there too. Issue 2 - Non-atomic copy in seed_bundle: FileUtils.cp writes directly to the destination. If killed mid-copy (OOM during Docker build), the cache is left with a partial bundleHash.js. A cp-to-tmp-then-mv pattern gives atomic promotion. Issue 3 - Node Renderer binary compatibility: The Ruby side now prefers RENDERER_SERVER_BUNDLE_CACHE_PATH and deprecates RENDERER_BUNDLE_PATH, but the updated docs show starting the renderer binary with the new var. Has the Node Renderer binary been updated to read RENDERER_SERVER_BUNDLE_CACHE_PATH? If not, users who follow the updated docs will configure the Ruby rake task correctly but the renderer falls back to its own default, making the pre-seeded cache unreachable. Should be called out in migration notes if this requires a renderer version bump. Issue 4 - compact_blank may silently discard valid paths: compact_blank (ActiveSupport) removes nil and blank? values including empty strings. compact would be safer — it only removes nil, and an empty/whitespace path would surface downstream as a warning rather than disappearing silently. Minor: resolve_cache_dir in PreSeedRendererCache is a one-line private wrapper around Utils.resolve_renderer_cache_dir — inlining removes unnecessary indirection. Also: make_relative_symlink was previously public; confirming no external consumers (e.g. host-app custom asset tasks) depend on it would be worth a quick grep. |
…l(mode:) Collapse the copy-based PreSeedRendererCache and the symlink-based PrepareNodeRenderBundles into a single entry point with a `mode:` keyword argument. Both modes produce the identical <cache>/<hash>/<hash>.js layout; only the file operation differs (FileUtils.cp vs relative symlink). Changes: - ReactOnRailsPro::PreSeedRendererCache.call now accepts mode: :copy (default, for Docker/image builds) or mode: :symlink (same-filesystem workflows). Rejects unknown modes with ArgumentError. - mode: :copy now raises a clear error when neither RENDERER_SERVER_BUNDLE_CACHE_PATH nor RENDERER_BUNDLE_PATH is set in non-dev/test environments. The Node renderer's default lookup can differ from the Ruby side (falling back to /tmp when cwd is outside the app tree), so silent fallback is unsafe for production-like deploys and was a silent-misconfig footgun. - `react_on_rails_pro:pre_seed_renderer_cache` rake task accepts MODE=copy (default) or MODE=symlink. - ReactOnRailsPro::PrepareNodeRenderBundles is now a thin deprecated shim that emits a once-per-process warning and delegates to PreSeedRendererCache.call(mode: :symlink). Public class and the `pre_stage_bundle_for_node_renderer` rake task remain for backward compatibility. - AssetsPrecompile.call invokes the unified API with mode: :symlink after precompile (no behavior change for existing users). - react_on_rails:doctor scans common deploy-script locations (Procfile*, Dockerfile, bin/*) for references to the deprecated pre_stage_bundle_for_node_renderer task and surfaces a migration warning. - Docs: docs/pro/node-renderer.md describes the unified API, both modes, the non-dev env-var requirement for copy mode, and the deprecation story. - CHANGELOG: new Changed entry describing the unification. - Tests: mode validation, raise-in-non-dev guard (including symlink opt-out), deprecation warning on the shim, and a doctor check test. Context: stacked on PR #3124 (jg/3122-preseed-renderer-cache). Refs: #3122 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion guard Addresses review feedback on PR #3124: - Add missing `require "fileutils"` to prepare_node_renderer_bundles.rb so FileUtils is never resolved via implicit loading. - Extract duplicated helpers (collect_assets, required_rsc_asset_paths, validate_bundle_exists!, bundle_sources) into RendererCacheHelpers. PreSeedRendererCache and PrepareNodeRenderBundles now delegate there, keeping only their copy-vs-symlink distinction local. - bundle_sources now resolves hash values at construction time so the caller iterates (path, hash) pairs directly — no more indirect public_send on method-name symbols. - Wrap the RENDERER_BUNDLE_PATH deprecation guard in a Mutex so the one-time warning holds under multi-threaded callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…l(mode:) Collapse the copy-based PreSeedRendererCache and the symlink-based PrepareNodeRenderBundles into a single entry point with a `mode:` keyword argument. Both modes produce the identical <cache>/<hash>/<hash>.js layout; only the file operation differs (FileUtils.cp vs relative symlink). Changes: - ReactOnRailsPro::PreSeedRendererCache.call now accepts mode: :copy (default, for Docker/image builds) or mode: :symlink (same-filesystem workflows). Rejects unknown modes with ArgumentError. - mode: :copy now raises a clear error when neither RENDERER_SERVER_BUNDLE_CACHE_PATH nor RENDERER_BUNDLE_PATH is set in non-dev/test environments. The Node renderer's default lookup can differ from the Ruby side (falling back to /tmp when cwd is outside the app tree), so silent fallback is unsafe for production-like deploys and was a silent-misconfig footgun. - `react_on_rails_pro:pre_seed_renderer_cache` rake task accepts MODE=copy (default) or MODE=symlink. - ReactOnRailsPro::PrepareNodeRenderBundles is now a thin deprecated shim that emits a once-per-process warning and delegates to PreSeedRendererCache.call(mode: :symlink). Public class and the `pre_stage_bundle_for_node_renderer` rake task remain for backward compatibility. - AssetsPrecompile.call invokes the unified API with mode: :symlink after precompile (no behavior change for existing users). - react_on_rails:doctor scans common deploy-script locations (Procfile*, Dockerfile, bin/*) for references to the deprecated pre_stage_bundle_for_node_renderer task and surfaces a migration warning. - Docs: docs/pro/node-renderer.md describes the unified API, both modes, the non-dev env-var requirement for copy mode, and the deprecation story. - CHANGELOG: new Changed entry describing the unification. - Tests: mode validation, raise-in-non-dev guard (including symlink opt-out), deprecation warning on the shim, and a doctor check test. Context: stacked on PR #3124 (jg/3122-preseed-renderer-cache). Refs: #3122 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicated asset validation logic across two classes
- Extracted the shared asset expansion/validation/error-handling loop into RendererCacheHelpers.each_validated_asset and updated both copy and symlink flows to use it.
Or push these changes by commenting:
@cursor push 86992fcd9f
Preview (86992fcd9f)
diff --git a/react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb b/react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb
--- a/react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb
+++ b/react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb
@@ -51,19 +51,8 @@
# against Rails.root to match how RendererCacheHelpers.required_rsc_asset_paths
# builds its Set.
def self.copy_assets(assets, bundle_dir, rsc_required_paths)
- assets.each do |asset_path|
- expanded = File.expand_path(asset_path.to_s, Rails.root)
- unless File.exist?(expanded)
- if rsc_required_paths.include?(expanded)
- raise ReactOnRailsPro::Error, "Required RSC asset not found: #{asset_path}. " \
- "Build your bundles before pre-seeding the renderer cache."
- end
- warn "[ReactOnRailsPro] Asset not found #{asset_path}"
- next
- end
-
- dest = File.join(bundle_dir, File.basename(expanded))
- FileUtils.cp(expanded, dest)
+ RendererCacheHelpers.each_validated_asset(assets, bundle_dir, rsc_required_paths, "pre-seeding") do |source, dest|
+ FileUtils.cp(source, dest)
puts "[ReactOnRailsPro] Copied asset: #{dest}"
end
end
diff --git a/react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb b/react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb
--- a/react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb
+++ b/react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb
@@ -45,23 +45,9 @@
end
end
- # Required assets are matched by expanded path rather than basename so a
- # same-named unrelated entry in assets_to_copy cannot trigger a false-
- # positive "required" error. Expand against Rails.root to match how
- # RendererCacheHelpers.required_rsc_asset_paths builds its Set.
def self.symlink_assets(assets, bundle_dir, rsc_required_paths)
- assets.each do |asset_path|
- expanded = File.expand_path(asset_path.to_s, Rails.root)
- unless File.exist?(expanded)
- if rsc_required_paths.include?(expanded)
- raise ReactOnRailsPro::Error, "Required RSC asset not found: #{asset_path}. " \
- "Build your bundles before pre-staging the renderer cache."
- end
- warn "[ReactOnRailsPro] Asset not found #{asset_path}"
- next
- end
-
- destination_full_path = File.join(bundle_dir, File.basename(expanded))
+ RendererCacheHelpers.each_validated_asset(assets, bundle_dir, rsc_required_paths, "pre-staging") do |expanded,
+ destination_full_path|
make_relative_symlink(expanded, destination_full_path)
end
end
diff --git a/react_on_rails_pro/lib/react_on_rails_pro/renderer_cache_helpers.rb b/react_on_rails_pro/lib/react_on_rails_pro/renderer_cache_helpers.rb
--- a/react_on_rails_pro/lib/react_on_rails_pro/renderer_cache_helpers.rb
+++ b/react_on_rails_pro/lib/react_on_rails_pro/renderer_cache_helpers.rb
@@ -64,5 +64,22 @@
sources << [rsc_bundle_path, pool.rsc_bundle_hash]
sources
end
+
+ def each_validated_asset(assets, bundle_dir, rsc_required_paths, action_description)
+ assets.each do |asset_path|
+ expanded = File.expand_path(asset_path.to_s, Rails.root)
+ unless File.exist?(expanded)
+ if rsc_required_paths.include?(expanded)
+ raise ReactOnRailsPro::Error, "Required RSC asset not found: #{asset_path}. " \
+ "Build your bundles before #{action_description} the renderer cache."
+ end
+ warn "[ReactOnRailsPro] Asset not found #{asset_path}"
+ next
+ end
+
+ destination_full_path = File.join(bundle_dir, File.basename(expanded))
+ yield expanded, destination_full_path
+ end
+ end
end
endYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit e120a09. Configure here.
|
Addressed the remaining renderer-cache review threads on this base PR. Fixed in the pushed stack:
Also verified the docs concern: the Node renderer reads RENDERER_SERVER_BUNDLE_CACHE_PATH in packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts and the compiled lib/shared/configBuilder.js. The lower-stack fixes were propagated upward into #3167 and #3173. |
|
@claude review this PR |
1 participant
Summary
react_on_rails_pro:pre_seed_renderer_cacheas the preferred copy-based path for Docker/image builds, using the renderer's existing bundle-hash cache layout (<cache>/<bundleHash>/<bundleHash>.js)react_on_rails_pro:pre_stage_bundle_for_node_renderertask to stage that same bundle-hash layout via symlinks for same-filesystem workflows such as local development, Heroku-style deploys, and bundle-caching restoresRENDERER_SERVER_BUNDLE_CACHE_PATHas the preferred configuration knob, withRENDERER_BUNDLE_PATHdocumented as the deprecated compatibility aliasCloses #3122
Test plan
RUBYOPT='-rostruct -rrb-readline' bundle exec rspec spec/prepare_node_renderer_bundles_spec.rb spec/pre_seed_renderer_cache_spec.rbinreact_on_rails_pro/spec/dummybundle exec rubocop react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb react_on_rails_pro/lib/react_on_rails_pro/prepare_node_renderer_bundles.rb react_on_rails_pro/lib/tasks/assets.rake react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rbpnpm run lintpnpm start format.listDifferentNotes
RUBYOPT='-rostruct -rrb-readline'because the local Ruby is4.0.1and the dummy app still pulls in older pry/pry-doc expectations.bundle installin the dummy app on Ruby4.0.1tries to refreshreact_on_rails_pro/spec/dummy/Gemfile.lock; that incidental lockfile diff was intentionally restored out of this branch after validation.Note
Medium Risk
Moderate risk because it changes how Pro stages/locates Node Renderer SSR bundles (new bundle-hash directory layout, new rake task, and env-var precedence), which can affect production deploy/startup behavior if misconfigured.
Overview
Adds a new Pro rake task
react_on_rails_pro:pre_seed_renderer_cachethat copies compiled SSR (and optional RSC) bundles plus required assets into the Node Renderer’s bundle-hash cache layout during Docker/image builds to avoid first-request 410→retry latency.Updates the legacy
react_on_rails_pro:pre_stage_bundle_for_node_rendererflow to symlink into the same<cache>/<bundleHash>/<bundleHash>.jslayout, centralizes staging logic inRendererCacheHelpers(including friendlier missing-bundle/RSC-manifest errors), and introducesRENDERER_SERVER_BUNDLE_CACHE_PATHas the preferred cache directory knob withRENDERER_BUNDLE_PATHkept as a deprecated alias (one-time warning). Docs/CHANGELOG and dummy-app specs are updated accordingly.Reviewed by Cursor Bugbot for commit e0ce40e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Deprecations
Documentation
Tests