Skip to content

Add optional compatibility module for capture/form/turbo compatibility fixes #1650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,34 @@ jobs:
include:
- rails_version: "5.2.6"
ruby_version: "2.7"
mode: "capture_patch_enabled"
- rails_version: "6.0.4.4"
ruby_version: "2.7"
mode: "capture_patch_enabled"
- rails_version: "6.1.4.4"
ruby_version: "2.7"
mode: "capture_patch_enabled"
- rails_version: "7.0.2.3"
ruby_version: "3.0"
mode: "capture_patch_enabled"
- rails_version: "main"
ruby_version: "3.1"
mode: "capture_patch_enabled"
- rails_version: "5.2.6"
ruby_version: "2.7"
mode: "capture_patch_disabled"
- rails_version: "6.0.4.4"
ruby_version: "2.7"
mode: "capture_patch_disabled"
- rails_version: "6.1.4.4"
ruby_version: "2.7"
mode: "capture_patch_disabled"
- rails_version: "7.0.2.3"
ruby_version: "3.0"
mode: "capture_patch_disabled"
- rails_version: "main"
ruby_version: "3.1"
mode: "capture_patch_disabled"
steps:
- uses: actions/checkout@master
- name: Setup Ruby
Expand All @@ -56,6 +76,7 @@ jobs:
RAISE_ON_WARNING: 1
MEASURE_COVERAGE: true
RAILS_VERSION: ${{ matrix.rails_version }}
CAPTURE_PATCH_ENABLED: ${{ matrix.mode == 'capture_patch_enabled' && 'true' || 'false' }}
- name: Upload coverage results
uses: actions/upload-artifact@master
if: always()
Expand Down
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ nav_order: 5

## main

* Add experimental `config.view_component.capture_compatibility_patch_enabled` option resolving rendering issues related to forms, capture, turbo frames, etc.

*Blake Williams*

* Add `#content?` method that indicates if content has been passed to component.

*Joel Hawksley*
Expand Down
1 change: 1 addition & 0 deletions lib/view_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module ViewComponent
extend ActiveSupport::Autoload

autoload :Base
autoload :CaptureCompatibility
autoload :Compiler
autoload :CompileCache
autoload :ComponentError
Expand Down
39 changes: 39 additions & 0 deletions lib/view_component/capture_compatibility.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# frozen_string_literal: true

module ViewComponent
# CaptureCompatibility is a module that patches #capture to fix issues
# related to ViewComponent and functionality that relies on `capture`
# like forms, capture itself, turbo frames, etc.
#
# This underlying incompatibility with ViewComponent and capture is
# that several features like forms keep a reference to the primary
# `ActionView::Base` instance which has its own @output_buffer. When
# `#capture` is called on the original `ActionView::Base` instance while
# evaluating a block from a ViewComponent the @output_buffer is overridden
# in the ActionView::Base instance, and *not* the component. This results
# in a double render due to `#capture` implementation details.
#
# To resolve the issue, we override `#capture` so that we can delegate
# the `capture` logic to the ViewComponent that created the block.
module CaptureCompatibility
def self.included(base)
base.class_eval do
alias_method :original_capture, :capture
end

base.prepend(InstanceMethods)
end

module InstanceMethods
def capture(*args, &block)
block_context = block.binding.receiver

if block_context.class < ActionView::Base && block_context != self
block_context.original_capture(*args, &block)
else
original_capture(*args, &block)
end
end
end
end
end
8 changes: 8 additions & 0 deletions lib/view_component/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ class Engine < Rails::Engine # :nodoc:
end
end

# :nocov:
initializer "view_component.enable_capture_patch" do |app|
ActiveSupport.on_load(:view_component) do
ActionView::Base.include(ViewComponent::CaptureCompatibility) if app.config.view_component.capture_compatibility_patch_enabled
end
end
# :nocov:

initializer "view_component.set_autoload_paths" do |app|
options = app.config.view_component

Expand Down
3 changes: 3 additions & 0 deletions test/sandbox/app/components/form_partial_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= form_for Post.new, url: "/" do |form| %>
<%= render "forms/label", form: form %>
<% end %>
4 changes: 4 additions & 0 deletions test/sandbox/app/components/form_partial_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# frozen_string_literal: true

class FormPartialComponent < ViewComponent::Base
end
3 changes: 3 additions & 0 deletions test/sandbox/app/views/forms/_label.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= form.label :published do %>
<%= form.check_box :published %>
<% end %>
1 change: 1 addition & 0 deletions test/sandbox/config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
config.view_component.render_monkey_patch_enabled = true
config.view_component.show_previews_source = true
config.view_component.test_controller = "IntegrationExamplesController"
config.view_component.capture_compatibility_patch_enabled = ENV["CAPTURE_PATCH_ENABLED"] == "true"

# Tell Action Mailer not to deliver emails to the real world.
# The :test delivery method accumulates sent emails in the
Expand Down
20 changes: 12 additions & 8 deletions test/sandbox/test/action_view_compatibility_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,39 @@

class ViewComponent::ActionViewCompatibilityTest < ViewComponent::TestCase
def test_renders_form_for_labels_with_block_correctly
skip

skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true"
render_inline(FormForComponent.new)

assert_selector("form > div > label > input")
refute_selector("form > div > input")
end

def test_renders_form_with_labels_with_block_correctly
skip

skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true"
render_inline(FormWithComponent.new)

assert_selector("form > div > label > input")
refute_selector("form > div > input")
end

def test_form_without_compatibility_does_not_raise
skip

skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true"
render_inline(IncompatibleFormComponent.new)

# Bad selector should be present, at least until fixed upstream or included by default
refute_selector("form > div > input")
end

def test_helper_with_content_tag
skip
def test_form_with_partial_render_works
skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true"
render_inline(FormPartialComponent.new)

# Bad selector should be present, at least until fixed upstream or included by default
refute_selector("form > div > input")
end

def test_helper_with_content_tag
skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true"
render_inline(ContentTagComponent.new)
assert_selector("div > p")
end
Expand Down