Skip to content
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

Add configuration and support for freezing string literals in compiled templates #1884

4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ nav_order: 5

*Cameron Dutro*

* Add configuration and support for compiling templates with `frozen_string_literal` magic comment.

*Mitchell Henke*

## 3.8.0

* Use correct value for the `config.action_dispatch.show_exceptions` config option for edge Rails.
Expand Down
10 changes: 5 additions & 5 deletions lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def set_original_view_context(view_context)
#
# @return [String]
def render_in(view_context, &block)
self.class.compile(raise_errors: true)
self.class.compile(raise_errors: true, frozen_string_literal: ViewComponent::Base.config.frozen_string_literal)

@view_context = view_context
self.__vc_original_view_context ||= view_context
Expand Down Expand Up @@ -509,7 +509,7 @@ def short_identifier
def inherited(child)
# Compile so child will inherit compiled `call_*` template methods that
# `compile` defines
compile
compile(frozen_string_literal: ViewComponent::Base.config.frozen_string_literal)

# Give the child its own personal #render_template_for to protect against the case when
# eager loading is disabled and the parent component is rendered before the child. In
Expand All @@ -520,7 +520,7 @@ def inherited(child)
def render_template_for(variant = nil)
# Force compilation here so the compiler always redefines render_template_for.
# This is mostly a safeguard to prevent infinite recursion.
self.class.compile(raise_errors: true, force: true)
self.class.compile(raise_errors: true, force: true, frozen_string_literal: #{ViewComponent::Base.config.frozen_string_literal})
# .compile replaces this method; call the new one
render_template_for(variant)
end
Expand Down Expand Up @@ -573,8 +573,8 @@ def ensure_compiled
# Do as much work as possible in this step, as doing so reduces the amount
# of work done each time a component is rendered.
# @private
def compile(raise_errors: false, force: false)
compiler.compile(raise_errors: raise_errors, force: force)
def compile(raise_errors: false, force: false, frozen_string_literal: false)
compiler.compile(raise_errors: raise_errors, force: force, frozen_string_literal: frozen_string_literal)
end

# @private
Expand Down
26 changes: 18 additions & 8 deletions lib/view_component/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def development?
self.class.mode == DEVELOPMENT_MODE
end

def compile(raise_errors: false, force: false)
def compile(raise_errors: false, force: false, frozen_string_literal: false)
return if compiled? && !force
return if component_class == ViewComponent::Base

Expand All @@ -49,12 +49,17 @@ def compile(raise_errors: false, force: false)

redefinition_lock.synchronize do
component_class.silence_redefinition_of_method("call")
# rubocop:disable Style/EvalWithLocation
component_class.class_eval <<-RUBY, template.path, template.lineno
source = <<-SOURCE
def call
#{compiled_inline_template(template)}
end
RUBY
SOURCE
# rubocop:disable Style/EvalWithLocation
if frozen_string_literal
component_class.class_eval(source.prepend("# frozen_string_literal: true\n"), template.path, template.lineno - 1)
else
component_class.class_eval(source, template.path, template.lineno)
end
# rubocop:enable Style/EvalWithLocation

component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call))
Expand All @@ -73,12 +78,17 @@ def render_template_for(variant = nil)

redefinition_lock.synchronize do
component_class.silence_redefinition_of_method(method_name)
source = <<-SOURCE
def #{method_name}
#{compiled_template(template[:path])}
end
SOURCE
# rubocop:disable Style/EvalWithLocation
component_class.class_eval <<-RUBY, template[:path], 0
def #{method_name}
#{compiled_template(template[:path])}
if frozen_string_literal
component_class.class_eval(source.prepend("# frozen_string_literal: true\n"), template[:path], -1)
else
component_class.class_eval(source, template[:path], 0)
end
RUBY
# rubocop:enable Style/EvalWithLocation
end
end
Expand Down
13 changes: 12 additions & 1 deletion lib/view_component/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def defaults
preview_paths: default_preview_paths,
test_controller: "ApplicationController",
default_preview_layout: nil,
capture_compatibility_patch_enabled: false
capture_compatibility_patch_enabled: false,
frozen_string_literal: :inherit_from_rails
})
end

Expand Down Expand Up @@ -154,6 +155,16 @@ def defaults
# previews.
# Defaults to `false`.

# @!attribute frozen_string_literal
# @return [Boolean]
# Enables compiling templates with the frozen_string_literal magic
# comment, which prevents modification of string objects by
# assuming they are frozen on initialize. Has performance
# benefits. This configuration will default to inheriting from
# `Rails.application.config.action_view.frozen_string_literal`, but can be
# overridden.
# Defaults to `:inherit_from_rails`.

def default_preview_paths
return [] unless defined?(Rails.root) && Dir.exist?("#{Rails.root}/test/components/previews")

Expand Down
11 changes: 9 additions & 2 deletions lib/view_component/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ class Engine < Rails::Engine # :nodoc:
end

initializer "view_component.set_configs" do |app|
if app.config.view_component.frozen_string_literal == :inherit_from_rails
app.config.view_component.frozen_string_literal = !!app.config.action_view.frozen_string_literal
end
options = app.config.view_component

%i[generate preview_controller preview_route show_previews_source].each do |config_option|
Expand Down Expand Up @@ -71,9 +74,13 @@ class Engine < Rails::Engine # :nodoc:
end
end

