Fix default layout handling for render component

[justin808]

Summary

• restore the implicit current-layout behavior for render component: when no explicit layout: option is passed
• preserve explicit layout overrides such as layout: false
• add regression coverage for the render-option contract and controller-level layout behavior
• update the changelog for issue Rails layout not applied when using render :component #1356

Closes #1356.

Testing

• mise exec ruby@3.2.9 -- env BUNDLE_PATH=/Users/justin/codex/react-rails/repo/vendor/bundle bundle exec ruby -Itest test/react/rails/railtie_test.rb
• mise exec ruby@3.2.9 -- env BUNDLE_PATH=/Users/justin/codex/react-rails/repo/vendor/bundle bundle exec ruby -Itest test/react/rails/component_renderer_test.rb
• mise exec ruby@3.2.9 -- env BUNDLE_GEMFILE=/Users/justin/codex/react-rails/issue-1356/LintingGemfile BUNDLE_PATH=/Users/justin/codex/react-rails/issue-1356/vendor/bundle bundle exec rubocop lib/react/rails/railtie.rb test/react/rails/railtie_test.rb test/react/rails/component_renderer_test.rb

[claude]

PR Review: Fix default layout handling for render component

This is a well-scoped fix for #1356. The logic is correct and the test coverage is solid. A few observations below.

---

Core logic (lib/react/rails/railtie.rb)

Concern: send used to call a private method in production code

render_options = Railtie.send(:component_render_options, options, html)

Using .send to invoke a private_class_method from outside the class is a code smell — it signals the method is internal while simultaneously bypassing that signal. Since component_render_options is called from a block that runs in controller context (not inside Railtie itself), it effectively has to be reachable from outside.

Two cleaner alternatives:

1. Remove it from private_class_method — it's part of the renderer contract now, not a pure implementation detail.
2. Inline the logic — it's only three lines, which avoids the false privacy signal entirely:

   ActionController::Renderers.add :component do |component_name, options|
     renderer = ::React::Rails::ControllerRenderer.new(controller: self)
     html = renderer.call(component_name, options)
     render_options = options.merge(inline: html)
     render_options = render_options.merge(layout: true) unless render_options.key?(:layout)
     render(render_options)
   end

The current approach does work and has the benefit of making the method unit-testable in railtie_test.rb, which is a reasonable trade-off. But it's worth a deliberate decision either way.

---

Tests

Minor gap: no test for a named layout override

The railtie_test covers layout: false, but not layout: "some_layout". Since the intent is to preserve any explicit layout, one more case would be cheap to add and would make the contract explicit:

test "component render options preserve an explicit named layout" do
  render_options = React::Rails::Railtie.send(:component_render_options, { layout: "admin" }, "<div/>")
  assert_equal "admin", render_options[:layout]
end

FakeRenderer constant placement

Defining FakeRenderer inside ComponentRendererTest is fine, but because Ruby hoists inner class constants, it becomes ComponentRendererTest::FakeRenderer. If another test file later defines a similarly named constant it could conflict. A top-level name like FakeComponentRenderer or wrapping it in a module would be more explicit. Not a blocker.

---

What's good

• The fix is correct: layout: true is exactly how Rails applies the controller's default layout, and key?(:layout) correctly distinguishes an explicitly-passed nil/false/name from no option at all.
• ControllerRenderer#call slices recognized keys (options.slice(:data, :aria, :tag, ...)) so the :layout option never bleeds into the component render — no accidental side-effects there.
• The two-level test approach (unit tests in railtie_test for the extracted method + controller integration in component_renderer_test) is the right pattern for this kind of change.
• The changelog entry and issue reference are clear.

---

Overall this is a clean, low-risk fix. The main point worth discussing before merge is whether to keep the private_class_method + send pattern or inline/expose the method.

[claude]

Code Review

Overall, this is a clean, well-scoped fix for the Rails 8 layout regression. The approach is correct and the test coverage is solid. A few notes below.

---

lib/react/rails/railtie.rb

The core fix is correct. Using render_options.key?(:layout) to distinguish between "no layout specified" and "layout explicitly set to false/nil" is the right approach — a truthiness check would incorrectly treat layout: false as absent.

Minor: component_render_options is not in private_class_method

The existing helpers versioned_assets_for and versioned_assets? are private class methods. component_render_options is currently public, which is intentional for direct unit testing (consistent with how append_react_build_to_assets_version! is tested). This is fine, but worth a deliberate decision since it becomes part of the public API surface.

Edge case: layout: nil

If a caller passes layout: nil, key?(:layout) returns true and the method returns { layout: nil } rather than forcing layout: true. In Rails, layout: nil and layout: true both resolve to the controller layout, so this is functionally equivalent — but it's a subtle distinction worth noting in a comment or doc if this is intentional.

---

test/react/rails/railtie_test.rb

Nitpick: loose assertions on layout value

# current
assert render_options[:layout]          # passes for any truthy value
refute render_options[:layout]          # passes for nil too, not just false

Prefer explicit equality checks to pin down the exact returned values:

assert_equal true,  render_options[:layout]   # default case
assert_equal false, render_options[:layout]   # explicit layout: false

This makes the contract of component_render_options explicit and catches regressions if the return value changes type.

---

test/react/rails/component_renderer_test.rb

FakeRenderer constant placement

Defining a named constant inside the test class (ComponentRendererTest::FakeRenderer) is fine for namespacing, but a Struct with a do...end block at the class level is a little unusual. Consider a plain inner class or an anonymous struct assigned to a local for slightly cleaner read. Not a blocker.

Missing integration test: named layout

The unit tests cover layout: "admin", but the integration test only covers the default and layout: false cases. Adding a test where the controller action renders with layout: "admin" would round out the coverage and give confidence the full render path works end-to-end for named overrides.

RuboCop suppression

The # rubocop:disable Minitest/MultipleAssertions comments are reasonable here — the assertions are tightly related and splitting them into separate tests would hurt readability. Alternatively, the project could configure a higher threshold for this cop in .rubocop.yml for test files.

---

CHANGELOG.md

Other entries include a PR link and author attribution (e.g., [PR 1345](https://github.com/reactjs/react-rails/pull/1345) by [elektronaut](...)). For consistency:

- Preserve default controller layouts for `render component:` after the Rails 8 render pipeline change.
  [PR #1418](https://github.com/reactjs/react-rails/pull/1418) by [justin808](https://github.com/justin808).
  Fixes [#1356](https://github.com/reactjs/react-rails/issues/1356).

---

Summary

Area	Status
Bug fix correctness	✅ Correct — key? guard handles layout: false properly
Unit test coverage	✅ Good — all three option combinations tested
Integration test coverage	✅ Good — could add named layout case
Assertion precision	⚠️ Minor — prefer assert_equal over truthiness checks
Changelog format	⚠️ Minor — add PR link + author attribution
Security / performance	✅ No concerns

The fix is sound and ready to merge once the minor items above are addressed (or consciously left as-is).