diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9b69d393..038751dab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,46 +31,18 @@ jobs: include: - ruby_version: "3.0" rails_version: "6.1" - mode: "capture_patch_enabled" - - ruby_version: "3.0" - rails_version: "6.1" - mode: "capture_patch_disabled" - - ruby_version: "3.1" - rails_version: "7.0" - mode: "capture_patch_enabled" - ruby_version: "3.1" rails_version: "7.0" - mode: "capture_patch_disabled" - ruby_version: "3.2" rails_version: "7.1" - mode: "capture_patch_enabled" - - ruby_version: "3.2" - rails_version: "7.1" - mode: "capture_patch_disabled" - - ruby_version: "3.3" - rails_version: "7.2" - mode: "capture_patch_disabled" - ruby_version: "3.3" rails_version: "7.2" - mode: "capture_patch_enabled" - - ruby_version: "3.3" - rails_version: "8.0" - mode: "capture_patch_disabled" - ruby_version: "3.3" rails_version: "8.0" - mode: "capture_patch_enabled" - ruby_version: "3.4" rails_version: "8.0" - mode: "capture_patch_disabled" - - ruby_version: "3.4" - rails_version: "8.0" - mode: "capture_patch_enabled" - - ruby_version: "head" - rails_version: "main" - mode: "capture_patch_disabled" - ruby_version: "head" rails_version: "main" - mode: "capture_patch_enabled" env: BUNDLE_GEMFILE: gemfiles/rails_${{ matrix.rails_version }}.gemfile steps: @@ -91,7 +63,6 @@ jobs: RAISE_ON_WARNING: 1 RAILS_VERSION: ${{ matrix.rails_version }} RUBY_VERSION: ${{ matrix.ruby_version }} - CAPTURE_PATCH_ENABLED: ${{ matrix.mode == 'capture_patch_enabled' && 'true' || 'false' }} - name: Upload coverage results uses: actions/upload-artifact@v4.4.0 if: always() diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 95a2e0ca5..3df2f6e2f 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -16,8 +16,21 @@ require "view_component/with_content_helper" require "view_component/use_helpers" +module ActionView + class OutputBuffer + def with_buffer(buf = nil) + new_buffer = buf || +"" + old_buffer, @raw_buffer = @raw_buffer, new_buffer + yield + new_buffer + ensure + @raw_buffer = old_buffer + end + end +end + module ViewComponent - class Base < ActionView::Base + class Base class << self delegate(*ViewComponent::Config.defaults.keys, to: :config) @@ -33,6 +46,11 @@ def config end end + include ActionView::Helpers + + include ERB::Escape + include ActiveSupport::CoreExt::ERBUtil + include ViewComponent::InlineTemplate include ViewComponent::UseHelpers include ViewComponent::Slotable @@ -48,6 +66,9 @@ def config # For Content Security Policy nonces delegate :content_security_policy_nonce, to: :helpers + # HTML construction methods + delegate :output_buffer, :lookup_context, :view_renderer, :view_flow, to: :helpers + # Config option that strips trailing whitespace in templates before compiling them. class_attribute :__vc_strip_trailing_whitespace, instance_accessor: false, instance_predicate: false self.__vc_strip_trailing_whitespace = false # class_attribute:default doesn't work until Rails 5.2 @@ -64,7 +85,7 @@ def config # @param view_context [ActionView::Base] The original view context. # @return [void] def set_original_view_context(view_context) - self.__vc_original_view_context = view_context + # no-op end # Entrypoint for rendering components. @@ -81,7 +102,7 @@ def render_in(view_context, &block) @view_context = view_context self.__vc_original_view_context ||= view_context - @output_buffer = ActionView::OutputBuffer.new + @output_buffer = view_context.output_buffer @lookup_context ||= view_context.lookup_context @@ -112,14 +133,20 @@ def render_in(view_context, &block) before_render if render? - rendered_template = render_template_for(@__vc_variant, __vc_request&.format&.to_sym).to_s + value = nil + + @output_buffer.with_buffer do + rendered_template = render_template_for(@__vc_variant, __vc_request&.format&.to_sym).to_s - # Avoid allocating new string when output_preamble and output_postamble are blank - if output_preamble.blank? && output_postamble.blank? - rendered_template - else - safe_output_preamble + rendered_template + safe_output_postamble + # Avoid allocating new string when output_preamble and output_postamble are blank + value = if output_preamble.blank? && output_postamble.blank? + rendered_template + else + safe_output_preamble + rendered_template + safe_output_postamble + end end + + value else "" end @@ -211,7 +238,7 @@ def initialize(*) def render(options = {}, args = {}, &block) if options.respond_to?(:set_original_view_context) options.set_original_view_context(self.__vc_original_view_context) - super + @view_context.render(options, args, &block) else __vc_original_view_context.render(options, args, &block) end diff --git a/lib/view_component/capture_compatibility.rb b/lib/view_component/capture_compatibility.rb deleted file mode 100644 index 15477b3a3..000000000 --- a/lib/view_component/capture_compatibility.rb +++ /dev/null @@ -1,44 +0,0 @@ -# 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) - return if base < InstanceMethods - - base.class_eval do - alias_method :original_capture, :capture - end - - base.prepend(InstanceMethods) - end - - module InstanceMethods - def capture(*args, &block) - # Handle blocks that originate from C code and raise, such as `&:method` - return original_capture(*args, &block) if block.source_location.nil? - - block_context = block.binding.receiver - - if block_context != self && block_context.class < ActionView::Base - block_context.original_capture(*args, &block) - else - original_capture(*args, &block) - end - end - end - end -end diff --git a/lib/view_component/collection.rb b/lib/view_component/collection.rb index 798e38c25..4555ffe05 100644 --- a/lib/view_component/collection.rb +++ b/lib/view_component/collection.rb @@ -9,15 +9,8 @@ class Collection delegate :size, to: :@collection - attr_accessor :__vc_original_view_context - - def set_original_view_context(view_context) - self.__vc_original_view_context = view_context - end - def render_in(view_context, &block) components.map do |component| - component.set_original_view_context(__vc_original_view_context) component.render_in(view_context, &block) end.join(rendered_spacer(view_context)).html_safe end @@ -73,7 +66,6 @@ def component_options(item, iterator) def rendered_spacer(view_context) if @spacer_component - @spacer_component.set_original_view_context(__vc_original_view_context) @spacer_component.render_in(view_context) else "" diff --git a/lib/view_component/engine.rb b/lib/view_component/engine.rb index b920eb5d8..2ba506661 100644 --- a/lib/view_component/engine.rb +++ b/lib/view_component/engine.rb @@ -68,14 +68,6 @@ 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 @@ -131,6 +123,39 @@ class Engine < Rails::Engine # :nodoc: # :nocov: end + initializer "view_component.propshaft_support" do |_app| + ActiveSupport.on_load(:view_component) do + if defined?(Propshaft) + include Propshaft::Helper + end + end + end + + config.after_initialize do |app| + ActiveSupport.on_load(:view_component) do + if defined?(Sprockets::Rails) + include Sprockets::Rails::Helper + + # Copy relevant config to VC context + # See: https://github.com/rails/sprockets-rails/blob/266ec49f3c7c96018dd75f9dc4f9b62fe3f7eecf/lib/sprockets/railtie.rb#L245 + self.debug_assets = app.config.assets.debug + self.digest_assets = app.config.assets.digest + self.assets_prefix = app.config.assets.prefix + self.assets_precompile = app.config.assets.precompile + + self.assets_environment = app.assets + self.assets_manifest = app.assets_manifest + + self.resolve_assets_with = app.config.assets.resolve_with + + self.check_precompiled_asset = app.config.assets.check_precompiled_asset + self.unknown_asset_fallback = app.config.assets.unknown_asset_fallback + # Expose the app precompiled asset check to the view + self.precompiled_asset_checker = -> logical_path { app.asset_precompiled? logical_path } + end + end + end + initializer "static assets" do |app| if serve_static_preview_assets?(app.config) app.middleware.use(::ActionDispatch::Static, "#{root}/app/assets/vendor") diff --git a/lib/view_component/render_component_helper.rb b/lib/view_component/render_component_helper.rb index 945cd539d..96320857c 100644 --- a/lib/view_component/render_component_helper.rb +++ b/lib/view_component/render_component_helper.rb @@ -3,7 +3,6 @@ module ViewComponent module RenderComponentHelper # :nodoc: def render_component(component, &block) - component.set_original_view_context(__vc_original_view_context) if is_a?(ViewComponent::Base) component.render_in(self, &block) end end diff --git a/lib/view_component/slot.rb b/lib/view_component/slot.rb index 205437040..33aa3ba38 100644 --- a/lib/view_component/slot.rb +++ b/lib/view_component/slot.rb @@ -58,15 +58,7 @@ def to_s if defined?(@__vc_content_block) # render_in is faster than `parent.render` @__vc_component_instance.render_in(view_context) do |*args| - return @__vc_content_block.call(*args) if @__vc_content_block&.source_location.nil? - - block_context = @__vc_content_block.binding.receiver - - if block_context.class < ActionView::Base - block_context.capture(*args, &@__vc_content_block) - else - @__vc_content_block.call(*args) - end + @__vc_content_block.call(*args) end else @__vc_component_instance.render_in(view_context) diff --git a/lib/view_component/template.rb b/lib/view_component/template.rb index ebdeb0e87..4a1fed390 100644 --- a/lib/view_component/template.rb +++ b/lib/view_component/template.rb @@ -48,8 +48,9 @@ def compile_to_component @component.silence_redefinition_of_method(@call_method_name) # rubocop:disable Style/EvalWithLocation - @component.class_eval <<-RUBY, @path, @lineno + @component.class_eval <<-RUBY, @path, @lineno - 1 def #{@call_method_name} + @view_context.instance_variable_set(:@virtual_path, virtual_path) #{compiled_source} end RUBY diff --git a/lib/view_component/translatable.rb b/lib/view_component/translatable.rb index 422218207..42936e6b7 100644 --- a/lib/view_component/translatable.rb +++ b/lib/view_component/translatable.rb @@ -91,7 +91,7 @@ def store_translations(locale, data, options = EMPTY_HASH) def translate(key = nil, **options) raise ViewComponent::TranslateCalledBeforeRenderError if view_context.nil? - return super unless i18n_backend + return @view_context.translate(key, **options) unless i18n_backend return key.map { |k| translate(k, **options) } if key.is_a?(Array) locale = options.delete(:locale) || ::I18n.locale @@ -108,13 +108,13 @@ def translate(key = nil, **options) # Fallback to the global translations if translated.is_a? ::I18n::MissingTranslation - return super(key, locale: locale, **options) + return @view_context.translate(key, locale: locale, **options) end translated = html_safe_translation(translated) if as_html translated else - super(key, locale: locale, **options) + @view_context.translate(key, locale: locale, **options) end end alias_method :t, :translate diff --git a/test/sandbox/app/components/renders_non_component.html.erb b/test/sandbox/app/components/renders_non_component.html.erb deleted file mode 100644 index a5a7b1a57..000000000 --- a/test/sandbox/app/components/renders_non_component.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render(@not_a_component) %> diff --git a/test/sandbox/app/components/renders_non_component.rb b/test/sandbox/app/components/renders_non_component.rb deleted file mode 100644 index 639d1b5af..000000000 --- a/test/sandbox/app/components/renders_non_component.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -class RendersNonComponent < ViewComponent::Base - class NotAComponent - attr_reader :render_in_view_context, :original_view_context - - def render_in(view_context) - @render_in_view_context = view_context - "I'm not a component".html_safe - end - - def set_original_view_context(view_context) - @original_view_context = view_context - end - end - - def initialize(not_a_component:) - @not_a_component = not_a_component - end -end diff --git a/test/sandbox/config/environments/test.rb b/test/sandbox/config/environments/test.rb index 8896f7e96..22f279726 100644 --- a/test/sandbox/config/environments/test.rb +++ b/test/sandbox/config/environments/test.rb @@ -36,7 +36,6 @@ 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 diff --git a/test/sandbox/test/action_view_compatibility_test.rb b/test/sandbox/test/action_view_compatibility_test.rb index 71348f2f7..625c6c6c6 100644 --- a/test/sandbox/test/action_view_compatibility_test.rb +++ b/test/sandbox/test/action_view_compatibility_test.rb @@ -4,7 +4,6 @@ class ViewComponent::ActionViewCompatibilityTest < ViewComponent::TestCase def test_renders_form_for_labels_with_block_correctly - skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true" render_inline(FormForComponent.new) assert_selector("form > div > label > input") @@ -12,7 +11,6 @@ def test_renders_form_for_labels_with_block_correctly end def test_renders_form_with_labels_with_block_correctly - skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true" render_inline(FormWithComponent.new) assert_selector("form > div > label > input") @@ -20,7 +18,6 @@ def test_renders_form_with_labels_with_block_correctly end def test_form_without_compatibility_does_not_raise - 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 @@ -28,7 +25,6 @@ def test_form_without_compatibility_does_not_raise end 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 @@ -36,16 +32,7 @@ def test_form_with_partial_render_works end def test_helper_with_content_tag - skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true" render_inline(ContentTagComponent.new) assert_selector("div > p") end - - def test_including_compat_module_twice_does_not_blow_the_stack - skip unless ENV["CAPTURE_PATCH_ENABLED"] == "true" - ActionView::Base.include(ViewComponent::CaptureCompatibility) - render_inline(FormForComponent.new) - assert_selector("form > div > label > input") - refute_selector("form > div > input") - end end diff --git a/test/sandbox/test/config_test.rb b/test/sandbox/test/config_test.rb index 86c662a02..96c9ecf6d 100644 --- a/test/sandbox/test/config_test.rb +++ b/test/sandbox/test/config_test.rb @@ -40,13 +40,5 @@ def test_all_methods_are_documented assert configuration_methods_to_document.map(&:docstring).all?(&:present?), "Configuration options are missing docstrings." end - - def test_compatibility_module_included - if ENV["CAPTURE_PATCH_ENABLED"] == "true" - assert ActionView::Base < ViewComponent::CaptureCompatibility - else - refute ActionView::Base < ViewComponent::CaptureCompatibility - end - end end end diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index 76e65cf4d..ca1666947 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -579,11 +579,7 @@ def test_render_component_in_turbo_stream expected_response_body = <<~TURBOSTREAM TURBOSTREAM - if ViewComponent::Base.config.capture_compatibility_patch_enabled - assert_equal expected_response_body, response.body - else - assert_not_equal expected_response_body, response.body - end + assert_equal expected_response_body, response.body end end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index fa3e6fb0e..4290ea4ae 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -16,7 +16,7 @@ def test_render_inline_allocations MyComponent.ensure_compiled allocations = (Rails.version.to_f >= 8.0) ? - {"3.5.0" => 119, "3.4.2" => 125, "3.3.7" => 137} : + {"3.5.0" => 119, "3.4.2" => 127, "3.3.7" => 137} : {"3.3.7" => 128, "3.3.0" => 140, "3.2.8" => 126, "3.1.7" => 126, "3.0.7" => 135} assert_allocations(**allocations) do @@ -1064,20 +1064,6 @@ def test_component_renders_without_trailing_whitespace refute @rendered_content =~ /\s+\z/, "Rendered component contains trailing whitespace" end - def test_renders_objects_in_component_view_context - not_a_component = RendersNonComponent::NotAComponent.new - component = RendersNonComponent.new(not_a_component: not_a_component) - - render_inline(component) - - assert_selector "span", text: "I'm not a component" - - assert( - not_a_component.render_in_view_context == component, - "Component-like object was not rendered in the parent component's view context" - ) - end - def test_renders_nested_collection items = %w[foo bar baz boo] render_inline(NestedCollectionWrapperComponent.new(items: items))