Skip to content

Commit b95e35e

Browse files
committed
Action View: Reduce public API of tag helper
The `TagBuilder` instance returned by the [ActionView::Helpers::TagHelper#tag][] method has the ability to build various HTML elements through its reliance on `method_missing`. The magic of that instance hinges on the fact that it has a **minimal** public interface, and transforms missing methods names into HTML elements. For example, calling `tag.div` invokes a missing `#div` method, which renders a `<div>` element. There are two exceptional cases: * `tag.p` is defined, since the existing [Kernel#p][] definition would prevent the underlying `#method_missing` invocation * `tag.attributes` is a defined method to transform `Hash` instances and keyword arguments in HTML attribute strings In addition to those two _intentional_ exceptions, there are also several methods that are incidentally part of the public interface: * `#tag_string` * `#content_tag_string` * `#tag_options` * `#boolean_tag_option` * `#tag_option` Along with those methods, the class also includes [OutputSafetyHelper][] and [CaptureHelper][], which expand surface area of the class's public interface even further. While it's unlikely that these methods would collide with method invocations intended to construct HTML elements, they still impact that design of the object. The "public" nature of the `TagBuilder` interface has some subtle nuances. While it **is** marked with `:nodoc:`, instances are returned by a public `tag` method, despite it being considered a private class only meant for internal consumption. That same is true for the incidentally public methods mentioned above: no consuming applications should be invoking `#tag_string` or `#tag_options`. While that's true, those methods _are_ being invoked **directly** by other Action View classes. This commit removes those invocations, and instead replaces them with public method calls. In cases where the tag name is statically know ahead of time, they're replaced with calls to that method (for example, `content_tag(:option)` becomes `tag.option`). When they're dynamic, they're replaced by calls to [public_send][] (for example, `tag(name)` becomes `tag.public_send(name)`). The majority of these changes are painless, with one exception. Some calls to the `#tag` helper also pass an `open` positional argument that isn't part of the `TagBuilder` class's public interface. As a result, that becomes slightly more complicated, and requires some String mutation to continue to pass the test suite. This commit also removes the `OutputSafetyHelper` and `CaptureHelper` modules from the class, and delegates to the methods it depended on to `@view_context`. Hopefully, calls to `#tag` with positional arguments are less and less common, since the documentation describes it as a [Legacy Syntax][] marked for deprecation in future versions of Rails. [Kernel#p]: https://ruby-doc.org/3.2.2/Kernel.html#method-i-p [ActionView::Helpers::TagHelper#tag]: https://api.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-tag [public_send]: https://ruby-doc.org/3.2.2/Object.html#method-i-public_send [Legacy Syntax]: https://api.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-tag-label-Legacy+syntax [OutputSafetyHelper]: https://api.rubyonrails.org/classes/ActionView/Helpers/OutputSafetyHelper.html [CaptureHelper]: https://api.rubyonrails.org/classes/ActionView/Helpers/CaptureHelper.html
1 parent 6025230 commit b95e35e

File tree

3 files changed

+34
-37
lines changed

3 files changed

+34
-37
lines changed

actionview/lib/action_view/helpers/form_options_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ def options_for_select(container, selected = nil)
371371
html_attributes[:disabled] ||= disabled && option_value_selected?(value, disabled)
372372
html_attributes[:value] = value
373373

374-
tag_builder.content_tag_string(:option, text, html_attributes)
374+
tag_builder.option(text, **html_attributes)
375375
end.join("\n").html_safe
376376
end
377377

actionview/lib/action_view/helpers/tag_helper.rb

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ module TagHelper
4444
PRE_CONTENT_STRINGS["textarea"] = "\n"
4545

4646
class TagBuilder # :nodoc:
47-
include CaptureHelper
48-
include OutputSafetyHelper
49-
5047
def self.define_element(name, code_generator:, method_name: name)
5148
return if method_defined?(name)
5249

@@ -226,17 +223,7 @@ def attributes(attributes)
226223
tag_options(attributes.to_h).to_s.strip.html_safe
227224
end
228225

229-
def tag_string(name, content = nil, options, escape: true, &block)
230-
content = @view_context.capture(self, &block) if block
231-
232-
content_tag_string(name, content, options, escape)
233-
end
234-
235-
def self_closing_tag_string(name, options, escape = true, tag_suffix = " />")
236-
"<#{name}#{tag_options(options, escape)}#{tag_suffix}".html_safe
237-
end
238-
239-
def content_tag_string(name, content, options, escape = true)
226+
def content_tag_string(name, content, options, escape = true) # :nodoc:
240227
tag_options = tag_options(options, escape) if options
241228

242229
if escape && content.present?
@@ -245,7 +232,7 @@ def content_tag_string(name, content, options, escape = true)
245232
"<#{name}#{tag_options}>#{PRE_CONTENT_STRINGS[name]}#{content}</#{name}>".html_safe
246233
end
247234

248-
def tag_options(options, escape = true)
235+
def tag_options(options, escape = true) # :nodoc:
249236
return if options.blank?
250237
output = +""
251238
sep = " "
@@ -266,7 +253,7 @@ def tag_options(options, escape = true)
266253
tokens = TagHelper.build_tag_values(v)
267254
next if tokens.none?
268255

269-
v = safe_join(tokens, " ")
256+
v = @view_context.safe_join(tokens, " ")
270257
else
271258
v = v.to_s
272259
end
@@ -287,28 +274,38 @@ def tag_options(options, escape = true)
287274
output unless output.empty?
288275
end
289276

290-
def boolean_tag_option(key)
291-
%(#{key}="#{key}")
292-
end
277+
private
278+
def tag_string(name, content = nil, options, escape: true, &block)
279+
content = @view_context.capture(self, &block) if block
293280

294-
def tag_option(key, value, escape)
295-
key = ERB::Util.xml_name_escape(key) if escape
296-
297-
case value
298-
when Array, Hash
299-
value = TagHelper.build_tag_values(value) if key.to_s == "class"
300-
value = escape ? safe_join(value, " ") : value.join(" ")
301-
when Regexp
302-
value = escape ? ERB::Util.unwrapped_html_escape(value.source) : value.source
303-
else
304-
value = escape ? ERB::Util.unwrapped_html_escape(value) : value.to_s
281+
content_tag_string(name, content, options, escape)
305282
end
306-
value = value.gsub('"', "&quot;") if value.include?('"')
307283

308-
%(#{key}="#{value}")
309-
end
284+
def self_closing_tag_string(name, options, escape = true, tag_suffix = " />")
285+
"<#{name}#{tag_options(options, escape)}#{tag_suffix}".html_safe
286+
end
287+
288+
def boolean_tag_option(key)
289+
%(#{key}="#{key}")
290+
end
291+
292+
def tag_option(key, value, escape)
293+
key = ERB::Util.xml_name_escape(key) if escape
294+
295+
case value
296+
when Array, Hash
297+
value = TagHelper.build_tag_values(value) if key.to_s == "class"
298+
value = escape ? @view_context.safe_join(value, " ") : value.join(" ")
299+
when Regexp
300+
value = escape ? ERB::Util.unwrapped_html_escape(value.source) : value.source
301+
else
302+
value = escape ? ERB::Util.unwrapped_html_escape(value) : value.to_s
303+
end
304+
value = value.gsub('"', "&quot;") if value.include?('"')
305+
306+
%(#{key}="#{value}")
307+
end
310308

311-
private
312309
def prefix_tag_option(prefix, key, value, escape)
313310
key = "#{prefix}-#{key.to_s.dasherize}"
314311
unless value.is_a?(String) || value.is_a?(Symbol) || value.is_a?(BigDecimal)

actionview/lib/action_view/helpers/tags/select_renderer.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ def add_options(option_tags, options, value = nil)
3737
if options[:include_blank]
3838
content = (options[:include_blank] if options[:include_blank].is_a?(String))
3939
label = (" " unless content)
40-
option_tags = tag_builder.content_tag_string("option", content, value: "", label: label) + "\n" + option_tags
40+
option_tags = tag_builder.option(content, value: "", label: label) + "\n" + option_tags
4141
end
4242

4343
if value.blank? && options[:prompt]
4444
tag_options = { value: "" }.tap do |prompt_opts|
4545
prompt_opts[:disabled] = true if options[:disabled] == ""
4646
prompt_opts[:selected] = true if options[:selected] == ""
4747
end
48-
option_tags = tag_builder.content_tag_string("option", prompt_text(options[:prompt]), tag_options) + "\n" + option_tags
48+
option_tags = tag_builder.option(prompt_text(options[:prompt]), **tag_options) + "\n" + option_tags
4949
end
5050

5151
option_tags

0 commit comments

Comments
 (0)