In 2abf6ca0c8304a3cfcdae6e14060b561780be43c @cache_hit got introduced.
This was renamed to @cache_hits and revised in the subsequent commit
8240636beda7b2b487217be1d945eb0d36145c4d but it seems one assignment was
overlooked.
This refactor incidentally fixes a corner case when `translate` is
called with a block, the translation is missing, and
`debug_missing_translation` is false.
This commit adds a test for the above corner case, and additional tests
for existing behavior.
Prior to this commit, when a translation key indicated that the
translation text was HTML, the value returned by `I18n.translate` would
always be marked as `html_safe`. However, the value returned by
`I18n.translate` could be an untrusted value directly from
`options[:default]`.
This commit ensures values directly from `options[:default]` are not
marked as `html_safe`.
In applications which use :all helpers (the default), most controllers
won't be making modifications to their _helpers module.
In CRuby this created many ICLASS objects which could cause a large
increase in memory usage in applications with many controllers and
helpers.
To avoid creating unnecessary modules this PR builds modules only when a
modification is being made: ethier by calling `helper`, `helper_method`,
or through having a default helper (one matching the controller's name)
included onto it.
If automatically_disable_submit_tag is set to false then disable_with
is ignored, as result in all cases where disable_with is explicitly set
to false will produce unexpected result
Previously we were using module_eval to manually add some helpers in
ActionView::TestCase, which assumes a lot about what _helpers allows.
Instead, we can use a standard helper block to define these methods only
one time, and add a method to the new uniq controller class to tie back
to the test instance.
Previously the same class, ActionView::TestCase::TestController, was
used to build a controller for every ActionView::TestCase class.
This caused issues when helpers/helper methods were set directly on the
controller (which from one test we seem to want to support).
This commit solves this by creating a new controller class for every
test case, which gives the controller a unique set of helpers to work
with.
Co-authored-by: John Crepezzi <seejohnrun@github.com>
We should avoid overriding name if possible because it is confusing to
developers and it also can causes bugs in code which deal with modules
but aren't using the Module.instance_method(:name).bind workaround.
I believe we can achieve the desired "method missing" behaviour by just
redefining inspect.
Refs https://github.com/rails/rails/pull/39819
All branches that use translated_text are covered so we can remove this
method call.
Also apply some whitespaces around conditionals to make them explicit.
This commit extends the `ActionView::Helpers#translate` (and by way of
alias, `#t`) helper methods to accept blocks.
When invoked with a block, the `translate` call will yield the
translated text as its first block argument, along with the resolved
translation key as its second:
```erb
<%= translate(".key") do |translation, resolved_key| %>
<span data-i18n-key="<%= resolved_key %>"><%= translation %></span>
<% end %>
```
In cases where relative translation keys are foregone in lieu of fully
qualified keys, or if the caller is not interested in the resolved key,
the second block argument can be omitted:
```erb
<%= translate("action.template.key") do |translation| %>
<p><%= translation %></p>
<p><%= translation %>, but a second time</p>
<% end %>
```
A benefit of yielding the translation is that it enabled template-local
variable re-use. Alternatively, [`Object#tap`][tap] could be used.
Prior to this commit, however, the resolution of the translation key was
internal to `ActionView`, and unavailable to the caller (unless they
were willing to explicitly determine the resolved key themselves). By
making it available as a block parameter, it could be used to annotate
the translated value in the resulting elements.
[tap]: https://ruby-doc.org/core-2.7.0/Object.html#method-i-tap
When rendering views an anonymous subclass is created by calling
ActionView::Base.with_empty_template_cache.
This causes inspect to return an unhelpful description when calling
inspect:
`#<#<Class:0x012345012345>:<0x012345012345>`.
This can be confusing when exceptions are raised because it's hard to
figure out where to look. For example calling an undefined method in a
template would raise the following exception:
undefined method `undefined' for #<#<Class:0x012345012345>:<0x012345012345>
Instead we can return the non-anonymous superclass name.
undefined method `undefined' for #<ActionView::Base:0x01234502345>
The anonymous class is created in ActionView::Base.with_empty_template_cache.
See f9bea6304dfba902b1937b3bc29b1ebc2f67e55b
This seems to be done for performance reasons only, without expecting a
change to calling `inspect`.
This pull request fixes#38697
It is caused by `@controller.singleton_class.include @routes.url_helpers` when `@controller` is nil in `ActionController::TestCase`.
* Without this commit
```ruby
% cd actionview
% PARALLEL_WORKERS=1 bin/test test/actionpack/controller/layout_test.rb test/template/url_helper_test.rb --seed 16702 -n "/^(?:LayoutSetInResponseTest#(?:test_layout_symbol_set_in_controller_returning_nil_falls_back_to_default)|UrlHelperTest#(?:test_url_for_with_array_and_only_path_set_to_false))$/"
Run options: --seed 16702 -n "/^(?:LayoutSetInResponseTest#(?:test_layout_symbol_set_in_controller_returning_nil_falls_back_to_default)|UrlHelperTest#(?:test_url_for_with_array_and_only_path_set_to_false))$/"
.E
Error:
UrlHelperTest#test_url_for_with_array_and_only_path_set_to_false:
ArgumentError: Missing host to link to! Please provide the :host parameter, set default_url_options[:host], or set :only_path to true
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/http/url.rb:64:in `full_url_for'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/http/url.rb:54:in `url_for'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:333:in `block in <class:RouteSet>'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:838:in `url_for'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:270:in `call'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:213:in `call'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:326:in `block in define_url_helper'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb:233:in `polymorphic_method'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb:116:in `polymorphic_url'
/Users/yahonda/src/github.com/rails/rails/actionview/lib/action_view/routing_url_for.rb:104:in `url_for'
/Users/yahonda/src/github.com/rails/rails/actionview/test/template/url_helper_test.rb:102:in `test_url_for_with_array_and_only_path_set_to_false'
bin/test test/template/url_helper_test.rb:100
Finished in 0.042275s, 47.3093 runs/s, 47.3093 assertions/s.
2 runs, 2 assertions, 0 failures, 1 errors, 0 skips
%
```
The `:include_blank` option of various `<select>`-related helpers causes
an `<option>` element with no content to be rendered. However, the
[HTML spec] says that unless an `<option>` element has a `label`
attribute (which must be non-empty), its content must be "Text that is
not inter-element whitespace."
In #24923, this issue was addressed for `select_tag` by adding a `label`
attribute to the `<option>`. This commit addresses the issue in the
same manner for `FormBuilder#select` and various date / time select
helpers.
[HTML spec]: https://html.spec.whatwg.org/multipage/form-elements.html#the-option-element
This change swaps the CommonJS require() syntax in the Webpacker
application.js pack template file and in documentation examples with ES
module import syntax.
Benefits of this change include:
Provides continuity with the larger frontend community: Arguably, one of
the main draws in adopting Webpacker is its integration with Babel to
support ES module syntax. For a fresh Rails install with Webpacker, the
application.js file will be the first impression most Rails developers
have with webpack and Webpacker. Most of the recent documentation and
examples they will find online for using other libraries will be based
on ES module syntax.
Reduces confusion: Developers commonly add ES imports to their
application.js pack, typically by following online examples, which means
mixing require() and import statements in a single file. This leads to
confusion and unnecessary friction about differences between require()
and import.
Embraces browser-friendliness: The ES module syntax forward-looking and
is meant to be supported in browsers. On the other hand, require()
syntax is synchronous by design and not browser-supported as CommonJS
originally was adopted in Node.js for server-side JavaScript. That
webpack supports require() syntax is merely a convenience.
Encourages best practices regarding optimization: webpack can statically
analyze ES modules and "tree-shake", i.e., strip out unused exports from
the final build (given certain conditions are met, including
`sideEffects: false` designation in package.json).
Follow up to c07dff72278fb7f2a3c4c71212a0773a2b25c790.
Actually it is not the cop's fault, but we mistakenly use `^`, `$`, and
`\Z` in much places, the cop doesn't correct those conservatively.
I've checked all those usage and replaced all safe ones.
When rendering a template with an explicit JS format, typically via
`respond_to :js`, we want to be able to render HTML partials without
having to specify their format, in order to make SJR more ergonomic.
Following the introduction of the @current_template variable in 1581cab,
the @virtual_path variable is now redundant, as the value of the virtual
path may be accessed via @current_template.virtual_path. This commit
removes @virtual_path and replaces any references to @virtual_path with
@current_template.virtual_path.
A Rails view may rely on several templates (e.g. layouts and partials)
in addition to the template for the action being rendered (e.g.
"show.html.erb"). To track which view file is currently being rendered
for the purpose of generating template tree digests used in cache
fragment keys, Action View uses a stack, the top item of which is
accessed via the @current_template variable (introduced in 1581cab).
Consider the following template:
<!-- home.html.erb -->
<%= render layout: "wrapper" do %>
<%= cache "foo" %>
HOME
<%= end %>
<%= end %>
Inside the block passed to the render helper, @current_template
corresponds to the wrapper.html.erb template instead of home.html.erb.
As wrapper.html.erb is then used as the root node for generating the
template tree digest used in the cache fragment key, the cache fragment
fails to expire upon changes to home.html.erb. Additionally, should a
second template use the wrapper.html.erb layout and contain a cache
fragment with the same key, the cache fragment keys for both templates
will be identical - causing cached content to "leak" from one view to
another (as described in #38984).
This commit skips adding templates to the stack when rendered as a
layout with a block via the render helper, ensuring correct and unique
cache fragment digests. Additionally, the virtual_path keyword arguments
found in CacheHelper and all references to the are removed as they no
longer possess any function. (Following the introduction of
@current_template, virtual_path is accessed via
@current_template.virtual_path rather than as a standalone variable.)
This allows `render_in` components that compile their own templates to
use their view context's current output buffer while a
`with_output_buffer` block is being evaluated.
Partially addresses #39377.
The template resolver is supposed to determine the handler, format, and
variant from the path in extract_handler_and_format_and_variant.
Previously this behaviour was close but didn't exactly match the
behaviour of finding templates, and in some cases (particularly with
handlers or formats missing) would return incorrect results.
This commit introduces Resolver::PathParser, a class which should be
able to accurately from any path inside a view directory be able to tell
us exactly the prefix, partial, variant, locale, format, variant, and
handler of that template.
This works by building a building a regexp from the known handlers and
file types. This requires that any resolvers have their cache cleared
when new handlers or types are registered (this was already somewhat the
requirement, since resolver lookups are cached, but this makes it
necessary in more situations).
The template resolver is supposed to determine the handler, format, and
variant from the path in extract_handler_and_format_and_variant.
This commit adds tests for how variants are parsed from the filenames
found in the resolver. It includes two skipped tests which currently fail.
- Add the configuration option for annotating templates with file names to the generated app.
- Add `annotate_rendered_view_with_filenames` option to configuring guide.
* master-sec:
Check that request is same-origin prior to including CSRF token in XHRs
HMAC raw CSRF token before masking it, so it cannot be used to reconstruct a per-form token
activesupport: Avoid Marshal.load on raw cache value in RedisCacheStore
activesupport: Avoid Marshal.load on raw cache value in MemCacheStore
Return self when calling #each, #each_pair, and #each_value instead of the raw @parameters hash
Include Content-Length in signature for ActiveStorage direct upload
Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.
This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.
This was actually once deprecated in the 3.x series (removed in
6c57177f2c7f4f934716d588545902d5fc00fa99) but I don't think we can rely
on nobody having introduced this in the past 8 years.
In the past, we sometimes hit missing `Symbol#start_with?` and
`Symbol#end_with?`.
63256bc5d7a8e812964d
So I proposed `Symbol#start_with?` and `Symbol#end_with?` to allow duck
typing that methods for String and Symbol, then now it is available in
Ruby 2.7.
https://bugs.ruby-lang.org/issues/16348
Using `String#starts_with?` and `String#ends_with?` could not be gained
that conveniency, so it is preferable to not use these in the future.
ActionView::DependencyTracker looks through ERB templates using a regex
to find render calls. Previously this would incorrectly pick up
interpolated strings, like `render "foo/#{bar}"`.
This does not attempt to completely correct DependencyTracker, we can't
parse Ruby accurately with a regex, but should avoid a relatively common
case that previously was generating warnings.
I wrote this shell script to find words from the Rails repo,
so I can paste them into https://www.horsepaste.com/ for
the [codenames game](https://en.m.wikipedia.org/wiki/Codenames_(board_game)).
```bash
git grep -Il '' | \
grep -v -E "CHANGELOG|Gemfile|gemspec|package\.json|yarn\.lock" | \
xargs cat | \
sed '/[^ ]\{10,\}/d' | \
sed 's/\([A-Z]\)/ \1/g' | \
tr 'A-Z' 'a-z' | \
tr -c -s 'a-z' '\n' | \
sed '/^.\{0,3\}$/d' | \
sort | \
uniq | \
tr '\n' ',' | \
pbcopy
```
You can see the result in https://www.horsepaste.com/rails-fixed.
Click "Next game" to cycle the words.
Found some typos in the codebase from this 😂
This is how I generated the list of possible typos:
```bash
git grep -Il '' | \
grep -v -E "CHANGELOG|Gemfile|gemspec|package\.json|yarn\.lock" | \
xargs cat | \
sed '/[^ ]\{10,\}/d' | \
sed 's/\([A-Z]\)/ \1/g' | \
tr 'A-Z' 'a-z' | \
tr -c -s 'a-z' '\n' | \
sed '/^.\{0,3\}$/d' | \
sort | \
uniq | \
aspell --ignore-case list
```
I manually reviewed the list and made the corrections
in this commit. The rest on the list are either:
* Bugs in my script: it split things like "doesn't" into
"doesn" and "t", if find things like `#ffffff` and
extracts "ffffff" as a word, etc
* British spelling: honour, optimised
* Foreign words: bonjour, espanol
* Names: nginx, hanekawa
* Technical words: mutex, xhtml
* Portmanteau words: autosave, nodelist
* Invented words: camelize, coachee
* Shortened words: attrs, repo
* Deliberate typos: hllo, hillo (used in code examples, etc)
* Lorem ipsum words: arcu, euismod
This is the [output](https://gist.github.com/chancancode/eb0b573d667dc31906f33f1fb0b22313)
of the script *after* fixing the typos included in this
commit. In theory, someone can run that command again in
the future and compare the output to catch new typos (i.e.
using my list to filter out known typos).
Limitations: the aspell dictionary could be wrong, I
could have miss things, and my script ignores words that
are less than 3 characters or longer than 10 characters.
In testing https://github.com/rails/rails/pull/38848 in the
GitHub monolith, we realized that we probably should only
be annotating HTML output with these comments, at least
in their current format. By passing `format` to
`erb_implementation`, we set ourselves up to eventually
support annotations for other formats as well.
Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.
This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.
This was actually once deprecated in the 3.x series (removed in
6c57177f2c7f4f934716d588545902d5fc00fa99) but I don't think we can rely
on nobody having introduced this in the past 8 years.
The instrument helper we had here would allocate a new string object for
the key each time it was called and also added two stack frames. The
interpolated strings are also _slightly_ slower to hash since they
aren't fstrings.
Though this was shorter before, I think this is a little more preferable
as a code style since the subscribed keys are now greppable. After this,
there's still one other place this pattern is used (in ActiveSupport
cache).
As a developer, when looking at a page in my web browser, it's sometimes
difficult to figure out which template(s) are being used to render the page.
config.action_view.annotate_template_file_names adds HTML comments to the
rendered output indicating where each template begins and ends.
Co-authored-by: Aaron Patterson <tenderlove@github.com>
* master:
Add a regression test that ActionText caught
[ci skip] Use yml extension for locale files
Fix `helper_method` in `ActionView::TestCase` to allow keyword arguments
Fix `delegate_missing_to` to allow keyword arguments
Dump the schema or structure of a database when calling db:migrate:name
Reset the `ActiveRecord::Base` connection after `rails db:migrate:name`
Fix `unscope` when an `eq` node which has no arel attribute
Remove unused argument
Disallow calling `connected_to` on subclasses of `ActiveRecord::Base`
More less and lazy allocation for `assign_attributes` and `_assign_attributes`
Tweak contributing_to_ruby_on_rails.md [ci skip]
Clarify the difference between (old) `spec_name` and `connection_specification_name`
Remove duplicate part from deprecation warning
Fix deprecation warnings in connection_handlers_sharding_db_test.rb
Fixup CHANGELOGs [ci skip]
`reset_column_information` does not reset @predicate_builder
Simplify FixtureResolver to reuse filtering logic
Mostly remove bad test
Use type attribute in ActionView::Helpers::JavaScriptHelper#javascript_tag example
Update some references to finder options [ci skip]
Details are only used if the user specifies a format or local, etc when
rendering a template. This is a fairly uncommon thing to do, so lets
make it cheaper when there are no details specified
Partial paths are only required for deriving template info for rendering
objects, which is only the case for the object renderer and the
collection renderer
The local variable name in `as:` may not be a valid local variable name,
but if there is no object specified to be assigned to the parameter,
then why supply the `as:`? This commit adds an object for the as param
If the partial renderer is passed a relation or collection proxy, it
will be good for us to resolve that relation or collection proxy as late
as possible.
Previously FixtureResolver had a copy-pasted version of the filtering
already done by OptimizedFileSystemResolver. This PR replaces this by
extracting the two places actual filesystem operations into separate
methods and overriding those.
It would be nice to not rely on overriding methods at all, and to
extract the actual filtering into a separate, reusable class, but I
don't want to do that until some other changes are made to the
filtering.
This should also make FixtureResolver much more accurate to
OptimizedFileSystemResolver, by also creating and caching the
UnboundTemplate classes, which de-duplicate templates.
This test was attempting to test how cache keys work by modifying the
templates and seeing when that cache was fresh. This doesn't actually
work for real Resolvers, only FixtureResolver, and isn't desirable. We
absolutely want to share templates if they resolve to the same file.
Instead, this simplifies the test to only check that we get the correct
template for the locale we request.
Followup to 88fe76e69328d38942130e16fb65f4aa1b5d1a6b.
These are new in RuboCop 0.80.0, and enforce a style we already prefer
for performance reasons (see df81f2e5f5df46c9c1db27530bbd301b6e23c4a7).
We're moving away from using validations in our
component framework, and feel that it's better
to avoid prescribing their usage in these
example classes, which exist to serve as example
objects that are compatible with the render_in
API.
We need a more complete understanding of mime types for the asset tags
to render properly, so we need to load this rather than just the stubs
included by actionview by default.
This fixes running these tests in isolation.
Before 2169bd3d2a9d2f331a5dd6e41d9d638e0da6117c, a template's source was
encoded in place when it was compiled, and the `source_extract` method
could rely on `Template#source` to return a properly encoded string.
Now that `Template#source` always returns a new copy of the template
source with no encoding, `source_extract` should call `encode!` itself.
- ### Problem
ActionPack requires "action_view/base" at boot time, this
causes a variety of issue that I described in detail in #38024.
There is no real reason to require av/base in the
ActionDispatch::Debugexceptions class.
### Solution
Like any other components (such as ActiveRecord, ActiveJob...),
ActionView::Base shouldn't be loaded at boot time.
Here are the two main changes needed for this:
1) Actionview has a special initializer that needs to run
before the app is fully booted (adding a executor needs to be done
before application is done booting)
63ec70e700/actionview/lib/action_view/railtie.rb (L81-L84)
That initializer used a lazy load hooks but we can't do that anymore
because Action::Base view won't be triggered during booting process.
When it will get triggered, (presumably on the first request),
it's too late to add an executor.
------------------------------------------------
2) Compare to other components, ActionView doesn't use `Base` for
configuration flag. A lot of flags ares instead set on modules
(FormHelper, FormTagHelper).
The problem is that those module depends on AV::Base to be
loaded, as otherwise configuration set by the user aren't applied.
(Since the lazy load hooks hasn't been triggered)
63ec70e700/actionview/lib/action_view/railtie.rb (L66-L69)
We shouldn't wait for AB::Base to be loaded in order to set these
configuration. However, we need to do it inside an
`after_initialize` block in order to let application
set it to the value they want.
Closes#28538
Co-authored-by: betesh <iybetesh@gmail.com>"
In https://github.com/rails/rails/pull/36388,
we supported passing objects that `respond_to` `render_in`
to `render`, but _only_ in views.
This change does the same for controllers, as Rails
generally gives the expectation that `render` behaves
the same in both contexts.
Co-authored-by: Aaron Patterson <tenderlove@github.com>
As a follow-up to https://github.com/rails/rails/pull/37872,
this change introduces a class_names view helper
to make it easier to conditionally apply class names
in views.
Before:
<div class="<%= item.for_sale? ? 'active' : '' %>">
After:
<div class="<%= class_names(active: item.for_sale?) %>">
We've been using this helper in the GitHub monolith
since 2016.
Co-authored-by: Aaron Patterson <tenderlove@github.com>
- #37872 introduced a regression and you can't do
```html.erb
hidden_field_tag('token', value: [1, 2, 3])
```
This will result in a `<input type="hidden" value=""`>.
I chose `hidden_field_tag` and the `value` attribute as an example
but this issue applies to any tag helper and any attributes.
https://github.com/rails/rails/pull/37872#issuecomment-561806468
mention that the feature should only apply for "class" attribute.
This commit fix original intent of #37872
This reverts commit 4e105385d046e2aeab16955943df97c5eefa3a6f, reversing
changes made to 62b43839098bbbbfc4be789128d33dc0612f1ab3.
The change in Ruby that made those changes required was reverted in
8852fa8760
Adds support for conditional values to TagBuilder,
extracting logic we use in the GitHub application,
inspired by https://github.com/JedWatson/classnames.
It’s common practice to conditionally apply CSS classes
in Rails views. This can lead to messy string interpolation,
often using ternaries:
```ruby
content_tag(
"My username",
class: "always #{'sometimes' if current_user.special?} another"
)
```
By adding support for hashes to TagBuilder, we can instead write the following:
```ruby
content_tag(
"My username",
class: ["always", "another", { 'sometimes' => current_user.special? }]
)
```
cc @JedWatson
