If one of the locals conflict with a keyword, typically `class`.
The potentially confusing part however is that if you define a default
value, `local_assigns` won't respect it.
This is getting the same treatment as `base64`, `mutex_m`, etc.
In Ruby 3.4 it will start to warn: d7e558e3c4
Remoce require from two files that don't seem to need it
Allocations count is often an interesting proxy for performance,
but not necessarily the most relevant thing to include in request
logs, given they aren't a per thread metric, so the reporting
is widely innacurate in multi-threaded environments.
Since Ruby 3.1 there is now `GC.total_time` which is a monotonically
increasing counter of time spent in GC. It still isn't really a per
thread metric, but is is more interesting because it uses the same
unit as the response time, allowing to better see when you have a GC
pause performance issue.
Follow-up to [#49819][]
Since [#49819][] resolves an issue with
`ActionDispatch::IntegrationTest#with_routing` helper support, Action
View's `test/abstract_unit.rb` file can rely on routing being reset
within the block argument.
This means that the `RoutedRackApp` class and `.build_app` method is can
be made unnecessary.
[#49819]: https://github.com/rails/rails/pull/49819
Ref: https://github.com/rails/rails/pull/8222
`mathn` used to be in the standard library, hence this was a
relatively common issue. But it has been extracted and deprecated
a long time ago, so It's no longer a big concern.
Making `Integer#/` private now emits a warning on Ruby 3.4:
```
/rails/actionview/test/template/date_helper_test.rb:148: warning: Redefining 'Integer#/' disables interpreter and JIT optimizations
```
`ostruct` was being implictly required by the `json` gem. But once it
was upgraded, these tests failed to initialize `OpenStruct`.
While we're trying to remove `ostruct` usage in #51510, CI is currently
failing so I'm pushing these in the mean time.
Replaced by `#lease_connection` to better reflect what it does.
`ActiveRecord::Base#connection` is deprecated in the same way
but without a removal timeline nor a deprecation warning.
Inside the Active Record test suite, we do remove `Base.connection`
to ensure it's not used internally.
Some callsites have been converted to use `with_connection`,
some other have been more simply migrated to `lease_connection`
and will serve as a list of callsites to convert for
https://github.com/rails/rails/pull/50793
Follow-up to [49856][]
Follow-up to [49194][]
The introduction of memoization as an optimization posed a backwards
incompatible change to View tests that call `render` multiple times.
This commit changes the `@rendered` instance variable from a `String` to
an instance of the `RenderedViewContent` specialized `String` subclass.
The end result is that there is no memoization to reset, and the
memoization optimization side-effect is preserved after rendering for
test cases where `rendered` (or parser methods like `rendered.html`)
might be invoked more than once.
[49856]: https://github.com/rails/rails/pull/49856#issuecomment-1945039015
[49194]: https://github.com/rails/rails/pull/49194/files#diff-ce84a807f3491121a5230d37bd40454bb1407fcca71179e1a2fa76d4c0ddfa2aR293
Fix: https://github.com/rails/rails/issues/50930
While Action View is predominantly meant to render text,
in sime case it's used to render more complex object.
So we shouldn't assume `#_run` returns a buffer.
Minitest::Test does not support all of the same options as
ActiveSupport::TestCase, such as running bin/test <filename>:<lineno>
and -n /regex/. Trying to use these options on this file would just run
all of the Minitest::Tests no matter what options were passed.
This commit fixes the ability to use those options by using
ActiveSupport::TestCase (like every other test in the repo).
Before:
```
$ bin/test test/template/dependency_tracker_test.rb:217
Running 59 tests in parallel using 8 processes
Run options: --seed 42725
.........................................................
Finished in 0.322759s, 176.6024 runs/s, 176.6024 assertions/s.
57 runs, 57 assertions, 0 failures, 0 errors, 0 skips
```
After:
```
$ bin/test test/template/dependency_tracker_test.rb:217
Running 59 tests in parallel using 8 processes
Run options: --seed 15213
...
Finished in 0.359162s, 8.3528 runs/s, 8.3528 assertions/s.
3 runs, 3 assertions, 0 failures, 0 errors, 0 skips
```
Motivation / Background
---
Strict Locals support was introduced in [#45727][] and announced as part
of the [7.1 Release][]. There are several mentions across the Guides,
but support is rarely mentioned in the API documentation.
Detail
----
Mention the template short identifier (the pathname, in most cases) as
part of the `ArgumentError` message.
This commit adds two test cases to ensure support for splatting
additional arguments, and for forbidding block and positional arguments.
It also makes mention of strict locals in more places, and links to the
guides.
[#45727]: https://github.com/rails/rails/pull/45727
[7.1 Release]: https://edgeguides.rubyonrails.org/7_1_release_notes.html#allow-templates-to-set-strict-locals
Previously, only the PrismRenderParser or RipperRenderParser would be
tested depending on if the Prism gem is available. This meant that
PrismRenderParser was being tested on Ruby 3.3 and RipperRenderParser
was tested on Ruby < 3.3. Additionally, if someone were to add prism to
the rails/rails Gemfile because they wrote a tool that uses it then the
RipperRenderParser would end up completely untested.
This commit is a small refactor to enable testing both RenderParsers in
all Ruby versions so that the prism gem can be added to the Gemfile.
Currently there's about a 35% difference between tags generated using
the `TagBuilder` and tags generated by passing a positional argument to
`#tag`.
This commit optimizes `TagBuilder` to reduce that difference down to 13%.
The first change is to perform less hash allocations by not splatting
the options twice in the `TagBuilder` (one at the `tag.a` invocation,
and one at `tag_string`). The extra splat for `tag_string` was moved
into `method_missing` since that is the only other caller of this
private method.
The other change is to only escape the content in `tag_string` if it a
non-empty.
Additionally, a test was tweaked to ensure that passing `options` to a
`self_closing_element` is tested as it was previously not.
Benchmark:
```
require "action_view"
require "benchmark/ips"
class Foo
include ActionView::Helpers
end
helpers = Foo.new
Benchmark.ips do |x|
x.report("tag") { helpers.tag("a", href: "foo") }
x.report("tag_builder") { helpers.tag.a(href: "foo") }
x.compare!
end
```
Before:
```
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]
Warming up --------------------------------------
tag 67.180k i/100ms
tag_builder 50.267k i/100ms
Calculating -------------------------------------
tag 673.064k (± 0.4%) i/s - 3.426M in 5.090520s
tag_builder 504.971k (± 0.4%) i/s - 2.564M in 5.076842s
Comparison:
tag: 673063.7 i/s
tag_builder: 504971.4 i/s - 1.33x slower
```
After:
```
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]
Warming up --------------------------------------
tag 67.374k i/100ms
tag_builder 59.702k i/100ms
Calculating -------------------------------------
tag 670.837k (± 0.4%) i/s - 3.369M in 5.021714s
tag_builder 592.727k (± 1.3%) i/s - 2.985M in 5.037088s
Comparison:
tag: 670836.6 i/s
tag_builder: 592726.7 i/s - 1.13x slower
```
Co-authored-by: Sean Doyle <seanpdoyle@users.noreply.github.com>
Follow-up to #50699.
This prevents a `NoMethodError` from being masked when the missing
method is itself named `render_in`.
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
Follow-up to #50665.
Unconditionally converting `NoMethodError` to `ArgumentError` can mask a
legitimate `NoMethodError` from within the `render_in` method. This
commit adds a check to prevent that.
When calling `render` with a `:renderable` argument, ensure that the
object responds to `#render_in`. If it doesn't, raise an
`ArgumentError`.
This commit also adjusts the `ArgumentError` that when a `:partial`
argument isn't Active Model compatible. Prior to this commit, the
message used `:` as a prefix to `to_partial_path`. This commit replaces
that with a `#` prefix to denote that it's expected to be an instance
method on the object.
Provide examples for rendering objects that respond to `render_in`. Also
highlight that the object can also define a `#format` method to control
how the rendered String should be treated.
Add test coverage for both Action View's and Action Pack's support for
`render` with `:renderable` options.
This provides a shortcut for setting a Content Security Policy nonce on
a stylesheet_link_tag.
Co-authored-by: AJ Esler <ajesler@users.noreply.github.com>
To integrate with [rails-dom-testing][] and its selector assertions,
`ActionView::TestCase` [defines a `#document_root_element`
method][document_root_element] that parses the HTML into a fully valid
HTML document and returns the "root".
In the case of most Action View partials rendered with `render partial:
"..."`, the resulting document would be invalid, so its constituent
parts (its `<html>`, `<head>`, and `<body>` elements) are synthesized in
during the parsing process. This results in a document whose _contents_
are equivalent to the original HTML string, but whose structure is not.
To share a concrete example:
```ruby
irb(main):002:0> rendered = "<h1>Hello world</h1><h2>Goodbye world</h2>"
=> "<h1>Hello world</h1><h2>Goodbye world</h2>"
irb(main):003:0> root = Rails::Dom::Testing.html_document.parse(rendered).root
=>
#(Element:0x57080 {
...
irb(main):004:0> rendered.to_s
=> "<h1>Hello world</h1><h2>Goodbye world</h2>"
irb(main):005:0> root.to_s
=> "<html><head></head><body><h1>Hello world</h1><h2>Goodbye world</h2></body></html>"
irb(main):006:0> rendered.to_s == root.to_s
=> false
```
Prior to this commit, the parsed HTML content returned from calling
`rendered.html` relied on the same mechanisms as
`#document_root_element`, and parsed the HTML fragment into a full
document, with a synthesized `<html>` element as its root. The
`rendered.html` value should reflect the content that was **rendered**
by the partial, and should not behave the same as
`#document_root_element`.
When the parsing class is changed from [Nokogiri::XML::Document][] to
[Nokogiri::XML::DocumentFragment][], the returned value reflects the
same **exact** content as what was rendered.
To elaborate on the previous example:
```ruby
irb(main):007:0> fragment = Rails::Dom::Testing.html_document_fragment.parse(rendered)
=>
#(DocumentFragment:0x62ee4 {
...
irb(main):008:0> fragment.to_s
=> "<h1>Hello world</h1><h2>Goodbye world</h2>"
irb(main):009:0> rendered.to_s == fragment.to_s
=> true
```
This commit changes the default `rendered.html` behavior to rely on
`Nokogiri::XML::DocumentFragment` instead of `Nokogiri::XML::Document`.
[Nokogiri::XML::Document]: https://nokogiri.org/rdoc/Nokogiri/XML/Document.html
[Nokogiri::XML::DocumentFragment]: https://nokogiri.org/rdoc/Nokogiri/XML/DocumentFragment.html
[document_root_element]: https://github.com/rails/rails-dom-testing/blob/v2.2.0/lib/rails/dom/testing/assertions/selector_assertions.rb#L75
Closes#49818
Renames `ActionView::TestCase::Behavior::Content` to
`RenderedViewContent`, with the goal of making it more of an internal
implementation detail that's unlikely to collide with an
application-side `::Content` class.
The `RenderedView`-prefix mirrors the module's `RenderedViewsCollection`
class. Since the intention is to treat it as a private implementation
detail, `RenderedViewContent` is marked with `:nodoc:`.
Along with the rename, this commit also modifies the class inheritance,
replacing the `SimpleDelegator` superclass with `String`. [String.new][]
accepts a `String` positional argument in the same way as
`SimpleDelegator.new` accepts a delegate object positional argument.
Sharing the `String` superclass also makes it a good candidate for being
passed to [Capybara.string][] (and [Capybara::Node::Simple.new][]) like
the documentation suggests.
[Capybara.string]: https://github.com/teamcapybara/capybara/blob/3.39.2/lib/capybara.rb#L212-L242
[Capybara::Node::Simple.new]: https://github.com/teamcapybara/capybara/blob/3.39.2/lib/capybara/node/simple.rb#L23
[String.new]: https://ruby-doc.org/core/String.html#method-c-new
Since `ActionText::Content` wraps an `ActionText::Fragment`, and
`ActionText::Fragment` wraps a `Nokogiri::XML::DocumentFragment`, then
`ActionText::Content` should be able to rely on the newer Ruby pattern
matching introduced by [nokogiri@1.16.0][] (mainly the
[DocumentFragment#deconstruct][] method):
```ruby
content = ActionText::Content.new <<~HTML
<h1>Hello, world</h1>
<div>The body</div>
HTML
content => [h1, div]
assert_pattern { h1 => { content: "Hello, world" } }
assert_pattern { div => { content: "The body" } }
```
The implementation change relies on delegating from `Content` to
`Fragment`, and from `Fragment` to `DocumentFragment#elements` (to
deliberately exclude text nodes).
[nokogiri@1.16.0]: https://nokogiri.org/CHANGELOG.html?h=pattern
[DocumentFragment#deconstruct]: https://nokogiri.org/rdoc/Nokogiri/XML/DocumentFragment.html?h=deconstruct#method-i-deconstruct
To assert the expected number of queries are made, Rails internally uses
`assert_queries` and `assert_no_queries`. These assertions can be
useful in applications as well.
By extracting these assertions to a module, the assertions can be
included where required.
These assertions are added to `ActiveSupport::TestCase` when
ActiveRecord is defined.
ActiveStorage, ActionView and ActionText are using this module now as
well, instead of duplicating the implementation.
The internal ActiveRecord::TestCase, used for testing ActiveRecord,
implements these assertions as well. However, these are slighlty more
advanced/complex and use the SQLCounter class. To keep things simple,
for now this implementation isn't used.
According to the [HTML5 Spec][1]
> Void elements can't have any contents (since there's no end tag, no
> content can be put between the start tag and the end tag).
Up to this point, the only special handling of void elements has been to
use ">" to close them instead of "/>" (which is optional but valid
according to the spec)
> Then, if the element is one of the void elements, ... , then there may
> be a single U+002F SOLIDUS character (/), ... . On void elements, it
> does not mark the start tag as self-closing but instead is unnecessary
> and has no effect of any kind.
This commit deprecates the ability to pass content (either through the
positional "content" parameter or a block) to a void element since it is
not valid according to the Spec. This has the benefit of both
encouraging more correct HTML generation, and also simplifying the
method definition of void elements once the deprecation has been
removed.
This commit additionally tweaks the signature of "void_tag_string"
slightly with two changes. The first change is renaming it to be
"self_closing_tag_string". This is more accurate than "void_tag_string"
because the definition of "void element" is more specific and has more
requirements than "self closing element". For example, tags in the SVG
namespace _can_ be self closing but the "/" at the end of the start tag
is _not_ optional because they are not void elements. The second change
to this method is swapping from a boolean "self_closing" parameter to a
string "tag_suffix" parameter. This enables the void element method
definition to specialize the tag_suffix (to just ">") without either
void elements or self closing elements having to pay the runtime cost of
the self_closing conditional since we know at method definition time
which suffix each type of tag should use.
[1]: https://html.spec.whatwg.org/multipage/syntax.html#void-elements
Prior to this commit, if `app/javascript/rails-ujs/index.js`
contained a syntax error, `JavascriptPackageTest` would still pass
because `system "yarn build"` would simply return `false` and the
compiled output would not change. This commit adds `exception: true` to
the `system` call so that an error will be raised if `yarn build` fails.
Previously, both the `@rendered_format` and
`@marked_for_same_origin_verification` instance variables would be
assigned to instances of `ActionView::Base`, making them accessible in
view templates. However, these instance variables are really internal to
the controller and result in extra string allocations because the `@`
gets stripped and readded when going through the assignment.
This commit prefixes the variables with an underscore to help indicate
that they are internal, and then adds them to the list of
`_protected_ivars` to prevent assigning them when rendering templates.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Fix: https://github.com/rails/rails/pull/49780
Fix: https://github.com/rails/rails/issues/49761
`CollectionRenderer` implictly inject `<name>_counter` and `<name>_iteration`
locals, but in the overwhelming majority of cases they aren't used.
So when the rendered template has strict locals we shouldn't require
these to be declared, and if they aren't we should simply not pass them.
Co-Authored-By: HolyWalley <yakau@hey.com>