initializer "view_component.eager_load_actions" do
initializer "view_component.eager_load_actions" do |app|
ActiveSupport.on_load(:after_initialize) do
ViewComponent::Base.descendants.each(&:compile) if Rails.application.config.eager_load
if Rails.application.config.eager_load
ViewComponent::Base.descendants.each do |descendant|
descendant.compile(frozen_string_literal: app.config.view_component.frozen_string_literal)
end
end
end
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= "a" << "b" %>
4 changes: 4 additions & 0 deletions test/sandbox/app/components/mutated_string_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# frozen_string_literal: true

class MutatedStringComponent < ViewComponent::Base
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class MutatedStringInlineComponent < ViewComponent::Base
erb_template <<~ERB
<%= "a" << "b" %>
ERB
end
1 change: 1 addition & 0 deletions test/sandbox/test/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def test_defaults_are_correct
assert_equal @config.render_monkey_patch_enabled, true
assert_equal @config.show_previews, true
assert_equal @config.preview_paths, ["#{Rails.root}/test/components/previews"]
assert_equal @config.frozen_string_literal, :inherit_from_rails
end

def test_all_methods_are_documented
Expand Down
32 changes: 32 additions & 0 deletions test/sandbox/test/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,38 @@ def test_sets_the_compiler_mode_in_development_mode
end
end

def test_default_sets_frozen_string_literal_to_rails_action_view_frozen_string_literal_value_true
fake_application = OpenStruct.new(config: OpenStruct.new(view_component: ViewComponent::Config.new, action_view: OpenStruct.new(frozen_string_literal: true)))
ViewComponent::Engine.initializers.find { |i| i.name == "view_component.set_configs" }.run(fake_application)
assert_equal fake_application.config.view_component.frozen_string_literal, true
end

def test_default_sets_frozen_string_literal_to_rails_action_view_frozen_string_literal_value_false
fake_application = OpenStruct.new(config: OpenStruct.new(view_component: ViewComponent::Config.new, action_view: OpenStruct.new(frozen_string_literal: false)))
ViewComponent::Engine.initializers.find { |i| i.name == "view_component.set_configs" }.run(fake_application)
assert_equal fake_application.config.view_component.frozen_string_literal, false
end

def test_default_sets_frozen_string_literal_to_rails_action_view_frozen_string_literal_value_nil
fake_application = OpenStruct.new(config: OpenStruct.new(view_component: ViewComponent::Config.new, action_view: OpenStruct.new(frozen_string_literal: nil)))
ViewComponent::Engine.initializers.find { |i| i.name == "view_component.set_configs" }.run(fake_application)
assert_equal fake_application.config.view_component.frozen_string_literal, false
end

def test_override_true_does_not_set_frozen_string_literal_to_rails_action_view_frozen_string_literal_value_false
fake_application = OpenStruct.new(config: OpenStruct.new(view_component: ViewComponent::Config.new, action_view: OpenStruct.new(frozen_string_literal: false)))
fake_application.config.view_component.frozen_string_literal = true
ViewComponent::Engine.initializers.find { |i| i.name == "view_component.set_configs" }.run(fake_application)
assert_equal fake_application.config.view_component.frozen_string_literal, true
end

def test_override_false_does_not_set_frozen_string_literal_to_rails_action_view_frozen_string_literal_value_true
fake_application = OpenStruct.new(config: OpenStruct.new(view_component: ViewComponent::Config.new, action_view: OpenStruct.new(frozen_string_literal: true)))
fake_application.config.view_component.frozen_string_literal = false
ViewComponent::Engine.initializers.find { |i| i.name == "view_component.set_configs" }.run(fake_application)
assert_equal fake_application.config.view_component.frozen_string_literal, false
end

def test_link_to_helper
get "/link_to_helper"
assert_select "a > i,span"
Expand Down
48 changes: 48 additions & 0 deletions test/sandbox/test/rendering_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1113,4 +1113,52 @@ def test_use_helper
render_inline(UseHelpersComponent.new)
assert_selector ".helper__message", text: "Hello helper method"
end

def test_frozen_string_literal_disabled
old_value = ViewComponent::Base.config.frozen_string_literal
ViewComponent::Base.config.frozen_string_literal = false

with_new_cache do
render_inline(MutatedStringComponent.new)
assert_includes rendered_content, "ab"
end
ensure
ViewComponent::Base.config.frozen_string_literal = old_value
end

def test_frozen_string_literal_enabled
old_value = ViewComponent::Base.config.frozen_string_literal
ViewComponent::Base.config.frozen_string_literal = true
with_new_cache do
assert_raises FrozenError do
render_inline(MutatedStringComponent.new)
end
end
ensure
ViewComponent::Base.config.frozen_string_literal = old_value
end

def test_inline_frozen_string_literal_disabled
old_value = ViewComponent::Base.config.frozen_string_literal
ViewComponent::Base.config.frozen_string_literal = false

with_new_cache do
render_inline(MutatedStringInlineComponent.new)
assert_includes rendered_content, "ab"
end
ensure
ViewComponent::Base.config.frozen_string_literal = old_value
end

def test_inline_frozen_string_literal_enabled
old_value = ViewComponent::Base.config.frozen_string_literal
ViewComponent::Base.config.frozen_string_literal = true
with_new_cache do
assert_raises FrozenError do
render_inline(MutatedStringInlineComponent.new)
end
end
ensure
ViewComponent::Base.config.frozen_string_literal = old_value
end
end
Loading