[codex] Fix Propshaft asset version cache busting

[justin808]

Summary

Fix the react_rails.bust_cache initializer so it can update the asset version for both Sprockets-style asset environments and Propshaft assemblies.

Root Cause

react-rails treated app.assets like a Sprockets environment and wrote version directly on it. In Propshaft 1.3.1, app.assets is a Propshaft::Assembly, and the writable version lives on app.assets.config, which caused boot to fail with undefined method 'version'.

Changes

• resolve the version-bearing object before appending the React build suffix
• keep the existing Sprockets behavior intact
• add regression tests for both direct version objects and Propshaft-style assemblies

Validation

• PATH=$HOME/.local/share/mise/installs/ruby/3.2.9/bin:$PATH bundle exec ruby -Itest test/react/rails/railtie_test.rb
• PATH=$HOME/.local/share/mise/installs/ruby/3.2.9/bin:$PATH BUNDLE_GEMFILE=gemfiles/propshaft.gemfile bundle exec ruby -Itest test/react/rails/railtie_test.rb

Fixes #1358

[claude]

Code Review

This is a clean, well-scoped fix for the Propshaft boot crash (fixes #1358). The duck-typing approach is the right call for supporting multiple asset pipeline backends. A few observations:

---

Positive

• Good decomposition — extracting the logic into named class methods with a bang suffix (append_react_build_to_assets_version!) is idiomatic Ruby and improves readability.
• Duck typing is the right approach — using respond_to? rather than is_a? checks avoids tight coupling to Sprockets or Propshaft internals, which will survive future upstream changes.
• Tests cover the two main paths — Sprockets-style direct version and Propshaft-style assembly with a nested config.

---

Issues & Suggestions

1. Missing test: nil assets

append_react_build_to_assets_version!(nil, "development") should be a silent no-op. Right now the code handles this correctly (via versioned_assets_for returning nil), but there's no test asserting that. Without it, a future refactor could accidentally introduce a NoMethodError on nil.

test "cache busting is a no-op when assets is nil" do
  assert_nothing_raised do
    React::Rails::Railtie.append_react_build_to_assets_version!(nil, "development")
  end
end

2. Missing test: assets with no resolvable version

Similarly, if assets doesn't respond to :version or :version= and its .config (if any) doesn't either, the method should silently return. Worth adding:

test "cache busting is a no-op when assets has no version" do
  assets = Object.new  # no version, no config
  assert_nothing_raised do
    React::Rails::Railtie.append_react_build_to_assets_version!(assets, "development")
  end
end

3. append_react_build_to_assets_version! is public but is an internal implementation detail

versioned_assets_for and versioned_assets? are correctly marked private_class_method, but append_react_build_to_assets_version! is left public (presumably to enable direct testing). This works, but it exposes an internal detail as part of the public API surface.

A small comment near the method declaration (e.g., # Exposed as a public method for testability) or a # :nodoc: would make the intent clear to future contributors.

4. Idempotency: multiple calls compound the version string

If the initializer runs more than once (e.g., in certain test harness or Spring reloader scenarios), each call appends another -react-development suffix. This was true of the old code too so it's not a regression, but it's worth a brief comment or a guard:

versioned_assets.version = [versioned_assets.version, "react-#{react_build}"].compact.join("-")
# Note: not idempotent — repeated calls keep appending. This is acceptable because
# Rails initializers run exactly once per process.

---

Minor nit

In versioned_assets?:

def self.versioned_assets?(assets)
  assets&.respond_to?(:version) && assets.respond_to?(:version=)
end

The first clause uses the safe-navigation &. (handles nil), but the second doesn't need to because short-circuit evaluation already protects it when assets is nil. This is correct as-is, but for consistency you could write both with &. or drop the & on the first — the current mixed style is slightly surprising on first read.

---

Summary

The fix itself is correct and handles the Propshaft case cleanly. The main gap is test coverage for edge-case inputs (nil, unrecognized objects). Addressing those would make this production-ready without concern.