Skip to content

Use a global output buffer #1307

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 26 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8effb71
Use a global output buffer
camertron Mar 10, 2022
b2bbac2
Fix tests for Rails < 6
camertron Mar 10, 2022
28439b6
Make rubocop happy
camertron Mar 10, 2022
f5a1bb5
Test fragment caching when the global output buffer is enabled
camertron Mar 10, 2022
2a66952
Fix test coverage
camertron Mar 10, 2022
ec31136
Add an additional test for nested HAML/ERB/components
camertron Mar 11, 2022
962301b
Run test suite with GOB and without
camertron Mar 11, 2022
8815444
Explicitly skip HAML tests for rails < 6.1
camertron Mar 12, 2022
9cc3d86
Fixed final line endings with Logerfo/newline-action.
Logerfo Mar 12, 2022
f870462
Support helper methods; fix rubocop issues
camertron Mar 15, 2022
bbc04f4
Merge branch 'output_buffer_stack' of github.com:github/view_componen…
camertron Mar 15, 2022
446ce79
Fix double render issue when rendering the same component instance tw…
camertron Mar 16, 2022
0d2c23a
Try a different strategy to get helpers to work
camertron Mar 16, 2022
46577c6
Fix code coverage
camertron Mar 16, 2022
60188b9
Make actionview monkeypatch a little less invasive
camertron Mar 17, 2022
0379827
Use instance variable instead of method arg for indicating use of a g…
camertron Mar 18, 2022
02c7bc7
Use human-readable name for GoB so CI jobs make sense
camertron Mar 18, 2022
6074b1e
Remove unnecessary test helper
camertron Mar 19, 2022
b7903ed
Merging latest main
camertron Mar 19, 2022
e5a2e4f
Fix CHANGELOG
camertron Mar 19, 2022
14b5aa3
Add tests for several common helpers
camertron Mar 21, 2022
96c3232
Fixed final line endings with Logerfo/newline-action.
Logerfo Mar 21, 2022
348f56d
Fix helpers used outside of components
camertron Mar 21, 2022
39a8d78
Merging upstream
camertron Mar 21, 2022
9fe51ad
Fix linting issues
camertron Mar 21, 2022
15f2948
Merge branch 'main' into output_buffer_stack
camertron Mar 22, 2022
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
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ jobs:
matrix:
rails_version: ['5.2.6', '6.0.4.4', '6.1.4.4', '7.0.1', 'main']
ruby_version: ['2.5', '2.6', '2.7', '3.0', '3.1']
output_buffer: ['global_buffer', 'local_buffer']
exclude:
- rails_version: '5.2.6'
ruby_version: '3.0'
Expand Down Expand Up @@ -64,6 +65,7 @@ jobs:
env:
MEASURE_COVERAGE: true
RAILS_VERSION: ${{ matrix.rails_version }}
VIEW_COMPONENT_USE_GLOBAL_OUTPUT_BUFFER: ${{ matrix.output_buffer == 'global_buffer' && '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 @@ -29,6 +29,10 @@ title: Changelog

*Cameron Dutro*

* Add the option to use a "global" output buffer so `form_for` and friends can be used with view components.

*Cameron Dutro*, *Blake Williams*

## 2.50.0

* Add tests for `layout` usage when rendering via controller.
Expand Down
18 changes: 17 additions & 1 deletion docs/known_issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,23 @@ title: Known issues

## form_for compatibility

ViewComponent [isn't currently compatible](https://github.com/github/view_component/issues/241) with `form_for` helpers.
ViewComponent [isn't compatible](https://github.com/github/view_component/issues/241) with `form_for` helpers by default.

### Using a Global Output Buffer (Experimental)

One possible solution to the form helpers problem is to use a single, global output buffer. For details, please refer to [this pull request](https://github.com/github/view_component/pull/1307).

The global output buffer behavior is opt-in. Prepend the `ViewComponent::GlobalOutputBuffer` module into individual component classes to use it.

For example:

```ruby
class MyComponent < ViewComponent::Base
prepend ViewComponent::GlobalOutputBuffer
end
```

It is also possible to enable the global output buffer globally by setting the `config.view_component.use_global_output_buffer` setting to `true` in your Rails config.

## Inconsistent controller rendering behavior between Rails versions

Expand Down
2 changes: 2 additions & 0 deletions lib/view_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ module ViewComponent
autoload :CompileCache
autoload :ComponentError
autoload :Deprecation
autoload :GlobalOutputBuffer
autoload :Instrumentation
autoload :OutputBufferStack
autoload :Preview
autoload :PreviewTemplateError
autoload :TestHelpers
Expand Down
8 changes: 7 additions & 1 deletion lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ def render_in(view_context, &block)
@view_context = view_context
self.__vc_original_view_context ||= view_context

@output_buffer = ActionView::OutputBuffer.new unless @global_buffer_in_use

@lookup_context ||= view_context.lookup_context

# required for path helpers in older Rails versions
Expand Down Expand Up @@ -102,14 +104,18 @@ def render_in(view_context, &block)
before_render

if render?
render_template_for(@__vc_variant).to_s + _output_postamble
perform_render
else
""
end
ensure
@current_template = old_current_template
end

def perform_render
render_template_for(@__vc_variant).to_s + _output_postamble
end

# :nocov:
def render_template_for(variant = nil)
# Force compilation here so the compiler always redefines render_template_for.
Expand Down
3 changes: 1 addition & 2 deletions lib/view_component/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ def compile(raise_errors: false, force: false)
component_class.send(:remove_method, method_name.to_sym)
end

component_class.class_eval <<-RUBY, template[:path], -1
component_class.class_eval <<-RUBY, template[:path], 0
def #{method_name}
@output_buffer = ActionView::OutputBuffer.new
#{compiled_template(template[:path])}
end
RUBY
Expand Down
16 changes: 16 additions & 0 deletions lib/view_component/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class Engine < Rails::Engine # :nodoc:
options.instrumentation_enabled = false if options.instrumentation_enabled.nil?
options.preview_route ||= ViewComponent::Base.preview_route
options.preview_controller ||= ViewComponent::Base.preview_controller
options.use_global_output_buffer = false if options.use_global_output_buffer.nil?

if options.show_previews
options.preview_paths << "#{Rails.root}/test/components/previews" if defined?(Rails.root) && Dir.exist?(
Expand Down Expand Up @@ -57,6 +58,21 @@ class Engine < Rails::Engine # :nodoc:
end
end

initializer "view_component.enable_global_output_buffer" do |app|
ActiveSupport.on_load(:view_component) do
env_use_gob = ENV.fetch("VIEW_COMPONENT_USE_GLOBAL_OUTPUT_BUFFER", "false") == "true"
config_use_gob = app.config.view_component.use_global_output_buffer

if config_use_gob || env_use_gob
# :nocov:
app.config.view_component.use_global_output_buffer = true
ViewComponent::Base.prepend(ViewComponent::GlobalOutputBuffer)
ActionView::Base.prepend(ViewComponent::GlobalOutputBuffer::ActionViewMods)
# :nocov:
end
end
end

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

Expand Down
89 changes: 89 additions & 0 deletions lib/view_component/global_output_buffer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# frozen_string_literal: true

module ViewComponent
module GlobalOutputBuffer
def render_in(view_context, &block)
unless view_context.output_buffer.is_a?(OutputBufferStack)
# use instance_variable_set here to avoid triggering the code in the #output_buffer= method below
view_context.instance_variable_set(:@output_buffer, OutputBufferStack.new(view_context.output_buffer))
end

@output_buffer = view_context.output_buffer
@global_buffer_in_use = true

super(view_context, &block)
end

def perform_render
# HAML unhelpfully assigns to @output_buffer directly, so we hold onto a reference to
# it and restore @output_buffer when the HAML engine is finished. In non-HAML cases,
# @output_buffer and orig_buf will point to the same object, making the reassignment
# statements no-ops.
orig_buf = @output_buffer
@output_buffer.push
result = render_template_for(@__vc_variant).to_s + _output_postamble
@output_buffer = orig_buf
@output_buffer.pop
result
end

def output_buffer=(other_buffer)
@output_buffer.replace(other_buffer)
end

def with_output_buffer(buf = nil)
unless buf
buf = ActionView::OutputBuffer.new
if output_buffer && output_buffer.respond_to?(:encoding)
buf.force_encoding(output_buffer.encoding)
end
end

output_buffer.push(buf)
result = nil

begin
yield
ensure
# assign result here to avoid a return statement, which will
# immediately return to the caller and swallow any errors
result = output_buffer.pop
end

result
end

module ActionViewMods
def with_output_buffer(buf = nil)
# save a reference to the stack-based buffer
orig_buf = output_buffer
result = nil

super do
if orig_buf.respond_to?(:push)
# super will have set self.output_buffer to the new buffer
new_buf = self.output_buffer
orig_buf.push(new_buf)

# set output_buffer back to the original, stack-based buffer we
# saved a reference to earlier
self.output_buffer = orig_buf

begin
yield
ensure
result = orig_buf.pop
end
else
# :nocov:
yield
result = output_buffer
# :nocov:
end
end

result
end
end
end
end
67 changes: 67 additions & 0 deletions lib/view_component/output_buffer_stack.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# frozen_string_literal: true

module ViewComponent
class OutputBufferStack
delegate_missing_to :@current_buffer
delegate :presence, :present?, :html_safe?, to: :@current_buffer

attr_reader :buffer_stack

def self.make_frame(*args)
ActionView::OutputBuffer.new(*args)
end

def initialize(initial_buffer = nil)
if initial_buffer.is_a?(self.class)
@current_buffer = self.class.make_frame(initial_buffer.current)
@buffer_stack = [*initial_buffer.buffer_stack[0..-2], @current_buffer]
else
@current_buffer = initial_buffer || self.class.make_frame
@buffer_stack = [@current_buffer]
end
end

def replace(buffer)
return if self == buffer

@current_buffer = buffer.current
@buffer_stack = buffer.buffer_stack
end

def append=(arg)
@current_buffer.append = arg
end

def safe_append=(arg)
@current_buffer.safe_append = arg
end

def safe_concat(arg)
# rubocop:disable Rails/OutputSafety
@current_buffer.safe_concat(arg)
# rubocop:enable Rails/OutputSafety
end

def length
@current_buffer.length
end

def push(buffer = nil)
buffer ||= self.class.make_frame
@buffer_stack.push(buffer)
@current_buffer = buffer
end

def pop
@buffer_stack.pop.tap do
@current_buffer = @buffer_stack.last
end
end

def to_s
@current_buffer
end

alias_method :current, :to_s
end
end
14 changes: 4 additions & 10 deletions lib/view_component/slot_v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,12 @@ def to_s
if defined?(@__vc_content_set_by_with_content)
@__vc_component_instance.with_content(@__vc_content_set_by_with_content)

view_context.capture do
@__vc_component_instance.render_in(view_context)
end
@__vc_component_instance.render_in(view_context)
elsif defined?(@__vc_content_block)
view_context.capture do
# render_in is faster than `parent.render`
@__vc_component_instance.render_in(view_context, &@__vc_content_block)
end
# render_in is faster than `parent.render`
@__vc_component_instance.render_in(view_context, &@__vc_content_block)
else
view_context.capture do
@__vc_component_instance.render_in(view_context)
end
@__vc_component_instance.render_in(view_context)
end
elsif defined?(@__vc_content)
@__vc_content
Expand Down
5 changes: 5 additions & 0 deletions performance/components/name_component_with_gob.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<h1>hello <%= @name %></h1>

<% 50.times do %>
<%= render Performance::NestedNameComponentWithGOB.new(name: @name) %>
<% end %>
5 changes: 5 additions & 0 deletions performance/components/name_component_with_gob.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# frozen_string_literal: true

class Performance::NameComponentWithGOB < Performance::NameComponent
prepend ViewComponent::GlobalOutputBuffer
end
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>nested hello <%= @name %></p>
5 changes: 5 additions & 0 deletions performance/components/nested_name_component_with_gob.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# frozen_string_literal: true

class Performance::NestedNameComponentWithGOB < Performance::NestedNameComponent
prepend ViewComponent::GlobalOutputBuffer
end
33 changes: 33 additions & 0 deletions performance/global_output_buffer_benchmark.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# frozen_string_literal: true

# Run `bundle exec rake benchmark` to execute benchmark.
# This is very much a work-in-progress. Please feel free to make/suggest improvements!

require "benchmark/ips"

# Configure Rails Environment
ENV["RAILS_ENV"] = "production"
require File.expand_path("../test/sandbox/config/environment.rb", __dir__)

module Performance
require_relative "components/name_component.rb"
require_relative "components/nested_name_component.rb"
require_relative "components/name_component_with_gob.rb"
require_relative "components/nested_name_component_with_gob.rb"
end

class BenchmarksController < ActionController::Base
end

BenchmarksController.view_paths = [File.expand_path("./views", __dir__)]
controller_view = BenchmarksController.new.view_context

Benchmark.ips do |x|
x.time = 10
x.warmup = 2

x.report("component") { controller_view.render(Performance::NameComponent.new(name: "Fox Mulder")) }
x.report("component with GOB") { controller_view.render(Performance::NameComponentWithGOB.new(name: "Fox Mulder")) }

x.compare!
end
3 changes: 3 additions & 0 deletions test/sandbox/app/components/content_tag_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= helpers.my_helper_method({ foo: :bar }) do %>
<p>Content inside helper method</p>
<% end %>
4 changes: 4 additions & 0 deletions test/sandbox/app/components/content_tag_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# frozen_string_literal: true

class ContentTagComponent < ViewComponent::Base
end
3 changes: 3 additions & 0 deletions test/sandbox/app/components/form_for_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 LabelComponent.new(form: form) %>
<% end %>
6 changes: 6 additions & 0 deletions test/sandbox/app/components/form_for_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

class FormForComponent < ViewComponent::Base
def initialize
end
end
3 changes: 3 additions & 0 deletions test/sandbox/app/components/form_with_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= form_with model: Post.new, url: "/" do |form| %>
<%= render LabelComponent.new(form: form) %>
<% end %>
6 changes: 6 additions & 0 deletions test/sandbox/app/components/form_with_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

class FormWithComponent < ViewComponent::Base
def initialize
end
end
Loading