Skip to content

Commit

Permalink
Allow templates to define which locals they accept.
Browse files Browse the repository at this point in the history
  • Loading branch information
joelhawksley committed Aug 1, 2022
1 parent 765caa4 commit bbe7d19
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 5 deletions.
24 changes: 24 additions & 0 deletions actionview/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
* Allow templates to set explicit locals.

By default, templates will accept any `locals` as keyword arguments. To define what `locals` a template accepts, add a `locals` magic comment:

```erb
<%# locals: (message:) -%>
<%= message %>
```
Default values can also be provided:
```erb
<%# locals: (message: "Hello, world!") -%>
<%= message %>
```
Or `locals` can be disabled entirely:
```erb
<%# locals: () %>
```
*Joel Hawksley*
* Add `include_seconds` option for `datetime_local_field`
This allows to omit seconds part in the input field, by passing `include_seconds: false`
Expand Down
20 changes: 18 additions & 2 deletions actionview/lib/action_view/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,27 @@ def initialize(lookup_context, assigns, controller) # :nodoc:
_prepare_context
end

def _run(method, template, locals, buffer, add_to_stack: true, &block)
def _run(method, template, locals, buffer, add_to_stack: true, has_explicit_locals: false, &block)
_old_output_buffer, _old_virtual_path, _old_template = @output_buffer, @virtual_path, @current_template
@current_template = template if add_to_stack
@output_buffer = buffer
public_send(method, locals, buffer, &block)

if has_explicit_locals
begin
public_send(method, buffer, **locals, &block)
rescue ArgumentError => argument_error
raise(
ArgumentError,
argument_error.
message.
gsub("unknown keyword:", "unknown local:").
gsub("missing keyword:", "missing local:").
gsub("no keywords accepted", "no locals accepted")
)
end
else
public_send(method, locals, buffer, &block)
end
ensure
@output_buffer, @virtual_path, @current_template = _old_output_buffer, _old_virtual_path, _old_template
end
Expand Down
50 changes: 47 additions & 3 deletions actionview/lib/action_view/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ module ActionView
class Template
extend ActiveSupport::Autoload

EXPLICIT_LOCALS_REGEX = /\#\s+locals:\s+\((.*)\)/

# === Encodings in ActionView::Template
#
# ActionView::Template is one of a few sources of potential
Expand Down Expand Up @@ -121,12 +123,13 @@ class Template
attr_reader :variable, :format, :variant, :locals, :virtual_path

def initialize(source, identifier, handler, locals:, format: nil, variant: nil, virtual_path: nil)
@source = source
@source = source.dup
@identifier = identifier
@handler = handler
@compiled = false
@locals = locals
@virtual_path = virtual_path
@explicit_locals = nil

@variable = if @virtual_path
base = @virtual_path.end_with?("/") ? "" : ::File.basename(@virtual_path)
Expand Down Expand Up @@ -154,7 +157,7 @@ def supports_streaming?
def render(view, locals, buffer = ActionView::OutputBuffer.new, add_to_stack: true, &block)
instrument_render_template do
compile!(view)
view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, &block)
view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, has_explicit_locals: @explicit_locals.present?, &block)
end
rescue => e
handle_render_error(view, e)
Expand Down Expand Up @@ -222,6 +225,17 @@ def encode!
end
end

# This method is responsible for marking a template as having explicit locals
# and extracting any arguments declared in the format
# locals: (message:, label: "My Message")
def explicit_locals!
self.source.sub!(EXPLICIT_LOCALS_REGEX, "")
@explicit_locals = $1

return if @explicit_locals.nil? # Magic comment not found

@explicit_locals = "**nil" if @explicit_locals.blank?
end

# Exceptions are marshalled when using the parallel test runner with DRb, so we need
# to ensure that references to the template object can be marshalled as well. This means forgoing
Expand Down Expand Up @@ -273,14 +287,22 @@ def compile!(view)
# In general, this means that templates will be UTF-8 inside of Rails,
# regardless of the original source encoding.
def compile(mod)
explicit_locals!
source = encode!
code = @handler.call(self, source)

method_arguments =
if @explicit_locals.present?
"output_buffer, #{@explicit_locals}"
else
"local_assigns, output_buffer"
end

# Make sure that the resulting String to be eval'd is in the
# encoding of the code
original_source = source
source = +<<-end_src
def #{method_name}(local_assigns, output_buffer)
def #{method_name}(#{method_arguments})
@virtual_path = #{@virtual_path.inspect};#{locals_code};#{code}
end
end_src
Expand Down Expand Up @@ -311,6 +333,26 @@ def #{method_name}(local_assigns, output_buffer)
# the result into the template, but missing an end parenthesis.
raise SyntaxErrorInTemplate.new(self, original_source)
end

