From e97260dc38ca70d5bac7b7adec467e2c3260b6ae Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Fri, 17 Apr 2026 20:56:27 -1000 Subject: [PATCH 01/17] refactor: unify renderer cache staging under PreSeedRendererCache#call(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 //.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) --- CHANGELOG.md | 4 + docs/pro/node-renderer.md | 21 +++- react_on_rails/lib/react_on_rails/doctor.rb | 37 ++++++ .../spec/lib/react_on_rails/doctor_spec.rb | 37 ++++++ .../react_on_rails_pro/assets_precompile.rb | 5 +- .../pre_seed_renderer_cache.rb | 115 ++++++++++++++---- .../prepare_node_renderer_bundles.rb | 76 +++--------- react_on_rails_pro/lib/tasks/assets.rake | 19 ++- .../spec/pre_seed_renderer_cache_spec.rb | 39 ++++++ .../prepare_node_renderer_bundles_spec.rb | 9 +- 10 files changed, 266 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1a5044032..7ec251a743 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ - **[Pro]** **Pre-seed renderer cache for Docker builds**: New `react_on_rails_pro:pre_seed_renderer_cache` rake task copies compiled server bundles into the Node Renderer's bundle-hash cache directory structure during Docker image builds, eliminating the 410→retry cold-start latency (200ms–1s+) on the first SSR request after deployment. Supports `RENDERER_SERVER_BUNDLE_CACHE_PATH`, RSC bundles, and rolling-deploy guidance centered on current and previous bundle hashes. The legacy `pre_stage_bundle_for_node_renderer` task now stages the same cache layout via symlinks for same-filesystem workflows. **Note:** `RENDERER_BUNDLE_PATH` is now deprecated in favor of `RENDERER_SERVER_BUNDLE_CACHE_PATH` across both tasks. Existing users with `RENDERER_BUNDLE_PATH` set will see a deprecation warning on stderr. [PR 3124](https://github.com/shakacode/react_on_rails/pull/3124) by [justin808](https://github.com/justin808). +#### Changed + +- **[Pro]** **Unified renderer cache staging**: `ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)` is now the single entry point for staging the Node Renderer cache. Both modes produce the same `//.js` layout. The `react_on_rails_pro:pre_seed_renderer_cache` rake task accepts `MODE=copy` (default; Docker/image builds) or `MODE=symlink` (same-filesystem). `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, because the Node renderer's default lookup can differ from the Ruby side and would silently drop pre-seeded bundles in the wrong directory. The legacy `react_on_rails_pro:pre_stage_bundle_for_node_renderer` task and `ReactOnRailsPro::PrepareNodeRenderBundles` class remain as deprecated shims that emit a once-per-process warning and delegate to `mode: :symlink`. `react_on_rails:doctor` flags deploy scripts that still reference the deprecated task. + #### Fixed - **Doctor accepts TypeScript server bundle entrypoints**: `react_on_rails:doctor` now resolves common source entrypoint suffixes (`.js`, `.jsx`, `.ts`, `.tsx`, `.mjs`, `.cjs`) before warning that the server bundle is missing, preventing false positives when apps use `server-bundle.ts`. [PR 3111](https://github.com/shakacode/react_on_rails/pull/3111) by [justin808](https://github.com/justin808). diff --git a/docs/pro/node-renderer.md b/docs/pro/node-renderer.md index 8219a590b1..4deeca36cc 100644 --- a/docs/pro/node-renderer.md +++ b/docs/pro/node-renderer.md @@ -119,16 +119,25 @@ When a new container starts, the Node Renderer has an empty bundle cache. The fi ### Pre-seeding the bundle cache -The `pre_seed_renderer_cache` rake task copies compiled server bundles directly into the renderer's cache directory during your Docker build, so the renderer finds them immediately on startup: +The `pre_seed_renderer_cache` rake task stages compiled server bundles directly into the renderer's cache directory, so the renderer finds them immediately on startup. + +It supports two modes, both producing the same on-disk cache layout (`//.js`): + +- **`MODE=copy`** (default) — copies files. Use in Docker/image builds so the cache is baked into an immutable artifact. +- **`MODE=symlink`** — creates relative symlinks. For same-filesystem workflows (local dev, CI, Heroku-style same-dyno deploys, bundle-caching restores). ```dockerfile -# After webpack/assets build step +# After webpack/assets build step (Docker image build) +ENV RENDERER_SERVER_BUNDLE_CACHE_PATH=/app/.node-renderer-bundles RUN bundle exec rake react_on_rails_pro:pre_seed_renderer_cache ``` -This copies the bundle into the renderer's expected directory structure (`//.js`), including any configured `assets_to_copy` and RSC bundles when RSC support is enabled. +Both modes stage the server bundle, any configured `assets_to_copy`, and (when RSC is enabled) the RSC bundle and its companion manifests. -This is the preferred path for Docker and other image-build workflows. React on Rails Pro has long supported runtime bundle uploads and the older `react_on_rails_pro:pre_stage_bundle_for_node_renderer` task for same-filesystem deployments; `pre_seed_renderer_cache` is the copy-based variant that fits immutable artifacts while using the same bundle-hash cache layout. +The `pre_seed_renderer_cache` task is also invoked automatically at the end of `assets:precompile` with `MODE=symlink`, so the local/CI/Heroku path has zero new configuration. + +> [!NOTE] +> The older `react_on_rails_pro:pre_stage_bundle_for_node_renderer` rake task and `ReactOnRailsPro::PrepareNodeRenderBundles` class are deprecated in favor of the unified API. Both remain available as thin shims that emit a deprecation warning and delegate to `MODE=symlink`. `react_on_rails:doctor` flags deploy scripts that still reference the deprecated task. ### Configuration @@ -136,9 +145,9 @@ The task follows the same environment-variable precedence as the Node Renderer, 1. `RENDERER_SERVER_BUNDLE_CACHE_PATH` environment variable (preferred) 2. `RENDERER_BUNDLE_PATH` environment variable (deprecated — emits a warning) -3. `Rails.root.join(".node-renderer-bundles")` (Rails-side default when env vars are unset) +3. `Rails.root.join(".node-renderer-bundles")` (Rails-side default when env vars are unset, only accepted for `MODE=symlink` and in dev/test) -Set `RENDERER_SERVER_BUNDLE_CACHE_PATH` in your Dockerfile to match the renderer's configuration: +In **`MODE=copy`** (Docker image builds) the task requires one of the env vars above to be set in non-dev/test environments. Because the Node renderer's own default can differ (e.g., falling back to `/tmp/react-on-rails-pro-node-renderer-bundles` when its `cwd` sits outside the app tree), relying on the silent fallback risks pre-seeded bundles landing in a directory the renderer never reads. The task raises a clear error if the env var is missing: ```dockerfile ENV RENDERER_SERVER_BUNDLE_CACHE_PATH=/app/.node-renderer-bundles diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 810555bafe..2e8dba15ee 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2694,6 +2694,7 @@ def check_pro_setup ensure_rails_environment_loaded check_pro_renderer_mode check_base_package_imports + check_deprecated_renderer_cache_task end def check_pro_initializer_existence @@ -2723,6 +2724,42 @@ def check_pro_renderer_mode checker.add_warning("⚠️ Could not detect Pro renderer mode: #{e.message}") end + # Scan common deploy-script locations for references to the deprecated + # pre_stage_bundle_for_node_renderer rake task, so users on older Procfile/ + # Dockerfile entries get a clear migration nudge before the task is removed. + DEPRECATED_RENDERER_CACHE_TASK = "pre_stage_bundle_for_node_renderer" + RENDERER_CACHE_DEPLOY_SCRIPT_PATHS = [ + "Procfile", + "Procfile.dev", + "Procfile.dev-static-assets", + "Procfile.production", + "Dockerfile", + "bin/deploy", + "bin/release", + "bin/docker-entrypoint" + ].freeze + + def check_deprecated_renderer_cache_task + matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| + File.exist?(path) && File.read(path).include?(DEPRECATED_RENDERER_CACHE_TASK) + end + + return if matches.empty? + + checker.add_warning(<<~MSG.strip) + ⚠️ Deprecated rake task '#{DEPRECATED_RENDERER_CACHE_TASK}' referenced in: + #{matches.map { |p| " • #{p}" }.join("\n")} + + Replace with: + rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink + + The unified 'pre_seed_renderer_cache' task uses MODE=copy by default (for + Docker/image builds) and MODE=symlink for same-filesystem workflows. + MSG + rescue StandardError => e + checker.add_warning("⚠️ Could not scan for deprecated renderer-cache task references: #{e.message}") + end + # The base 'react-on-rails' npm package is a transitive dependency of 'react-on-rails-pro', # so `import ... from 'react-on-rails'` resolves silently — loading the base package instead # of Pro. Components registered through the base package won't have Pro features (streaming, diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 409881f772..20288d388b 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2396,6 +2396,43 @@ class << self end end + describe "check_deprecated_renderer_cache_task" do + let(:doctor) { described_class.new(verbose: false, fix: false) } + let(:checker) { doctor.instance_variable_get(:@checker) } + + context "when a Procfile references the deprecated task" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + File.write( + "Procfile", + "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer && bundle exec puma\n" + ) + example.run + end + end + end + + it "warns with migration guidance" do + doctor.send(:check_deprecated_renderer_cache_task) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("pre_stage_bundle_for_node_renderer") }).to be(true) + expect(warning_msgs.any? { |m| m[:content].include?("MODE=symlink") }).to be(true) + end + end + + context "when no deploy scripts reference the deprecated task" do + around do |example| + Dir.mktmpdir { |tmpdir| Dir.chdir(tmpdir) { example.run } } + end + + it "adds no warnings" do + doctor.send(:check_deprecated_renderer_cache_task) + expect(checker.messages.select { |m| m[:type] == :warning }).to be_empty + end + end + end + describe "check_base_package_imports" do let(:doctor) { described_class.new(verbose: false, fix: false) } let(:checker) { doctor.instance_variable_get(:@checker) } diff --git a/react_on_rails_pro/lib/react_on_rails_pro/assets_precompile.rb b/react_on_rails_pro/lib/react_on_rails_pro/assets_precompile.rb index 43fc6d59e6..2747933174 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/assets_precompile.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/assets_precompile.rb @@ -63,7 +63,10 @@ def build_bundles def self.call instance.build_or_fetch_bundles - ReactOnRailsPro::PrepareNodeRenderBundles.call if ReactOnRailsPro.configuration.node_renderer? + # Auto-stage via symlink after asset precompile (same-filesystem default). + # Docker/image builds should invoke `rake react_on_rails_pro:pre_seed_renderer_cache` + # (MODE=copy, the default) as a separate step. + ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink) if ReactOnRailsPro.configuration.node_renderer? end def build_or_fetch_bundles 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 index ee811b5d2d..c1147199ec 100644 --- 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 @@ -1,47 +1,94 @@ # frozen_string_literal: true require "fileutils" +require "pathname" require "react_on_rails_pro/renderer_cache_helpers" module ReactOnRailsPro - # Pre-seeds the Node Renderer bundle cache by copying compiled server bundles - # into the renderer's expected directory structure. Designed for Docker builds - # where the bundle can be baked into the image, eliminating the 410→retry - # cold-start latency on first SSR request after deployment. + # Stages the Node Renderer bundle cache in the renderer's expected directory + # structure (`//.js`), including any configured + # assets_to_copy and, when RSC support is enabled, the RSC bundle and manifests. # - # Unlike PrepareNodeRenderBundles (which stages the same cache layout via - # symlinks for same-filesystem workflows), this class copies files so the - # cache can be baked into an image or other immutable artifact. + # Supports two modes: + # + # * `:copy` (default) — copies bundle and assets. Designed for Docker image + # builds where the cache must be baked into an immutable artifact. + # * `:symlink` — creates relative symlinks. For same-filesystem workflows + # (local dev, CI, Heroku-style same-dyno deploys, bundle-caching restores). + # + # Both modes produce the same on-disk cache layout, matching the renderer's + # runtime contract. The 410→retry cold-start round-trip on first SSR request + # is eliminated when the pre-seeded bundle is present at renderer startup. class PreSeedRendererCache - def self.call - cache_dir = resolve_cache_dir - puts "[ReactOnRailsPro] Pre-seeding renderer cache in: #{cache_dir}" + VALID_MODES = %i[copy symlink].freeze + + def self.call(mode: :copy) + unless VALID_MODES.include?(mode) + raise ArgumentError, "mode must be one of #{VALID_MODES.inspect}, got #{mode.inspect}" + end + + cache_dir = resolve_cache_dir(mode) + puts "[ReactOnRailsPro] Staging renderer cache (mode: #{mode}) in: #{cache_dir}" pool = ReactOnRailsPro::ServerRenderingPool::NodeRenderingPool assets = RendererCacheHelpers.collect_assets rsc_required_paths = RendererCacheHelpers.required_rsc_asset_paths - RendererCacheHelpers.bundle_sources(pool, "pre-seeding").each do |src_bundle_path, bundle_hash| - seed_bundle(src_bundle_path, bundle_hash, cache_dir) + RendererCacheHelpers.bundle_sources(pool, action_description(mode)).each do |src_bundle_path, bundle_hash| + bundle_dir = File.join(cache_dir, bundle_hash.to_s) + stage_bundle(src_bundle_path, bundle_dir, bundle_hash, mode) # The Node Renderer serves manifests from whichever bundle dir it loaded, # so both server and RSC dirs need the manifests present. - copy_assets(assets, File.join(cache_dir, bundle_hash.to_s), rsc_required_paths) + stage_assets(assets, bundle_dir, rsc_required_paths, mode) end end - def self.resolve_cache_dir + def self.resolve_cache_dir(mode) + enforce_cache_dir_env_var!(mode) ReactOnRailsPro::Utils.resolve_renderer_cache_dir end private_class_method :resolve_cache_dir - def self.seed_bundle(src_path, bundle_hash, cache_dir) - bundle_dir = File.join(cache_dir, bundle_hash.to_s) + # In copy mode (Docker image builds), silent fallback to Rails.root/.node-renderer-bundles + # is a footgun: the renderer process may run from a different cwd and resolve its default + # cache directory to a different path (e.g., /tmp/react-on-rails-pro-node-renderer-bundles), + # causing pre-seeded bundles to land somewhere the renderer never reads. Require an + # explicit env var in non-dev/test environments. + def self.enforce_cache_dir_env_var!(mode) + return unless mode == :copy + return if ENV["RENDERER_SERVER_BUNDLE_CACHE_PATH"].present? || ENV["RENDERER_BUNDLE_PATH"].present? + return if Rails.env.development? || Rails.env.test? + + raise ReactOnRailsPro::Error, <<~MSG.strip + Pre-seeding the renderer cache in copy mode (#{Rails.env}) requires an explicit + cache directory. Set RENDERER_SERVER_BUNDLE_CACHE_PATH in your environment, e.g. + in your Dockerfile: + + ENV RENDERER_SERVER_BUNDLE_CACHE_PATH=/app/.node-renderer-bundles + + The Node Renderer's default cache directory resolution differs between the Ruby + and standalone Node environments, so relying on the default in production-like + deploys can cause pre-seeded bundles to land in a path the renderer never reads. + MSG + end + private_class_method :enforce_cache_dir_env_var! + + def self.action_description(mode) + mode == :copy ? "pre-seeding" : "pre-staging" + end + private_class_method :action_description + + def self.stage_bundle(src_path, bundle_dir, bundle_hash, mode) dest_file = File.join(bundle_dir, "#{bundle_hash}.js") - FileUtils.mkdir_p(bundle_dir) - FileUtils.cp(src_path, dest_file) - puts "[ReactOnRailsPro] Pre-seeded renderer cache: #{dest_file}" + if mode == :copy + FileUtils.mkdir_p(bundle_dir) + FileUtils.cp(src_path, dest_file) + puts "[ReactOnRailsPro] Pre-seeded renderer cache: #{dest_file}" + else + make_relative_symlink(src_path, dest_file) + end end - private_class_method :seed_bundle + private_class_method :stage_bundle # RSC manifests are required when RSC is enabled — a missing manifest would cause # the renderer to fail at runtime with a hard-to-diagnose error. User-configured @@ -50,23 +97,41 @@ def self.seed_bundle(src_path, bundle_hash, cache_dir) # 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.copy_assets(assets, bundle_dir, rsc_required_paths) + def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) 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." + "Build your bundles before #{action_description(mode)} 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) - puts "[ReactOnRailsPro] Copied asset: #{dest}" + if mode == :copy + FileUtils.cp(expanded, dest) + puts "[ReactOnRailsPro] Copied asset: #{dest}" + else + make_relative_symlink(expanded, dest) + end end end - private_class_method :copy_assets + private_class_method :stage_assets + + def self.make_relative_symlink(source, destination) + destination_dir = Pathname.new(destination).dirname + FileUtils.mkdir_p(destination_dir) + FileUtils.rm_f(destination) + + # Canonicalize both sides so paths like /var -> /private/var do not + # produce broken relative symlinks when the cache dir comes from tmpdir. + source_path = Pathname.new(source).realpath + relative_source_path = source_path.relative_path_from(destination_dir.realpath) + File.symlink(relative_source_path, destination) + puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" + end + private_class_method :make_relative_symlink 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 index b917364a39..34cc4ad5cf 100644 --- 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 @@ -1,70 +1,32 @@ # frozen_string_literal: true -require "fileutils" -require "pathname" -require "react_on_rails_pro/renderer_cache_helpers" +require "react_on_rails_pro/pre_seed_renderer_cache" module ReactOnRailsPro - # Pre-stages the Node Renderer cache via symlinks for same-filesystem workflows - # such as local development, Heroku-style same-dyno deploys, and bundle-caching - # restores. The staged layout matches the renderer's runtime cache contract: - # //.js + # DEPRECATED: use `ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink)` directly. + # Retained as a thin shim so existing callers (custom rake tasks, Procfile entries, + # deploy scripts) keep working during the deprecation cycle. Emits a warning once + # per process on first call. class PrepareNodeRenderBundles - def self.make_relative_symlink(source, destination) - destination_dir = Pathname.new(destination).dirname - FileUtils.mkdir_p(destination_dir) - FileUtils.rm_f(destination) - - # Canonicalize both sides so paths like /var -> /private/var do not - # produce broken relative symlinks when the cache dir comes from tmpdir. - source_path = Pathname.new(source).realpath - relative_source_path = source_path.relative_path_from(destination_dir.realpath) - File.symlink(relative_source_path, destination) - puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" - end - private_class_method :make_relative_symlink - - def self.resolve_dest_path - ReactOnRailsPro::Utils.resolve_renderer_cache_dir - end - private_class_method :resolve_dest_path - def self.call - cache_dir = resolve_dest_path - pool = ReactOnRailsPro::ServerRenderingPool::NodeRenderingPool - puts "[ReactOnRailsPro] Pre-staging renderer cache via symlinks at: #{cache_dir}" + emit_deprecation_warning! + PreSeedRendererCache.call(mode: :symlink) + end - assets = RendererCacheHelpers.collect_assets - rsc_required_paths = RendererCacheHelpers.required_rsc_asset_paths + def self.emit_deprecation_warning! + return if @deprecation_warned - RendererCacheHelpers.bundle_sources(pool, "pre-staging").each do |src_bundle_path, bundle_hash| - bundle_dir = File.join(cache_dir, bundle_hash.to_s) - bundle_dest_path = File.join(bundle_dir, "#{bundle_hash}.js") - make_relative_symlink(src_bundle_path, bundle_dest_path) - symlink_assets(assets, bundle_dir, rsc_required_paths) - end + warn "[ReactOnRailsPro] ReactOnRailsPro::PrepareNodeRenderBundles is deprecated. " \ + "Use ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink) instead. " \ + "The rake task equivalent is 'rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink'." + @deprecation_warned = true end + private_class_method :emit_deprecation_warning! - # 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)) - make_relative_symlink(expanded, destination_full_path) - end + # :nodoc: Test helper — resets the one-time deprecation-warning guard so + # specs can exercise the warning path without leaking state between examples. + def self.reset_deprecation_warned! + @deprecation_warned = nil end - private_class_method :symlink_assets end end diff --git a/react_on_rails_pro/lib/tasks/assets.rake b/react_on_rails_pro/lib/tasks/assets.rake index 2c1bb79780..aa868fed30 100644 --- a/react_on_rails_pro/lib/tasks/assets.rake +++ b/react_on_rails_pro/lib/tasks/assets.rake @@ -3,14 +3,21 @@ require "active_support" namespace :react_on_rails_pro do - desc "Pre-stage renderer cache locally via symlinks (legacy same-filesystem workflow)" - task pre_stage_bundle_for_node_renderer: :environment do - ReactOnRailsPro::PrepareNodeRenderBundles.call + desc "Stage the Node Renderer bundle cache. MODE=copy (default; Docker/image builds) " \ + "or MODE=symlink (dev/CI/same-filesystem deploys)." + task pre_seed_renderer_cache: :environment do + mode = (ENV["MODE"] || "copy").to_sym + ReactOnRailsPro::PreSeedRendererCache.call(mode: mode) end - desc "Pre-seed renderer cache for Docker/image builds via copies" - task pre_seed_renderer_cache: :environment do - ReactOnRailsPro::PreSeedRendererCache.call + # Deprecated alias. Delegates to pre_seed_renderer_cache with MODE=symlink so + # existing Procfile/Dockerfile/deploy-script entries keep working during the + # deprecation cycle. + desc "DEPRECATED: use 'pre_seed_renderer_cache MODE=symlink' instead." + task pre_stage_bundle_for_node_renderer: :environment do + warn "[ReactOnRailsPro] The 'react_on_rails_pro:pre_stage_bundle_for_node_renderer' rake task " \ + "is deprecated. Use 'rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink' instead." + ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink) end desc "Copy assets to remote node-renderer" diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index 52e9ef2a6c..460b4b49db 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -53,6 +53,45 @@ ENV.delete("RENDERER_BUNDLE_PATH") end + context "when mode is invalid" do + it "raises ArgumentError" do + expect { described_class.call(mode: :hardlink) }.to raise_error(ArgumentError, /mode must be one of/) + end + end + + context "when mode is :symlink" do + it "symlinks instead of copying" do + described_class.call(mode: :symlink) + + dest_file = File.join(bundle_dir, "#{bundle_hash}.js") + expect(File.exist?(dest_file)).to be(true) + expect(File.symlink?(dest_file)).to be(true) + end + end + + context "when mode is :copy and no env var is set in a non-dev/test environment" do + before do + allow(Rails.env).to receive_messages(development?: false, test?: false) + allow(ReactOnRailsPro.configuration).to receive(:assets_to_copy).and_return(nil) + end + + it "raises a clear error pointing at RENDERER_SERVER_BUNDLE_CACHE_PATH" do + expect { described_class.call(mode: :copy) } + .to raise_error(ReactOnRailsPro::Error, /RENDERER_SERVER_BUNDLE_CACHE_PATH/) + end + + it "does not raise when the preferred env var is set" do + ENV["RENDERER_SERVER_BUNDLE_CACHE_PATH"] = Dir.mktmpdir("renderer-cache-test") + expect { described_class.call(mode: :copy) }.not_to raise_error + ensure + FileUtils.rm_rf(ENV.fetch("RENDERER_SERVER_BUNDLE_CACHE_PATH", nil)) + end + + it "does not raise in :symlink mode even without an env var" do + expect { described_class.call(mode: :symlink) }.not_to raise_error + end + end + context "when assets exist" do before do FileUtils.cp(fixture_path, path_in_webpack_folder(asset_filename)) diff --git a/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb b/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb index a8115527fa..359ddf0d77 100644 --- a/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb @@ -39,6 +39,7 @@ ENV.delete("RENDERER_SERVER_BUNDLE_CACHE_PATH") ENV.delete("RENDERER_BUNDLE_PATH") ReactOnRailsPro::Utils.reset_renderer_bundle_path_deprecation_warned! + described_class.reset_deprecation_warned! end after do @@ -48,6 +49,11 @@ FileUtils.rm_f(path_in_webpack_folder(asset_filename2)) ENV.delete("RENDERER_SERVER_BUNDLE_CACHE_PATH") ENV.delete("RENDERER_BUNDLE_PATH") + described_class.reset_deprecation_warned! + end + + it "emits a deprecation warning pointing at PreSeedRendererCache" do + expect { pre_stage_cache }.to output(/deprecated.*PreSeedRendererCache/m).to_stderr end context "when assets exist" do @@ -84,7 +90,8 @@ allow(ReactOnRailsPro.configuration).to receive(:assets_to_copy).and_return([first_asset_path]) FileUtils.rm_f(first_asset_path) - expect { pre_stage_cache }.to output("[ReactOnRailsPro] Asset not found #{first_asset_path}\n").to_stderr + expected = /\[ReactOnRailsPro\] Asset not found #{Regexp.escape(first_asset_path.to_s)}/ + expect { pre_stage_cache }.to output(expected).to_stderr end end From 14f4a45aeec9f33c88c9c5ea433628df571d4df6 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Fri, 17 Apr 2026 21:19:13 -1000 Subject: [PATCH 02/17] fix: address PR B review feedback for unified renderer cache staging - Doctor check now tailors the suggested migration command per matched file type: Dockerfile entries get the bare 'rake pre_seed_renderer_cache' (MODE=copy is the default); Procfile/bin entries get 'MODE=symlink'. - The copy-mode env-var error now mentions 'MODE=symlink' as an escape hatch for CI users who don't need an immutable artifact. - make_relative_symlink rescues Errno::ENOENT from Pathname#realpath and surfaces a clearer ReactOnRailsPro::Error when a bundle/asset vanished between existence check and staging (e.g. webpack output rotating mid-stage, dangling symlinks in the source tree). - PrepareNodeRenderBundles.reset_deprecation_warned! is now private_class_method; specs invoke it via send. - Rake task now downcases ENV['MODE'] and validates against PreSeedRendererCache::VALID_MODES before to_sym, accepting 'MODE=Copy', 'MODE=COPY', etc. Unknown values abort with a clear error listing valid modes. - Added a :symlink-mode spec that exercises asset symlinking (previously only covered via the PrepareNodeRenderBundles shim). - env-var-bypass spec now uses ensure block with a local tmpdir variable to guarantee cleanup even if Dir.mktmpdir or an intermediate assertion raises. Declined: thread-safety wrapper around @deprecation_warned. This code runs only from rake tasks and AssetsPrecompile.call (single-threaded contexts); a race would produce at most a harmless duplicate warning, not a correctness issue. Consistent with PR A precedent for @renderer_bundle_path_deprecation_warned. 27 dummy specs pass; rubocop clean on all changed files. Refs: #3122, #3167 Co-Authored-By: Claude Opus 4.7 (1M context) --- react_on_rails/lib/react_on_rails/doctor.rb | 16 ++++++++++---- .../pre_seed_renderer_cache.rb | 14 +++++++++++++ .../prepare_node_renderer_bundles.rb | 3 +++ react_on_rails_pro/lib/tasks/assets.rake | 9 ++++++-- .../spec/pre_seed_renderer_cache_spec.rb | 21 ++++++++++++++++--- .../prepare_node_renderer_bundles_spec.rb | 4 ++-- 6 files changed, 56 insertions(+), 11 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 2e8dba15ee..bc0b2d4839 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2748,10 +2748,7 @@ def check_deprecated_renderer_cache_task checker.add_warning(<<~MSG.strip) ⚠️ Deprecated rake task '#{DEPRECATED_RENDERER_CACHE_TASK}' referenced in: - #{matches.map { |p| " • #{p}" }.join("\n")} - - Replace with: - rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink + #{matches.map { |p| " • #{p} → #{renderer_cache_migration_suggestion(p)}" }.join("\n")} The unified 'pre_seed_renderer_cache' task uses MODE=copy by default (for Docker/image builds) and MODE=symlink for same-filesystem workflows. @@ -2760,6 +2757,17 @@ def check_deprecated_renderer_cache_task checker.add_warning("⚠️ Could not scan for deprecated renderer-cache task references: #{e.message}") end + # Dockerfile matches mean the user is building an image, so they want the + # copy-mode default (no MODE needed). Procfile/bin scripts mean same-filesystem + # runtime staging, which needs the symlink mode. + def renderer_cache_migration_suggestion(path) + if path.start_with?("Dockerfile") + "rake react_on_rails_pro:pre_seed_renderer_cache" + else + "rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink" + end + end + # The base 'react-on-rails' npm package is a transitive dependency of 'react-on-rails-pro', # so `import ... from 'react-on-rails'` resolves silently — loading the base package instead # of Pro. Components registered through the base package won't have Pro features (streaming, 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 index c1147199ec..6d3aa6c60d 100644 --- 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 @@ -69,6 +69,11 @@ def self.enforce_cache_dir_env_var!(mode) The Node Renderer's default cache directory resolution differs between the Ruby and standalone Node environments, so relying on the default in production-like deploys can cause pre-seeded bundles to land in a path the renderer never reads. + + If you don't need an immutable artifact (e.g. in CI or same-filesystem deploys), + use mode: :symlink instead: + + rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink MSG end private_class_method :enforce_cache_dir_env_var! @@ -127,10 +132,19 @@ def self.make_relative_symlink(source, destination) # Canonicalize both sides so paths like /var -> /private/var do not # produce broken relative symlinks when the cache dir comes from tmpdir. + # Pathname#realpath raises Errno::ENOENT on a dangling symlink or a + # path that vanished between File.exist? and here (e.g. webpack output + # rotating mid-stage). Surface that as a clear ReactOnRailsPro::Error + # rather than a raw system error. source_path = Pathname.new(source).realpath relative_source_path = source_path.relative_path_from(destination_dir.realpath) File.symlink(relative_source_path, destination) puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" + rescue Errno::ENOENT => e + raise ReactOnRailsPro::Error, + "Could not resolve real path for symlink source #{source} " \ + "(#{e.message}). The file may have been removed or may be a dangling symlink. " \ + "Rebuild your bundles before staging the renderer cache." end private_class_method :make_relative_symlink 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 index 34cc4ad5cf..8257ad34bc 100644 --- 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 @@ -25,8 +25,11 @@ def self.emit_deprecation_warning! # :nodoc: Test helper — resets the one-time deprecation-warning guard so # specs can exercise the warning path without leaking state between examples. + # Private so it can only be invoked from specs via `send`; prevents accidental + # reset from production code. def self.reset_deprecation_warned! @deprecation_warned = nil end + private_class_method :reset_deprecation_warned! end end diff --git a/react_on_rails_pro/lib/tasks/assets.rake b/react_on_rails_pro/lib/tasks/assets.rake index aa868fed30..d305cde861 100644 --- a/react_on_rails_pro/lib/tasks/assets.rake +++ b/react_on_rails_pro/lib/tasks/assets.rake @@ -6,8 +6,13 @@ namespace :react_on_rails_pro do desc "Stage the Node Renderer bundle cache. MODE=copy (default; Docker/image builds) " \ "or MODE=symlink (dev/CI/same-filesystem deploys)." task pre_seed_renderer_cache: :environment do - mode = (ENV["MODE"] || "copy").to_sym - ReactOnRailsPro::PreSeedRendererCache.call(mode: mode) + raw_mode = ENV["MODE"].to_s.downcase + raw_mode = "copy" if raw_mode.empty? + unless ReactOnRailsPro::PreSeedRendererCache::VALID_MODES.map(&:to_s).include?(raw_mode) + valid = ReactOnRailsPro::PreSeedRendererCache::VALID_MODES.map(&:to_s).join(", ") + abort "[ReactOnRailsPro] Unknown MODE=#{ENV.fetch('MODE', nil).inspect}. Expected one of: #{valid}" + end + ReactOnRailsPro::PreSeedRendererCache.call(mode: raw_mode.to_sym) end # Deprecated alias. Delegates to pre_seed_renderer_cache with MODE=symlink so diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index 460b4b49db..7372526c30 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -60,13 +60,26 @@ end context "when mode is :symlink" do - it "symlinks instead of copying" do + it "symlinks the bundle instead of copying it" do described_class.call(mode: :symlink) dest_file = File.join(bundle_dir, "#{bundle_hash}.js") expect(File.exist?(dest_file)).to be(true) expect(File.symlink?(dest_file)).to be(true) end + + it "symlinks assets rather than copying them" do + FileUtils.cp(fixture_path, path_in_webpack_folder(asset_filename)) + FileUtils.cp(fixture_path2, path_in_webpack_folder(asset_filename2)) + + described_class.call(mode: :symlink) + + first_asset = File.join(bundle_dir, asset_filename) + second_asset = File.join(bundle_dir, asset_filename2) + expect(File.symlink?(first_asset)).to be(true) + expect(File.symlink?(second_asset)).to be(true) + expect(File.realpath(first_asset)).to eq(path_in_webpack_folder(asset_filename).to_s) + end end context "when mode is :copy and no env var is set in a non-dev/test environment" do @@ -81,10 +94,12 @@ end it "does not raise when the preferred env var is set" do - ENV["RENDERER_SERVER_BUNDLE_CACHE_PATH"] = Dir.mktmpdir("renderer-cache-test") + tmpdir = Dir.mktmpdir("renderer-cache-test") + ENV["RENDERER_SERVER_BUNDLE_CACHE_PATH"] = tmpdir expect { described_class.call(mode: :copy) }.not_to raise_error ensure - FileUtils.rm_rf(ENV.fetch("RENDERER_SERVER_BUNDLE_CACHE_PATH", nil)) + FileUtils.rm_rf(tmpdir) if tmpdir + ENV.delete("RENDERER_SERVER_BUNDLE_CACHE_PATH") end it "does not raise in :symlink mode even without an env var" do diff --git a/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb b/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb index 359ddf0d77..46611c8dda 100644 --- a/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/prepare_node_renderer_bundles_spec.rb @@ -39,7 +39,7 @@ ENV.delete("RENDERER_SERVER_BUNDLE_CACHE_PATH") ENV.delete("RENDERER_BUNDLE_PATH") ReactOnRailsPro::Utils.reset_renderer_bundle_path_deprecation_warned! - described_class.reset_deprecation_warned! + described_class.send(:reset_deprecation_warned!) end after do @@ -49,7 +49,7 @@ FileUtils.rm_f(path_in_webpack_folder(asset_filename2)) ENV.delete("RENDERER_SERVER_BUNDLE_CACHE_PATH") ENV.delete("RENDERER_BUNDLE_PATH") - described_class.reset_deprecation_warned! + described_class.send(:reset_deprecation_warned!) end it "emits a deprecation warning pointing at PreSeedRendererCache" do From 1f9e79681fbc43c6e9e616cd14b5938fcc3a8fb7 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 18 Apr 2026 09:49:51 -1000 Subject: [PATCH 03/17] fix: address PR #3167 review feedback (realpath, mutex, size guard) - pre_seed_renderer_cache: rescue Errno::ENOENT inside make_relative_symlink and raise ReactOnRailsPro::Error so a dangling symlink or race between File.exist? and realpath surfaces with a clear message instead of a raw system error. - prepare_node_renderer_bundles: guard the check-then-set on @deprecation_warned with a Mutex so concurrent callers (multi-worker Puma boots of the shim) still emit exactly one warning per process. - doctor: cap File.read in check_deprecated_renderer_cache_task at 1 MiB and skip unusually large deploy scripts defensively. Co-Authored-By: Claude Opus 4.7 (1M context) --- react_on_rails/lib/react_on_rails/doctor.rb | 6 +++++- .../pre_seed_renderer_cache.rb | 8 +++++++- .../prepare_node_renderer_bundles.rb | 19 +++++++++++++------ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index bc0b2d4839..5db22b0c4a 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2738,10 +2738,14 @@ def check_pro_renderer_mode "bin/release", "bin/docker-entrypoint" ].freeze + RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES = 1_048_576 def check_deprecated_renderer_cache_task matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| - File.exist?(path) && File.read(path).include?(DEPRECATED_RENDERER_CACHE_TASK) + next false unless File.exist?(path) + next false if File.size(path) >= RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES + + File.read(path).include?(DEPRECATED_RENDERER_CACHE_TASK) end return if matches.empty? 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 index 6d3aa6c60d..9adb92a5b0 100644 --- 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 @@ -136,7 +136,13 @@ def self.make_relative_symlink(source, destination) # path that vanished between File.exist? and here (e.g. webpack output # rotating mid-stage). Surface that as a clear ReactOnRailsPro::Error # rather than a raw system error. - source_path = Pathname.new(source).realpath + source_path = + begin + Pathname.new(source).realpath + rescue Errno::ENOENT + raise ReactOnRailsPro::Error, + "Cannot resolve real path for #{source} — it does not exist or is a dangling symlink." + end relative_source_path = source_path.relative_path_from(destination_dir.realpath) File.symlink(relative_source_path, destination) puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" 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 index 8257ad34bc..6a005223a4 100644 --- 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 @@ -8,18 +8,25 @@ module ReactOnRailsPro # deploy scripts) keep working during the deprecation cycle. Emits a warning once # per process on first call. class PrepareNodeRenderBundles + # Mutex guards the check-then-set on @deprecation_warned so concurrent callers + # (e.g. multiple Puma workers invoking the shim at boot) still see exactly one + # warning per process. + @deprecation_mutex = Mutex.new + def self.call emit_deprecation_warning! PreSeedRendererCache.call(mode: :symlink) end def self.emit_deprecation_warning! - return if @deprecation_warned + @deprecation_mutex.synchronize do + return if @deprecation_warned - warn "[ReactOnRailsPro] ReactOnRailsPro::PrepareNodeRenderBundles is deprecated. " \ - "Use ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink) instead. " \ - "The rake task equivalent is 'rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink'." - @deprecation_warned = true + warn "[ReactOnRailsPro] ReactOnRailsPro::PrepareNodeRenderBundles is deprecated. " \ + "Use ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink) instead. " \ + "The rake task equivalent is 'rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink'." + @deprecation_warned = true + end end private_class_method :emit_deprecation_warning! @@ -28,7 +35,7 @@ def self.emit_deprecation_warning! # Private so it can only be invoked from specs via `send`; prevents accidental # reset from production code. def self.reset_deprecation_warned! - @deprecation_warned = nil + @deprecation_mutex.synchronize { @deprecation_warned = nil } end private_class_method :reset_deprecation_warned! end From 5b9e0611454558dbcba7890c2af0bb09520ed859 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 15:02:35 -1000 Subject: [PATCH 04/17] fix: address PR #3167 review feedback (optional items) - doctor.rb: scan deprecated-task references via Rails.root.join so the check still fires when doctor is invoked from a subdirectory - doctor.rb: move renderer-cache constants to the top-of-class constant group for readability - pre_seed_renderer_cache.rb: drop ActiveSupport .present? in enforce_cache_dir_env_var! and treat whitespace-only env vars as unset - pre_seed_renderer_cache.rb: wrap destination_dir.realpath in its own rescue in make_relative_symlink so the error message correctly names the destination when the race fires, and drop the now-redundant outer rescue - assets.rake: rewrite deprecated-task warning to mention MODE=copy for Docker builds and point readers to `rake react_on_rails:doctor` for per-file migration guidance Co-Authored-By: Claude Opus 4.7 (1M context) --- react_on_rails/lib/react_on_rails/doctor.rb | 43 +++++++++++-------- .../spec/lib/react_on_rails/doctor_spec.rb | 27 ++++++------ .../pre_seed_renderer_cache.rb | 28 +++++++----- react_on_rails_pro/lib/tasks/assets.rake | 8 ++-- 4 files changed, 61 insertions(+), 45 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 5db22b0c4a..fc0b194cd8 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -57,6 +57,23 @@ class Doctor DEFAULT_SHAKAPACKER_CONFIG_PATH = "config/shakapacker.yml" SERVER_BUNDLE_SOURCE_EXTENSIONS = %w[.js .jsx .ts .tsx .mjs .cjs].freeze + # Deprecated-renderer-cache scan (used by check_deprecated_renderer_cache_task): + # look for references to the old pre_stage_bundle_for_node_renderer task in + # common deploy-script locations so users on older Procfile/Dockerfile entries + # get a migration nudge before the task is removed. + DEPRECATED_RENDERER_CACHE_TASK = "pre_stage_bundle_for_node_renderer" + RENDERER_CACHE_DEPLOY_SCRIPT_PATHS = [ + "Procfile", + "Procfile.dev", + "Procfile.dev-static-assets", + "Procfile.production", + "Dockerfile", + "bin/deploy", + "bin/release", + "bin/docker-entrypoint" + ].freeze + RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES = 1_048_576 + def initialize(verbose: false, fix: false) @verbose = verbose @fix = fix @@ -2724,28 +2741,16 @@ def check_pro_renderer_mode checker.add_warning("⚠️ Could not detect Pro renderer mode: #{e.message}") end - # Scan common deploy-script locations for references to the deprecated - # pre_stage_bundle_for_node_renderer rake task, so users on older Procfile/ - # Dockerfile entries get a clear migration nudge before the task is removed. - DEPRECATED_RENDERER_CACHE_TASK = "pre_stage_bundle_for_node_renderer" - RENDERER_CACHE_DEPLOY_SCRIPT_PATHS = [ - "Procfile", - "Procfile.dev", - "Procfile.dev-static-assets", - "Procfile.production", - "Dockerfile", - "bin/deploy", - "bin/release", - "bin/docker-entrypoint" - ].freeze - RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES = 1_048_576 - def check_deprecated_renderer_cache_task + # Resolve against Rails.root (not Dir.pwd) so the scan still fires when + # doctor is invoked from a subdirectory — otherwise the checks silently + # find nothing and the deprecation warning never surfaces. matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| - next false unless File.exist?(path) - next false if File.size(path) >= RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES + full_path = Rails.root.join(path) + next false unless full_path.exist? + next false if full_path.size >= RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES - File.read(path).include?(DEPRECATED_RENDERER_CACHE_TASK) + full_path.read.include?(DEPRECATED_RENDERER_CACHE_TASK) end return if matches.empty? diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 20288d388b..00451e8e9e 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2401,18 +2401,18 @@ class << self let(:checker) { doctor.instance_variable_get(:@checker) } context "when a Procfile references the deprecated task" do - around do |example| - Dir.mktmpdir do |tmpdir| - Dir.chdir(tmpdir) do - File.write( - "Procfile", - "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer && bundle exec puma\n" - ) - example.run - end - end + let(:tmpdir) { Dir.mktmpdir } + + before do + File.write( + File.join(tmpdir, "Procfile"), + "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer && bundle exec puma\n" + ) + allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) end + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } + it "warns with migration guidance" do doctor.send(:check_deprecated_renderer_cache_task) warning_msgs = checker.messages.select { |m| m[:type] == :warning } @@ -2422,9 +2422,10 @@ class << self end context "when no deploy scripts reference the deprecated task" do - around do |example| - Dir.mktmpdir { |tmpdir| Dir.chdir(tmpdir) { example.run } } - end + let(:tmpdir) { Dir.mktmpdir } + + before { allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) } + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } it "adds no warnings" do doctor.send(:check_deprecated_renderer_cache_task) 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 index 9adb92a5b0..08454dea70 100644 --- 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 @@ -56,7 +56,10 @@ def self.resolve_cache_dir(mode) # explicit env var in non-dev/test environments. def self.enforce_cache_dir_env_var!(mode) return unless mode == :copy - return if ENV["RENDERER_SERVER_BUNDLE_CACHE_PATH"].present? || ENV["RENDERER_BUNDLE_PATH"].present? + # Use a plain-Ruby check (no ActiveSupport .present?) so whitespace-only + # values are treated as "not set" and the guard remains portable. + return unless ENV.fetch("RENDERER_SERVER_BUNDLE_CACHE_PATH", "").strip.empty? && + ENV.fetch("RENDERER_BUNDLE_PATH", "").strip.empty? return if Rails.env.development? || Rails.env.test? raise ReactOnRailsPro::Error, <<~MSG.strip @@ -134,23 +137,28 @@ def self.make_relative_symlink(source, destination) # produce broken relative symlinks when the cache dir comes from tmpdir. # Pathname#realpath raises Errno::ENOENT on a dangling symlink or a # path that vanished between File.exist? and here (e.g. webpack output - # rotating mid-stage). Surface that as a clear ReactOnRailsPro::Error - # rather than a raw system error. + # rotating mid-stage). Wrap each realpath call separately so the error + # message correctly names the side that failed. source_path = begin Pathname.new(source).realpath rescue Errno::ENOENT raise ReactOnRailsPro::Error, - "Cannot resolve real path for #{source} — it does not exist or is a dangling symlink." + "Cannot resolve real path for symlink source #{source} — " \ + "it does not exist or is a dangling symlink. " \ + "Rebuild your bundles before staging the renderer cache." end - relative_source_path = source_path.relative_path_from(destination_dir.realpath) + destination_dir_real = + begin + destination_dir.realpath + rescue Errno::ENOENT + raise ReactOnRailsPro::Error, + "Cannot resolve real path for symlink destination dir #{destination_dir} — " \ + "it may have been removed after mkdir_p (race with an external cleanup)." + end + relative_source_path = source_path.relative_path_from(destination_dir_real) File.symlink(relative_source_path, destination) puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" - rescue Errno::ENOENT => e - raise ReactOnRailsPro::Error, - "Could not resolve real path for symlink source #{source} " \ - "(#{e.message}). The file may have been removed or may be a dangling symlink. " \ - "Rebuild your bundles before staging the renderer cache." end private_class_method :make_relative_symlink end diff --git a/react_on_rails_pro/lib/tasks/assets.rake b/react_on_rails_pro/lib/tasks/assets.rake index d305cde861..1e7324cf27 100644 --- a/react_on_rails_pro/lib/tasks/assets.rake +++ b/react_on_rails_pro/lib/tasks/assets.rake @@ -18,10 +18,12 @@ namespace :react_on_rails_pro do # Deprecated alias. Delegates to pre_seed_renderer_cache with MODE=symlink so # existing Procfile/Dockerfile/deploy-script entries keep working during the # deprecation cycle. - desc "DEPRECATED: use 'pre_seed_renderer_cache MODE=symlink' instead." + desc "DEPRECATED: use 'pre_seed_renderer_cache' (MODE=copy for Docker, MODE=symlink for same-filesystem)." task pre_stage_bundle_for_node_renderer: :environment do - warn "[ReactOnRailsPro] The 'react_on_rails_pro:pre_stage_bundle_for_node_renderer' rake task " \ - "is deprecated. Use 'rake react_on_rails_pro:pre_seed_renderer_cache MODE=symlink' instead." + warn "[ReactOnRailsPro] The 'react_on_rails_pro:pre_stage_bundle_for_node_renderer' rake task is deprecated. " \ + "Migrate to 'rake react_on_rails_pro:pre_seed_renderer_cache' — use MODE=copy (default) for Docker/image " \ + "builds, or MODE=symlink for same-filesystem deploys (this shim preserves the old symlink behavior). " \ + "Run 'rake react_on_rails:doctor' for a per-file migration suggestion based on where the task is referenced." ReactOnRailsPro::PreSeedRendererCache.call(mode: :symlink) end From 7a06c5c4c29fe32bc9ed85ede79f757bd8a4ac89 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 16:05:58 -1000 Subject: [PATCH 05/17] fix: address PR #3167 round-4 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add doctor spec for Dockerfile branch of renderer_cache_migration_suggestion (asserts the per-file suggestion line omits MODE=symlink) - Cache VALID_MODES.map(&:to_s) once in the pre_seed_renderer_cache rake task - Document the substring-match heuristic in check_deprecated_renderer_cache_task - Document make_relative_symlink's rm_f/symlink non-atomicity (benign — covered by the renderer's 410→refetch retry) - Document why the deprecated rake-shim warning isn't mutex-guarded (rake is invoked once per deploy) Co-Authored-By: Claude Opus 4.7 (1M context) --- react_on_rails/lib/react_on_rails/doctor.rb | 5 ++++ .../spec/lib/react_on_rails/doctor_spec.rb | 26 +++++++++++++++++++ .../pre_seed_renderer_cache.rb | 4 +++ react_on_rails_pro/lib/tasks/assets.rake | 11 +++++--- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index fc0b194cd8..778fa2080c 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2745,6 +2745,11 @@ def check_deprecated_renderer_cache_task # Resolve against Rails.root (not Dir.pwd) so the scan still fires when # doctor is invoked from a subdirectory — otherwise the checks silently # find nothing and the deprecation warning never surfaces. + # + # Substring match is intentional: a comment line in a Procfile/Dockerfile + # that mentions the old task name will also trigger the warning. That is + # acceptable — the worst case is a benign migration nudge on a file that's + # already been migrated but still references the old name in a comment. matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| full_path = Rails.root.join(path) next false unless full_path.exist? diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 00451e8e9e..7f833bc955 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2421,6 +2421,32 @@ class << self end end + context "when a Dockerfile references the deprecated task" do + let(:tmpdir) { Dir.mktmpdir } + + before do + File.write( + File.join(tmpdir, "Dockerfile"), + "RUN bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n" + ) + allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) + end + + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } + + it "suggests the copy-mode task without MODE=symlink" do + doctor.send(:check_deprecated_renderer_cache_task) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs).not_to be_empty + suggestion_line = warning_msgs + .flat_map { |m| m[:content].split("\n") } + .find { |line| line.include?("Dockerfile →") } + expect(suggestion_line).not_to be_nil + expect(suggestion_line).to include("pre_seed_renderer_cache") + expect(suggestion_line).not_to include("MODE=symlink") + end + end + context "when no deploy scripts reference the deprecated task" do let(:tmpdir) { Dir.mktmpdir } 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 index 08454dea70..c4a4be2828 100644 --- 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 @@ -128,6 +128,10 @@ def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) end private_class_method :stage_assets + # Replaces `destination` with a relative symlink to `source`. Not atomic: + # if the process is killed between `rm_f` and `File.symlink` the destination + # is briefly absent. In practice the renderer's 410→refetch retry at + # request time recovers from a missing bundle, so the brief gap is benign. def self.make_relative_symlink(source, destination) destination_dir = Pathname.new(destination).dirname FileUtils.mkdir_p(destination_dir) diff --git a/react_on_rails_pro/lib/tasks/assets.rake b/react_on_rails_pro/lib/tasks/assets.rake index 1e7324cf27..0f25571db5 100644 --- a/react_on_rails_pro/lib/tasks/assets.rake +++ b/react_on_rails_pro/lib/tasks/assets.rake @@ -8,9 +8,10 @@ namespace :react_on_rails_pro do task pre_seed_renderer_cache: :environment do raw_mode = ENV["MODE"].to_s.downcase raw_mode = "copy" if raw_mode.empty? - unless ReactOnRailsPro::PreSeedRendererCache::VALID_MODES.map(&:to_s).include?(raw_mode) - valid = ReactOnRailsPro::PreSeedRendererCache::VALID_MODES.map(&:to_s).join(", ") - abort "[ReactOnRailsPro] Unknown MODE=#{ENV.fetch('MODE', nil).inspect}. Expected one of: #{valid}" + valid_modes = ReactOnRailsPro::PreSeedRendererCache::VALID_MODES.map(&:to_s) + unless valid_modes.include?(raw_mode) + abort "[ReactOnRailsPro] Unknown MODE=#{ENV.fetch('MODE', nil).inspect}. " \ + "Expected one of: #{valid_modes.join(', ')}" end ReactOnRailsPro::PreSeedRendererCache.call(mode: raw_mode.to_sym) end @@ -18,6 +19,10 @@ namespace :react_on_rails_pro do # Deprecated alias. Delegates to pre_seed_renderer_cache with MODE=symlink so # existing Procfile/Dockerfile/deploy-script entries keep working during the # deprecation cycle. + # + # The warning below fires on every task invocation (it is not guarded by the + # once-per-process mutex in PrepareNodeRenderBundles). Rake tasks are invoked + # once per deploy, so repeated output is not a concern here. desc "DEPRECATED: use 'pre_seed_renderer_cache' (MODE=copy for Docker, MODE=symlink for same-filesystem)." task pre_stage_bundle_for_node_renderer: :environment do warn "[ReactOnRailsPro] The 'react_on_rails_pro:pre_stage_bundle_for_node_renderer' rake task is deprecated. " \ From 788ce44ab82ec70043305000d9c3b9ec05b9aa96 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 16:32:12 -1000 Subject: [PATCH 06/17] fix: address PR #3167 round-5 review feedback - pre_seed_renderer_cache.rb: rescue Errno::EEXIST at File.symlink so a concurrent winner between rm_f and the symlink call is treated as success (two Puma workers racing through AssetsPrecompile.call). - pre_seed_renderer_cache.rb: extract shared stage_file(src, dest, mode, log) helper so stage_bundle and stage_assets share the mode dispatch. - prepare_node_renderer_bundles.rb: reset @deprecation_warned to false (symmetric with the = true write above) instead of nil. - docs/pro/node-renderer.md: clarify that "non-dev/test" covers custom environments (staging, review, ci), so RENDERER_SERVER_BUNDLE_CACHE_PATH is required there in MODE=copy. - pre_seed_renderer_cache_spec.rb: stub File.symlink to raise Errno::EEXIST once and assert the guard does not propagate. - doctor_spec.rb: stub RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES to 64 bytes and assert the size gate silently skips an over-cap Procfile. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/pro/node-renderer.md | 2 +- .../spec/lib/react_on_rails/doctor_spec.rb | 23 +++++++++++ .../pre_seed_renderer_cache.rb | 38 ++++++++++++------- .../prepare_node_renderer_bundles.rb | 2 +- .../spec/pre_seed_renderer_cache_spec.rb | 9 +++++ 5 files changed, 59 insertions(+), 15 deletions(-) diff --git a/docs/pro/node-renderer.md b/docs/pro/node-renderer.md index 4deeca36cc..0647f0b7d8 100644 --- a/docs/pro/node-renderer.md +++ b/docs/pro/node-renderer.md @@ -147,7 +147,7 @@ The task follows the same environment-variable precedence as the Node Renderer, 2. `RENDERER_BUNDLE_PATH` environment variable (deprecated — emits a warning) 3. `Rails.root.join(".node-renderer-bundles")` (Rails-side default when env vars are unset, only accepted for `MODE=symlink` and in dev/test) -In **`MODE=copy`** (Docker image builds) the task requires one of the env vars above to be set in non-dev/test environments. Because the Node renderer's own default can differ (e.g., falling back to `/tmp/react-on-rails-pro-node-renderer-bundles` when its `cwd` sits outside the app tree), relying on the silent fallback risks pre-seeded bundles landing in a directory the renderer never reads. The task raises a clear error if the env var is missing: +In **`MODE=copy`** (Docker image builds) the task requires one of the env vars above to be set in non-dev/test environments. "Non-dev/test" means any `RAILS_ENV` other than `development` or `test` — including custom environments like `staging`, `review`, or `ci` — so set `RENDERER_SERVER_BUNDLE_CACHE_PATH` wherever you run `MODE=copy` outside of local/CI-test runs. Because the Node renderer's own default can differ (e.g., falling back to `/tmp/react-on-rails-pro-node-renderer-bundles` when its `cwd` sits outside the app tree), relying on the silent fallback risks pre-seeded bundles landing in a directory the renderer never reads. The task raises a clear error if the env var is missing: ```dockerfile ENV RENDERER_SERVER_BUNDLE_CACHE_PATH=/app/.node-renderer-bundles diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 7f833bc955..787c01576b 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2458,6 +2458,29 @@ class << self expect(checker.messages.select { |m| m[:type] == :warning }).to be_empty end end + + context "when a deploy-script file exceeds the size gate" do + let(:tmpdir) { Dir.mktmpdir } + + before do + # Stub the cap so we do not have to write a real 1 MB file — the gate + # logic is what we are exercising, not the specific threshold. + stub_const("ReactOnRails::Doctor::RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES", 64) + padding = "x" * 128 + File.write( + File.join(tmpdir, "Procfile"), + "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n#{padding}" + ) + allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) + end + + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } + + it "silently skips the file and emits no warning" do + doctor.send(:check_deprecated_renderer_cache_task) + expect(checker.messages.select { |m| m[:type] == :warning }).to be_empty + end + end end describe "check_base_package_imports" do 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 index c4a4be2828..661f7b7354 100644 --- 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 @@ -88,15 +88,22 @@ def self.action_description(mode) def self.stage_bundle(src_path, bundle_dir, bundle_hash, mode) dest_file = File.join(bundle_dir, "#{bundle_hash}.js") + stage_file(src_path, dest_file, mode, "Pre-seeded renderer cache") + end + private_class_method :stage_bundle + + # Shared mode dispatch. In :copy mode ensures the destination directory + # exists; make_relative_symlink handles its own mkdir_p in :symlink mode. + def self.stage_file(src, dest, mode, copy_log_prefix) if mode == :copy - FileUtils.mkdir_p(bundle_dir) - FileUtils.cp(src_path, dest_file) - puts "[ReactOnRailsPro] Pre-seeded renderer cache: #{dest_file}" + FileUtils.mkdir_p(File.dirname(dest)) + FileUtils.cp(src, dest) + puts "[ReactOnRailsPro] #{copy_log_prefix}: #{dest}" else - make_relative_symlink(src_path, dest_file) + make_relative_symlink(src, dest) end end - private_class_method :stage_bundle + private_class_method :stage_file # RSC manifests are required when RSC is enabled — a missing manifest would cause # the renderer to fail at runtime with a hard-to-diagnose error. User-configured @@ -118,12 +125,7 @@ def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) end dest = File.join(bundle_dir, File.basename(expanded)) - if mode == :copy - FileUtils.cp(expanded, dest) - puts "[ReactOnRailsPro] Copied asset: #{dest}" - else - make_relative_symlink(expanded, dest) - end + stage_file(expanded, dest, mode, "Copied asset") end end private_class_method :stage_assets @@ -161,8 +163,18 @@ def self.make_relative_symlink(source, destination) "it may have been removed after mkdir_p (race with an external cleanup)." end relative_source_path = source_path.relative_path_from(destination_dir_real) - File.symlink(relative_source_path, destination) - puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" + # File.symlink raises Errno::EEXIST if the destination reappears between + # the rm_f above and this call (e.g. two Puma workers racing through + # AssetsPrecompile.call at boot). Treat a concurrent winner as success: + # both processes compute the same relative source, so the existing link + # is already correct for us. + begin + File.symlink(relative_source_path, destination) + puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" + rescue Errno::EEXIST + puts "[ReactOnRailsPro] Symlink already present at #{destination} " \ + "(concurrent creator won the race); leaving existing link." + end end private_class_method :make_relative_symlink 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 index 6a005223a4..69399f7bd9 100644 --- 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 @@ -35,7 +35,7 @@ def self.emit_deprecation_warning! # Private so it can only be invoked from specs via `send`; prevents accidental # reset from production code. def self.reset_deprecation_warned! - @deprecation_mutex.synchronize { @deprecation_warned = nil } + @deprecation_mutex.synchronize { @deprecation_warned = false } end private_class_method :reset_deprecation_warned! end diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index 7372526c30..7a1d1326cd 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -80,6 +80,15 @@ expect(File.symlink?(second_asset)).to be(true) expect(File.realpath(first_asset)).to eq(path_in_webpack_folder(asset_filename).to_s) end + + it "treats a concurrent Errno::EEXIST from File.symlink as success" do + # Simulates two processes racing through make_relative_symlink: the + # other process recreated the destination between rm_f and File.symlink, + # so our syscall raises EEXIST. The guard should swallow that instead + # of propagating. + allow(File).to receive(:symlink).and_raise(Errno::EEXIST) + expect { described_class.call(mode: :symlink) }.not_to raise_error + end end context "when mode is :copy and no env var is set in a non-dev/test environment" do From f27dafc42e7fe5d4c6286b36413d37d95d06eaaa Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 16:47:16 -1000 Subject: [PATCH 07/17] Fix stacked markdown link check --- .lychee.toml | 1 + docs/oss/building-features/node-renderer/js-configuration.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.lychee.toml b/.lychee.toml index 730f373ce7..cead55d605 100644 --- a/.lychee.toml +++ b/.lychee.toml @@ -85,6 +85,7 @@ exclude = [ '^https://(www\.)?estately\.com', # Returns 403 '^https://hiring\.careerbuilder\.com', # Returns 403 '^https://(www\.)?yourmechanic\.com', # Returns 403/503 + '^https://(www\.)?guavapass\.com', # TLS handshake failures from CI '^https?://(www\.)?hvmn\.com', # Returns 503 from CI '^https://(www\.)?hawaiichee\.com/?$', # Intermittent 500 from CI diff --git a/docs/oss/building-features/node-renderer/js-configuration.md b/docs/oss/building-features/node-renderer/js-configuration.md index d524c424b9..94b64357d4 100644 --- a/docs/oss/building-features/node-renderer/js-configuration.md +++ b/docs/oss/building-features/node-renderer/js-configuration.md @@ -58,7 +58,7 @@ Deprecated options: ### Testing example: -[spec/dummy/client/node-renderer.js](https://github.com/shakacode/react_on_rails/blob/main/react_on_rails_pro/spec/dummy/client/node-renderer.js) +[spec/dummy/client/node-renderer.js](../../../../react_on_rails_pro/spec/dummy/client/node-renderer.js) ### Simple example: From b42dc5214bc3e4a0db8f6b47474e93f04a019e24 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 17:04:26 -1000 Subject: [PATCH 08/17] Polish renderer cache staging review feedback --- react_on_rails/lib/react_on_rails/doctor.rb | 4 ++++ .../spec/lib/react_on_rails/doctor_spec.rb | 6 +++--- .../react_on_rails_pro/pre_seed_renderer_cache.rb | 13 +++++++------ react_on_rails_pro/lib/tasks/assets.rake | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 778fa2080c..23dd41abb0 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -68,6 +68,9 @@ class Doctor "Procfile.dev-static-assets", "Procfile.production", "Dockerfile", + "Dockerfile.production", + "Dockerfile.staging", + "Dockerfile.review", "bin/deploy", "bin/release", "bin/docker-entrypoint" @@ -2753,6 +2756,7 @@ def check_deprecated_renderer_cache_task matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| full_path = Rails.root.join(path) next false unless full_path.exist? + # Skip files that are 1 MB or larger; deploy scripts should be tiny. next false if full_path.size >= RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES full_path.read.include?(DEPRECATED_RENDERER_CACHE_TASK) diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 787c01576b..e9ae5e343d 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2421,12 +2421,12 @@ class << self end end - context "when a Dockerfile references the deprecated task" do + context "when a Dockerfile variant references the deprecated task" do let(:tmpdir) { Dir.mktmpdir } before do File.write( - File.join(tmpdir, "Dockerfile"), + File.join(tmpdir, "Dockerfile.production"), "RUN bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n" ) allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) @@ -2440,7 +2440,7 @@ class << self expect(warning_msgs).not_to be_empty suggestion_line = warning_msgs .flat_map { |m| m[:content].split("\n") } - .find { |line| line.include?("Dockerfile →") } + .find { |line| line.include?("Dockerfile.production →") } expect(suggestion_line).not_to be_nil expect(suggestion_line).to include("pre_seed_renderer_cache") expect(suggestion_line).not_to include("MODE=symlink") 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 index 661f7b7354..bb4d040526 100644 --- 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 @@ -56,11 +56,12 @@ def self.resolve_cache_dir(mode) # explicit env var in non-dev/test environments. def self.enforce_cache_dir_env_var!(mode) return unless mode == :copy + return if Rails.env.development? || Rails.env.test? + # Use a plain-Ruby check (no ActiveSupport .present?) so whitespace-only # values are treated as "not set" and the guard remains portable. return unless ENV.fetch("RENDERER_SERVER_BUNDLE_CACHE_PATH", "").strip.empty? && ENV.fetch("RENDERER_BUNDLE_PATH", "").strip.empty? - return if Rails.env.development? || Rails.env.test? raise ReactOnRailsPro::Error, <<~MSG.strip Pre-seeding the renderer cache in copy mode (#{Rails.env}) requires an explicit @@ -94,13 +95,13 @@ def self.stage_bundle(src_path, bundle_dir, bundle_hash, mode) # Shared mode dispatch. In :copy mode ensures the destination directory # exists; make_relative_symlink handles its own mkdir_p in :symlink mode. - def self.stage_file(src, dest, mode, copy_log_prefix) + def self.stage_file(src, dest, mode, log_prefix) if mode == :copy FileUtils.mkdir_p(File.dirname(dest)) FileUtils.cp(src, dest) - puts "[ReactOnRailsPro] #{copy_log_prefix}: #{dest}" + puts "[ReactOnRailsPro] #{log_prefix}: #{dest}" else - make_relative_symlink(src, dest) + make_relative_symlink(src, dest, log_prefix) end end private_class_method :stage_file @@ -134,7 +135,7 @@ def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) # if the process is killed between `rm_f` and `File.symlink` the destination # is briefly absent. In practice the renderer's 410→refetch retry at # request time recovers from a missing bundle, so the brief gap is benign. - def self.make_relative_symlink(source, destination) + def self.make_relative_symlink(source, destination, log_prefix) destination_dir = Pathname.new(destination).dirname FileUtils.mkdir_p(destination_dir) FileUtils.rm_f(destination) @@ -170,7 +171,7 @@ def self.make_relative_symlink(source, destination) # is already correct for us. begin File.symlink(relative_source_path, destination) - puts "[ReactOnRailsPro] Symlinked #{relative_source_path} to #{destination}" + puts "[ReactOnRailsPro] #{log_prefix}: #{relative_source_path} -> #{destination}" rescue Errno::EEXIST puts "[ReactOnRailsPro] Symlink already present at #{destination} " \ "(concurrent creator won the race); leaving existing link." diff --git a/react_on_rails_pro/lib/tasks/assets.rake b/react_on_rails_pro/lib/tasks/assets.rake index 0f25571db5..7789ed5e2f 100644 --- a/react_on_rails_pro/lib/tasks/assets.rake +++ b/react_on_rails_pro/lib/tasks/assets.rake @@ -10,7 +10,7 @@ namespace :react_on_rails_pro do raw_mode = "copy" if raw_mode.empty? valid_modes = ReactOnRailsPro::PreSeedRendererCache::VALID_MODES.map(&:to_s) unless valid_modes.include?(raw_mode) - abort "[ReactOnRailsPro] Unknown MODE=#{ENV.fetch('MODE', nil).inspect}. " \ + abort "[ReactOnRailsPro] Unknown MODE=#{raw_mode.inspect}. " \ "Expected one of: #{valid_modes.join(', ')}" end ReactOnRailsPro::PreSeedRendererCache.call(mode: raw_mode.to_sym) From 9012814be514efcac1181478d2e99e99caa5ec93 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 17:21:54 -1000 Subject: [PATCH 09/17] Polish renderer cache review follow-ups --- .../node-renderer/js-configuration.md | 2 +- react_on_rails/lib/react_on_rails/doctor.rb | 9 ++++---- .../spec/lib/react_on_rails/doctor_spec.rb | 21 +++++++++++++++++++ .../pre_seed_renderer_cache.rb | 6 ++++-- .../prepare_node_renderer_bundles.rb | 2 ++ .../spec/pre_seed_renderer_cache_spec.rb | 7 +++++++ 6 files changed, 39 insertions(+), 8 deletions(-) diff --git a/docs/oss/building-features/node-renderer/js-configuration.md b/docs/oss/building-features/node-renderer/js-configuration.md index 94b64357d4..16a5939f8f 100644 --- a/docs/oss/building-features/node-renderer/js-configuration.md +++ b/docs/oss/building-features/node-renderer/js-configuration.md @@ -58,7 +58,7 @@ Deprecated options: ### Testing example: -[spec/dummy/client/node-renderer.js](../../../../react_on_rails_pro/spec/dummy/client/node-renderer.js) +[spec/dummy/client/node-renderer.js](https://github.com/shakacode/react_on_rails/blob/b42dc5214bc3e4a0db8f6b47474e93f04a019e24/react_on_rails_pro/spec/dummy/client/node-renderer.js) ### Simple example: diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 23dd41abb0..d3c3da6787 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2756,8 +2756,8 @@ def check_deprecated_renderer_cache_task matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| full_path = Rails.root.join(path) next false unless full_path.exist? - # Skip files that are 1 MB or larger; deploy scripts should be tiny. - next false if full_path.size >= RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES + # Skip files larger than 1 MB; deploy scripts should be tiny. + next false if full_path.size > RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES full_path.read.include?(DEPRECATED_RENDERER_CACHE_TASK) end @@ -2775,10 +2775,9 @@ def check_deprecated_renderer_cache_task checker.add_warning("⚠️ Could not scan for deprecated renderer-cache task references: #{e.message}") end - # Dockerfile matches mean the user is building an image, so they want the - # copy-mode default (no MODE needed). Procfile/bin scripts mean same-filesystem - # runtime staging, which needs the symlink mode. def renderer_cache_migration_suggestion(path) + # Dockerfile* entries run while building an image, so copy mode is correct. + # Procfile/bin entries run in the deployed filesystem and need symlink mode. if path.start_with?("Dockerfile") "rake react_on_rails_pro:pre_seed_renderer_cache" else diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index e9ae5e343d..d31c44e1fc 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2481,6 +2481,27 @@ class << self expect(checker.messages.select { |m| m[:type] == :warning }).to be_empty end end + + context "when a deploy-script file is exactly the size gate" do + let(:tmpdir) { Dir.mktmpdir } + let(:script_content) do + "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n" + end + + before do + stub_const("ReactOnRails::Doctor::RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES", script_content.bytesize) + File.write(File.join(tmpdir, "Procfile"), script_content) + allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) + end + + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } + + it "still scans the file" do + doctor.send(:check_deprecated_renderer_cache_task) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("pre_stage_bundle_for_node_renderer") }).to be(true) + end + end end describe "check_base_package_imports" do 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 index bb4d040526..74388eae7a 100644 --- 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 @@ -89,7 +89,8 @@ def self.action_description(mode) def self.stage_bundle(src_path, bundle_dir, bundle_hash, mode) dest_file = File.join(bundle_dir, "#{bundle_hash}.js") - stage_file(src_path, dest_file, mode, "Pre-seeded renderer cache") + log_prefix = mode == :copy ? "Pre-seeded renderer cache" : "Pre-staged renderer cache" + stage_file(src_path, dest_file, mode, log_prefix) end private_class_method :stage_bundle @@ -126,7 +127,8 @@ def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) end dest = File.join(bundle_dir, File.basename(expanded)) - stage_file(expanded, dest, mode, "Copied asset") + log_prefix = mode == :copy ? "Copied asset" : "Symlinked asset" + stage_file(expanded, dest, mode, log_prefix) end end private_class_method :stage_assets 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 index 69399f7bd9..77313c239a 100644 --- 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 @@ -13,6 +13,8 @@ class PrepareNodeRenderBundles # warning per process. @deprecation_mutex = Mutex.new + # The deprecated rake task emits its own warning and calls PreSeedRendererCache + # directly; it does not set this one-time guard. See assets.rake for that path. def self.call emit_deprecation_warning! PreSeedRendererCache.call(mode: :symlink) diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index 7a1d1326cd..5792c5f2f3 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -81,6 +81,13 @@ expect(File.realpath(first_asset)).to eq(path_in_webpack_folder(asset_filename).to_s) end + it "logs symlink operations with symlink-specific labels" do + FileUtils.cp(fixture_path, path_in_webpack_folder(asset_filename)) + + expect { described_class.call(mode: :symlink) } + .to output(/Pre-staged renderer cache: .* -> .*Symlinked asset: .* ->/m).to_stdout + end + it "treats a concurrent Errno::EEXIST from File.symlink as success" do # Simulates two processes racing through make_relative_symlink: the # other process recreated the destination between rm_f and File.symlink, From de644db8d56cf9201901ab92a71d6973a295beaf Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 17:32:26 -1000 Subject: [PATCH 10/17] Add rescue-branch spec and resolve_cache_dir doc comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two items from the #3167 round-6 review that were not covered by 9012814be: - `doctor_spec.rb` — new context exercising the `rescue StandardError` branch in `check_deprecated_renderer_cache_task` by stubbing `File.read` to raise Errno::EIO for the target path. Confirms the rescue turns the error into a warning instead of crashing the doctor run. - `pre_seed_renderer_cache.rb` — one-line comment on `resolve_cache_dir` flagging the non-obvious side effect that it validates the cache-dir env var (and may raise) before resolving, so callers reading the signature alone do not miss the production-mode footgun guard. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../spec/lib/react_on_rails/doctor_spec.rb | 29 +++++++++++++++++++ .../pre_seed_renderer_cache.rb | 2 ++ .../spec/pre_seed_renderer_cache_spec.rb | 13 +++++++++ 3 files changed, 44 insertions(+) diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index d31c44e1fc..8d5e557a1a 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2502,6 +2502,35 @@ class << self expect(warning_msgs.any? { |m| m[:content].include?("pre_stage_bundle_for_node_renderer") }).to be(true) end end + + context "when reading a deploy-script file raises an unexpected error" do + let(:tmpdir) { Dir.mktmpdir } + let(:procfile_path) { File.join(tmpdir, "Procfile") } + + before do + File.write( + procfile_path, + "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n" + ) + allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) + # Simulate a filesystem error (e.g. transient EIO or a permissions race) + # during the file-read step so the rescue branch is exercised. + # Pathname#read delegates to File.read, so stubbing File.read for this + # specific path exercises the rescue without impacting other reads. + allow(File).to receive(:read).and_call_original + allow(File).to receive(:read).with(procfile_path).and_raise(Errno::EIO, "simulated read failure") + end + + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } + + it "captures the error as a warning instead of failing the doctor check" do + expect { doctor.send(:check_deprecated_renderer_cache_task) }.not_to raise_error + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? do |m| + m[:content].include?("Could not scan for deprecated renderer-cache task") + end).to be(true) + end + end end describe "check_base_package_imports" do 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 index 74388eae7a..31d6e80d3d 100644 --- 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 @@ -43,6 +43,8 @@ def self.call(mode: :copy) end end + # Validates the cache-dir env var (raises in production-like copy mode when + # unset) before resolving. See enforce_cache_dir_env_var! for the rationale. def self.resolve_cache_dir(mode) enforce_cache_dir_env_var!(mode) ReactOnRailsPro::Utils.resolve_renderer_cache_dir diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index 5792c5f2f3..b7d3101104 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -96,6 +96,19 @@ allow(File).to receive(:symlink).and_raise(Errno::EEXIST) expect { described_class.call(mode: :symlink) }.not_to raise_error end + + it "logs mode-accurate prefixes (Pre-staged / Symlinked) instead of copy-oriented wording" do + FileUtils.cp(fixture_path, path_in_webpack_folder(asset_filename)) + FileUtils.cp(fixture_path2, path_in_webpack_folder(asset_filename2)) + + accurate_symlink_logs = satisfy("uses mode-aware log prefixes") do |out| + out.match?(/Pre-staged renderer cache:.*->/) && + out.match?(/Symlinked asset:.*->/) && + !out.include?("Copied asset") && + !out.include?("Pre-seeded renderer cache") + end + expect { described_class.call(mode: :symlink) }.to output(accurate_symlink_logs).to_stdout + end end context "when mode is :copy and no env var is set in a non-dev/test environment" do From c1db2a76eb563dcde1e6bb2f74dffd0ad679ad53 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 17:39:28 -1000 Subject: [PATCH 11/17] Harden renderer cache symlink staging --- CHANGELOG.md | 2 +- .../node-renderer/js-configuration.md | 3 +- react_on_rails/lib/react_on_rails/doctor.rb | 6 ++-- .../pre_seed_renderer_cache.rb | 22 +++++++++++---- .../spec/pre_seed_renderer_cache_spec.rb | 28 +++++++++++++++++-- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec251a743..2381c492f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ #### Changed -- **[Pro]** **Unified renderer cache staging**: `ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)` is now the single entry point for staging the Node Renderer cache. Both modes produce the same `//.js` layout. The `react_on_rails_pro:pre_seed_renderer_cache` rake task accepts `MODE=copy` (default; Docker/image builds) or `MODE=symlink` (same-filesystem). `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, because the Node renderer's default lookup can differ from the Ruby side and would silently drop pre-seeded bundles in the wrong directory. The legacy `react_on_rails_pro:pre_stage_bundle_for_node_renderer` task and `ReactOnRailsPro::PrepareNodeRenderBundles` class remain as deprecated shims that emit a once-per-process warning and delegate to `mode: :symlink`. `react_on_rails:doctor` flags deploy scripts that still reference the deprecated task. +- **[Pro]** **Unified renderer cache staging**: `ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)` is now the single entry point for staging the Node Renderer cache. Both modes produce the same `//.js` layout. The `react_on_rails_pro:pre_seed_renderer_cache` rake task accepts `MODE=copy` (default; Docker/image builds) or `MODE=symlink` (same-filesystem). `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, because the Node renderer's default lookup can differ from the Ruby side and would silently drop pre-seeded bundles in the wrong directory. The legacy `react_on_rails_pro:pre_stage_bundle_for_node_renderer` task and `ReactOnRailsPro::PrepareNodeRenderBundles` class remain as deprecated shims that emit a once-per-process warning and delegate to `mode: :symlink`. `react_on_rails:doctor` flags deploy scripts that still reference the deprecated task. [PR 3167](https://github.com/shakacode/react_on_rails/pull/3167) by [justin808](https://github.com/justin808). #### Fixed diff --git a/docs/oss/building-features/node-renderer/js-configuration.md b/docs/oss/building-features/node-renderer/js-configuration.md index 16a5939f8f..5f94d8812d 100644 --- a/docs/oss/building-features/node-renderer/js-configuration.md +++ b/docs/oss/building-features/node-renderer/js-configuration.md @@ -58,7 +58,8 @@ Deprecated options: ### Testing example: -[spec/dummy/client/node-renderer.js](https://github.com/shakacode/react_on_rails/blob/b42dc5214bc3e4a0db8f6b47474e93f04a019e24/react_on_rails_pro/spec/dummy/client/node-renderer.js) +The repository's dummy app keeps a full integration-test launcher at +`react_on_rails_pro/spec/dummy/client/node-renderer.js`. ### Simple example: diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index d3c3da6787..f0643971f8 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2776,8 +2776,10 @@ def check_deprecated_renderer_cache_task end def renderer_cache_migration_suggestion(path) - # Dockerfile* entries run while building an image, so copy mode is correct. - # Procfile/bin entries run in the deployed filesystem and need symlink mode. + # Dockerfile* entries are RUN steps during image build, so copy mode bakes + # the cache into the layer. Procfile, bin/*, and other runtime scripts run + # inside the already-booted container or dyno, where both the app and + # renderer share the same filesystem, so symlink mode is correct. if path.start_with?("Dockerfile") "rake react_on_rails_pro:pre_seed_renderer_cache" else 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 index 31d6e80d3d..e8e6618c29 100644 --- 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 @@ -62,6 +62,8 @@ def self.enforce_cache_dir_env_var!(mode) # Use a plain-Ruby check (no ActiveSupport .present?) so whitespace-only # values are treated as "not set" and the guard remains portable. + # RENDERER_BUNDLE_PATH remains accepted for compatibility, but new deploys + # should migrate to RENDERER_SERVER_BUNDLE_CACHE_PATH. return unless ENV.fetch("RENDERER_SERVER_BUNDLE_CACHE_PATH", "").strip.empty? && ENV.fetch("RENDERER_BUNDLE_PATH", "").strip.empty? @@ -168,19 +170,27 @@ def self.make_relative_symlink(source, destination, log_prefix) "it may have been removed after mkdir_p (race with an external cleanup)." end relative_source_path = source_path.relative_path_from(destination_dir_real) - # File.symlink raises Errno::EEXIST if the destination reappears between - # the rm_f above and this call (e.g. two Puma workers racing through - # AssetsPrecompile.call at boot). Treat a concurrent winner as success: - # both processes compute the same relative source, so the existing link - # is already correct for us. begin File.symlink(relative_source_path, destination) puts "[ReactOnRailsPro] #{log_prefix}: #{relative_source_path} -> #{destination}" rescue Errno::EEXIST + replace_existing_symlink(destination, relative_source_path, log_prefix) + end + end + private_class_method :make_relative_symlink + + def self.replace_existing_symlink(destination, relative_source_path, log_prefix) + if File.symlink?(destination) && File.readlink(destination) == relative_source_path.to_s puts "[ReactOnRailsPro] Symlink already present at #{destination} " \ "(concurrent creator won the race); leaving existing link." + return end + + FileUtils.rm_f(destination) + File.symlink(relative_source_path, destination) + puts "[ReactOnRailsPro] #{log_prefix}: #{relative_source_path} -> #{destination} " \ + "(replaced stale symlink)" end - private_class_method :make_relative_symlink + private_class_method :replace_existing_symlink end end diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index b7d3101104..33f45dcc4b 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -88,15 +88,37 @@ .to output(/Pre-staged renderer cache: .* -> .*Symlinked asset: .* ->/m).to_stdout end - it "treats a concurrent Errno::EEXIST from File.symlink as success" do + it "treats a concurrent matching symlink as success" do # Simulates two processes racing through make_relative_symlink: the # other process recreated the destination between rm_f and File.symlink, # so our syscall raises EEXIST. The guard should swallow that instead # of propagating. - allow(File).to receive(:symlink).and_raise(Errno::EEXIST) + allow(File).to receive(:symlink).and_wrap_original do |original, source, destination| + original.call(source, destination) + raise Errno::EEXIST + end + expect { described_class.call(mode: :symlink) }.not_to raise_error end + it "replaces a mismatched symlink created by a concurrent stage" do + stale_source = "stale-server-bundle.js" + created_stale_link = false + allow(File).to receive(:symlink).and_wrap_original do |original, source, destination| + unless created_stale_link + created_stale_link = true + original.call(stale_source, destination) + raise Errno::EEXIST + end + + original.call(source, destination) + end + + expect { described_class.call(mode: :symlink) }.to output(/replaced stale symlink/).to_stdout + dest_file = File.join(bundle_dir, "#{bundle_hash}.js") + expect(File.realpath(dest_file)).to eq(server_bundle_path) + end + it "logs mode-accurate prefixes (Pre-staged / Symlinked) instead of copy-oriented wording" do FileUtils.cp(fixture_path, path_in_webpack_folder(asset_filename)) FileUtils.cp(fixture_path2, path_in_webpack_folder(asset_filename2)) @@ -127,7 +149,7 @@ ENV["RENDERER_SERVER_BUNDLE_CACHE_PATH"] = tmpdir expect { described_class.call(mode: :copy) }.not_to raise_error ensure - FileUtils.rm_rf(tmpdir) if tmpdir + FileUtils.rm_rf(tmpdir) ENV.delete("RENDERER_SERVER_BUNDLE_CACHE_PATH") end From acd4bbdd259a528f8add01821220390533e2ec03 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 17:51:50 -1000 Subject: [PATCH 12/17] Polish renderer cache review follow-ups --- .../spec/lib/react_on_rails/doctor_spec.rb | 15 +++++++++------ .../react_on_rails_pro/pre_seed_renderer_cache.rb | 3 ++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 8d5e557a1a..79562f2e26 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2512,13 +2512,16 @@ class << self procfile_path, "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n" ) - allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) + root_path = Pathname.new(tmpdir) + allow(Rails).to receive(:root).and_return(root_path) + # Simulate a filesystem error (e.g. transient EIO or a permissions race) - # during the file-read step so the rescue branch is exercised. - # Pathname#read delegates to File.read, so stubbing File.read for this - # specific path exercises the rescue without impacting other reads. - allow(File).to receive(:read).and_call_original - allow(File).to receive(:read).with(procfile_path).and_raise(Errno::EIO, "simulated read failure") + # on the actual Pathname receiver used by the doctor scan. + failing_procfile = instance_double(Pathname) + allow(failing_procfile).to receive_messages(exist?: true, size: File.size(procfile_path)) + allow(failing_procfile).to receive(:read).and_raise(Errno::EIO, "simulated read failure") + allow(root_path).to receive(:join).and_call_original + allow(root_path).to receive(:join).with("Procfile").and_return(failing_procfile) end after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } 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 index e8e6618c29..59966bd213 100644 --- 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 @@ -119,12 +119,13 @@ def self.stage_file(src, dest, mode, log_prefix) # against Rails.root to match how RendererCacheHelpers.required_rsc_asset_paths # builds its Set. def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) + action_desc = action_description(mode) 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(mode)} the renderer cache." + "Build your bundles before #{action_desc} the renderer cache." end warn "[ReactOnRailsPro] Asset not found #{asset_path}" next From 25826d08af3f8108e5d0b72607327b1b9f6dadd5 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 17:58:40 -1000 Subject: [PATCH 13/17] Harden renderer cache symlink races --- react_on_rails/lib/react_on_rails/doctor.rb | 1 + .../pre_seed_renderer_cache.rb | 17 +++++++++++--- .../prepare_node_renderer_bundles.rb | 1 + .../spec/pre_seed_renderer_cache_spec.rb | 22 +++++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index f0643971f8..03fb023f56 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2770,6 +2770,7 @@ def check_deprecated_renderer_cache_task The unified 'pre_seed_renderer_cache' task uses MODE=copy by default (for Docker/image builds) and MODE=symlink for same-filesystem workflows. + This scan also matches comments; remove stale mentions after migrating. MSG rescue StandardError => e checker.add_warning("⚠️ Could not scan for deprecated renderer-cache task references: #{e.message}") 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 index 59966bd213..d0fd672ca6 100644 --- 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 @@ -144,8 +144,6 @@ def self.stage_assets(assets, bundle_dir, rsc_required_paths, mode) # request time recovers from a missing bundle, so the brief gap is benign. def self.make_relative_symlink(source, destination, log_prefix) destination_dir = Pathname.new(destination).dirname - FileUtils.mkdir_p(destination_dir) - FileUtils.rm_f(destination) # Canonicalize both sides so paths like /var -> /private/var do not # produce broken relative symlinks when the cache dir comes from tmpdir. @@ -162,6 +160,7 @@ def self.make_relative_symlink(source, destination, log_prefix) "it does not exist or is a dangling symlink. " \ "Rebuild your bundles before staging the renderer cache." end + FileUtils.mkdir_p(destination_dir) destination_dir_real = begin destination_dir.realpath @@ -171,6 +170,7 @@ def self.make_relative_symlink(source, destination, log_prefix) "it may have been removed after mkdir_p (race with an external cleanup)." end relative_source_path = source_path.relative_path_from(destination_dir_real) + FileUtils.rm_f(destination) begin File.symlink(relative_source_path, destination) puts "[ReactOnRailsPro] #{log_prefix}: #{relative_source_path} -> #{destination}" @@ -188,10 +188,21 @@ def self.replace_existing_symlink(destination, relative_source_path, log_prefix) end FileUtils.rm_f(destination) - File.symlink(relative_source_path, destination) + begin + File.symlink(relative_source_path, destination) + rescue Errno::EEXIST + return if matching_symlink?(destination, relative_source_path) + + raise + end puts "[ReactOnRailsPro] #{log_prefix}: #{relative_source_path} -> #{destination} " \ "(replaced stale symlink)" end private_class_method :replace_existing_symlink + + def self.matching_symlink?(destination, relative_source_path) + File.symlink?(destination) && File.readlink(destination) == relative_source_path.to_s + end + private_class_method :matching_symlink? 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 index 77313c239a..87b9fa5fd9 100644 --- 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 @@ -12,6 +12,7 @@ class PrepareNodeRenderBundles # (e.g. multiple Puma workers invoking the shim at boot) still see exactly one # warning per process. @deprecation_mutex = Mutex.new + @deprecation_warned = false # The deprecated rake task emits its own warning and calls PreSeedRendererCache # directly; it does not set this one-time guard. See assets.rake for that path. diff --git a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb index 33f45dcc4b..acbcbdba1a 100644 --- a/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/pre_seed_renderer_cache_spec.rb @@ -119,6 +119,28 @@ expect(File.realpath(dest_file)).to eq(server_bundle_path) end + it "treats a concurrent matching symlink during stale replacement as success" do + stale_source = "stale-server-bundle.js" + symlink_calls = 0 + allow(File).to receive(:symlink).and_wrap_original do |original, source, destination| + symlink_calls += 1 + case symlink_calls + when 1 + original.call(stale_source, destination) + raise Errno::EEXIST + when 2 + original.call(source, destination) + raise Errno::EEXIST + else + original.call(source, destination) + end + end + + expect { described_class.call(mode: :symlink) }.not_to raise_error + dest_file = File.join(bundle_dir, "#{bundle_hash}.js") + expect(File.realpath(dest_file)).to eq(server_bundle_path) + end + it "logs mode-accurate prefixes (Pre-staged / Symlinked) instead of copy-oriented wording" do FileUtils.cp(fixture_path, path_in_webpack_folder(asset_filename)) FileUtils.cp(fixture_path2, path_in_webpack_folder(asset_filename2)) From 95b271ccefde914f950f1676a7bc81fca489d786 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 18:08:12 -1000 Subject: [PATCH 14/17] Reuse symlink match helper --- .../lib/react_on_rails_pro/pre_seed_renderer_cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index d0fd672ca6..86006ea46b 100644 --- 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 @@ -181,7 +181,7 @@ def self.make_relative_symlink(source, destination, log_prefix) private_class_method :make_relative_symlink def self.replace_existing_symlink(destination, relative_source_path, log_prefix) - if File.symlink?(destination) && File.readlink(destination) == relative_source_path.to_s + if matching_symlink?(destination, relative_source_path) puts "[ReactOnRailsPro] Symlink already present at #{destination} " \ "(concurrent creator won the race); leaving existing link." return From b4cf8754a96e73b7323ab60f0ed3303d5a18e592 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 19:00:35 -1000 Subject: [PATCH 15/17] Address renderer cache review follow-ups --- docs/oss/building-features/node-renderer/js-configuration.md | 2 +- .../spec/react_on_rails_pro/support/mock_block_helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/oss/building-features/node-renderer/js-configuration.md b/docs/oss/building-features/node-renderer/js-configuration.md index 6c474a09ce..2df3f8241b 100644 --- a/docs/oss/building-features/node-renderer/js-configuration.md +++ b/docs/oss/building-features/node-renderer/js-configuration.md @@ -61,7 +61,7 @@ Deprecated options: ### Testing example: The repository's dummy app keeps a full integration-test launcher at -`react_on_rails_pro/spec/dummy/renderer/node-renderer.js`. +[`react_on_rails_pro/spec/dummy/renderer/node-renderer.js`](https://github.com/shakacode/react_on_rails/blob/main/react_on_rails_pro/spec/dummy/renderer/node-renderer.js). ### Simple example: diff --git a/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb b/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb index aaae4e6821..1e1330e562 100644 --- a/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb +++ b/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb @@ -10,7 +10,7 @@ module MockBlockHelper # testing_method_taking_block(&mocked_block.block) # expect(mocked_block).to have_received(:call).with(1, 2, 3) def mock_block(&block) - double("BlockMock").tap do |mock| # rubocop:disable RSpec/VerifiedDoubles + double("BlockMock").tap do |mock| allow(mock).to receive(:call) do |*args, &inner_block| block&.call(*args, &inner_block) end From 0752feb9366f71e83ef85c509c0c4a16224ff547 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 19:16:11 -1000 Subject: [PATCH 16/17] Fix Pro lint block helper --- .../support/mock_block_helper.rb | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb b/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb index 1e1330e562..f71c8221d7 100644 --- a/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb +++ b/react_on_rails_pro/spec/react_on_rails_pro/support/mock_block_helper.rb @@ -1,6 +1,20 @@ # frozen_string_literal: true module MockBlockHelper + class BlockMock + def initialize(callback) + @callback = callback + end + + def call(*args, &inner_block) + @callback&.call(*args, &inner_block) + end + + def block + method(:call).to_proc + end + end + # This is a class that can be used to mock a block. # It can be used to test that a block is called with the correct arguments. # @@ -10,13 +24,8 @@ module MockBlockHelper # testing_method_taking_block(&mocked_block.block) # expect(mocked_block).to have_received(:call).with(1, 2, 3) def mock_block(&block) - double("BlockMock").tap do |mock| - allow(mock).to receive(:call) do |*args, &inner_block| - block&.call(*args, &inner_block) - end - def mock.block - method(:call).to_proc - end + BlockMock.new(block).tap do |mock| + allow(mock).to receive(:call).and_call_original end end end From f511867a78e88f446af0ad26a7a3069529ffd76b Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 22 Apr 2026 19:38:17 -1000 Subject: [PATCH 17/17] Skip directories in renderer cache task scan --- react_on_rails/lib/react_on_rails/doctor.rb | 2 +- .../spec/lib/react_on_rails/doctor_spec.rb | 27 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 9f9418fbc2..e9d9375338 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2778,7 +2778,7 @@ def check_deprecated_renderer_cache_task # already been migrated but still references the old name in a comment. matches = RENDERER_CACHE_DEPLOY_SCRIPT_PATHS.select do |path| full_path = Rails.root.join(path) - next false unless full_path.exist? + next false unless full_path.file? # Skip files larger than 1 MB; deploy scripts should be tiny. next false if full_path.size > RENDERER_CACHE_DEPLOY_SCRIPT_MAX_BYTES diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 695a5f1ac6..e909b7f50c 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2531,6 +2531,31 @@ class << self end end + context "when a configured deploy-script path is a directory" do + let(:tmpdir) { Dir.mktmpdir } + + before do + FileUtils.mkdir_p(File.join(tmpdir, "bin/deploy")) + File.write( + File.join(tmpdir, "Procfile"), + "web: bundle exec rake react_on_rails_pro:pre_stage_bundle_for_node_renderer\n" + ) + allow(Rails).to receive(:root).and_return(Pathname.new(tmpdir)) + end + + after { FileUtils.remove_entry(tmpdir) if File.directory?(tmpdir) } + + it "skips the directory and continues scanning real files" do + doctor.send(:check_deprecated_renderer_cache_task) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + + expect(warning_msgs.any? { |m| m[:content].include?("Procfile") }).to be(true) + expect(warning_msgs.none? do |m| + m[:content].include?("Could not scan for deprecated renderer-cache task") + end).to be(true) + end + end + context "when a deploy-script file exceeds the size gate" do let(:tmpdir) { Dir.mktmpdir } @@ -2590,7 +2615,7 @@ class << self # Simulate a filesystem error (e.g. transient EIO or a permissions race) # on the actual Pathname receiver used by the doctor scan. failing_procfile = instance_double(Pathname) - allow(failing_procfile).to receive_messages(exist?: true, size: File.size(procfile_path)) + allow(failing_procfile).to receive_messages(file?: true, size: File.size(procfile_path)) allow(failing_procfile).to receive(:read).and_raise(Errno::EIO, "simulated read failure") allow(root_path).to receive(:join).and_call_original allow(root_path).to receive(:join).with("Procfile").and_return(failing_procfile)