return unless @explicit_locals.present?

# Check compiled method parameters to ensure that only kwargs
# were provided as explicit locals, preventing `locals: (foo, *foo)` etc
# and allowing `locals: (foo:)`.

non_kwarg_parameters =
(mod.instance_method(method_name).parameters - [[:req, :output_buffer]]).
select { |parameter| ![:keyreq, :key, :keyrest, :nokey].include?(parameter[0]) }

return unless non_kwarg_parameters.any?

mod.undef_method(method_name)

raise ArgumentError.new(
"#{non_kwarg_parameters.map { |_, name| "`#{name}`" }.to_sentence} set as non-keyword " \
"#{'argument'.pluralize(non_kwarg_parameters.length)} for #{short_identifier}. " \
"Locals can only be set as keyword arguments."
)
end

def handle_render_error(view, e)
Expand All @@ -323,6 +365,8 @@ def handle_render_error(view, e)
end

def locals_code
return "" if @explicit_locals.present?

# Only locals with valid variable names get set directly. Others will
# still be available in local_assigns.
locals = @locals - Module::RUBY_RESERVED_KEYWORDS
Expand Down
59 changes: 59 additions & 0 deletions actionview/test/template/template_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,57 @@ def test_encoding_can_be_specified_with_magic_comment
assert_equal "\nhello \u{fc}mlat", render
end

def test_locals_can_be_disabled
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: () -%>")
render(foo: "bar")
end

assert_match(/no locals accepted/, error.message)
end

def test_locals_can_not_be_specified_with_positional_arguments
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: (foo) -%>")
render(foo: "bar")
end

assert_match(/`foo` set as non-keyword argument/, error.message)
end

def test_locals_can_be_specified_with_splat_arguments
@template = new_template("<%# locals: (**etc) -%><%= etc[:foo] %>")
assert_equal "bar", render(foo: "bar")
end

def test_locals_can_be_specified
@template = new_template("<%# locals: (message:) -%>\n<%= message %>")
assert_equal "Hello", render(message: "Hello")
end

def test_default_locals_can_be_specified
@template = new_template("<%# locals: (message: 'Hello') -%>\n<%= message %>")
assert_equal "Hello", render
end

def test_required_locals_can_be_specified
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: (message:) -%>")
render
end

assert_match(/missing local: :message/, error.message)
end

def test_extra_locals_raises_error
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: (message:) -%>")
render(message: "Hi", foo: "bar")
end

assert_match(/unknown local: :foo/, error.message)
end

# TODO: This is currently handled inside ERB. The case of explicitly
# lying about encodings via the normal Rails API should be handled
# inside Rails.
Expand All @@ -167,6 +218,14 @@ def test_encoding_can_be_specified_with_magic_comment_in_erb
end
end

def test_encoding_and_arguments_can_be_specified_with_magic_comment_in_erb
with_external_encoding Encoding::UTF_8 do
@template = new_template("<%# encoding: ISO-8859-1 %>\n<%# locals: (message: 'Hi!') %>\nhello \xFCmlat\n<%= message %>", virtual_path: nil)
assert_equal Encoding::UTF_8, render.encoding
assert_match(/hello \u{fc}mlat\nHi!/, render)
end
end

def test_error_when_template_isnt_valid_utf8
e = assert_raises ActionView::Template::Error do
@template = new_template("hello \xFCmlat", virtual_path: nil)
Expand Down
22 changes: 22 additions & 0 deletions guides/source/action_view_overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,28 @@ You can also specify a second partial to be rendered between instances of the ma

Rails will render the `_product_ruler` partial (with no data passed to it) between each pair of `_product` partials.

#### Explicit Locals

By default, templates will accept any `locals` as keyword arguments. To define what `locals` a template accepts, add a `locals` magic comment:

```erb
<%# locals: (message:) -%>
<%= message %>
```

Default values can also be provided:

```erb
<%# locals: (message: "Hello, world!") -%>
<%= message %>
```

Or `locals` can be disabled entirely:

```erb
<%# locals: () %>
```

### Layouts

Layouts can be used to render a common view template around the results of Rails controller actions. Typically, a Rails application will have a couple of layouts that pages will be rendered within. For example, a site might have one layout for a logged in user and another for the marketing or sales side of the site. The logged in user layout might include top-level navigation that should be present across many controller actions. The sales layout for a SaaS app might include top-level navigation for things like "Pricing" and "Contact Us" pages. You would expect each layout to have a different look and feel. You can read about layouts in more detail in the [Layouts and Rendering in Rails](layouts_and_rendering.html) guide.
Expand Down

0 comments on commit bbe7d19

Please sign in to comment.