This hack prevails everywhere in the codebase by being copy & pasted, and it's actually not a negative thing but a necessary thing for framework implementors,
so it should better have a name and be a thing.
And with this commit, activesupport/test/abstract_unit.rb now doesn't silently autoload AS::TestCase,
so we're ready to establish clearner environment for running AS tests (probably in later commits)
* Added a phone_to helper method, on the style of mail_to and sms_to.
It creates an anchor tag with the href set to tel: *here your number*
which, when clicked on a mobile phone, or on a desktop with a supported
application, lets the phone app kick in, and it prepopulates it with the
phone number specified.
[Pietro Moro + Rafael Mendonça França]
The source_extract method will return nil when it can't find the file name in
the backtrace, methods that consume this method expect an array and the nil ends
up causing type errors down the road like it happened here: #36341. This
patch refactors the source_extract method so that it returns an empty
array instead of nil when it can't find the source code.
Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
When a SyntaxError is detected in a template we raise this exception. On
a first request to the server the exception we get a NameError since the
exception is not required from `active_view/template/error.rb` yet.
However later on it gets required and a second request will succeed.
On the first request we see the rails "Something Wen Wrong" page and not
the expected syntax error in template error page with the webconsole and
stacktrace. By autoloading the constant we fix this issue.
Co-authored-by: Gannon McGibbon <gannon.mcgibbon@gmail.com>
It is sometimes expected of the `translate` methods to return a Hash,
for instance it's the case of the `number.format` key.
As such users might need to specify a Hash default, e.g.
`translate(:'some.format', default: { separator: '.', delimiter: ',' })`.
This works as expected with the `I18n.translate` methods,
however `TranslationHelper#translate` apply `Array()` on the default value.
As a result the default value end up as `[:separator, '.', :delimiter, ',']`.
This is really a nit pick, but as this is the framework's documentation
I think it should follow standards as many times as possible to avoid
confusion in new users.
If we were using `resources :articles` in routes. which is what scaffold
adds, the generated helper would be `new_article_path` instead of
`new_articles_path`.
We shouldn't modify fixtures (or any files which are checked-in). It
prevents us from parallelizing, and probably has other issues.
We could fix these tests by copying the file to a tmpdir and modifying
it there, but I don't think they are testing anything useful anymore.
Re-initializing a resolver isn't representative of "uncached" rendering
(either in dev-mode or using lookup_context.disable_cache).
Currently, `clear_cache_if_necessary` is executed even if view paths are
not set like `rails console`.
If the watcher class is `EventedFileUpdateChecker` and the watch
directories are empty, the application root directory will watch. This
is because listen uses the current directory as the default watch directory.
8d85b4cd57/lib/listen/adapter/config.rb (L13)
As a result, `node_modules` also watch. This cause a warning of `listen`.
Ref: https://github.com/rails/rails/pull/36377#issuecomment-498399576
Since #35709, `Response#conten_type` returns only MIME type correctly.
It is a documented behavior that this method only returns MIME type, so
this change seems appropriate.
39de7fac05/actionpack/lib/action_dispatch/http/response.rb (L245-L249)
But unfortunately, some users expect this method to return all
Content-Type that does not contain charset. This seems to be breaking
changes.
We can change this behavior with the deprecate cycle.
But, in that case, a method needs that include Content-Type with
additional parameters. And that method name is probably the
`content_type` seems to properly.
So I changed the new behavior to more appropriate `media_type` method.
And `Response#content_type` changed (as the method name) to return Content-Type
header as it is.
Fixes#35709.
[Rafael Mendonça França & Yuuji Yaginuma ]
The `number_to_human_size` helpers in Action View and Active Support
calculate the "human size" with a base of 1024. The examples should
reflect that so they don't confuse the reader.
The updated documentations use the values from:
helper.number_to_human_size(1500)
The templates rendered in RenderTestCases tests will be cached by the
resolvers unexpectedly. And this will break other tests when executed in
certain order. (See https://github.com/rails/rails/issues/36154 for more
detail)
So to fix this issue, we just need to clear the caches on all resolvers.
Performance cops will be extracted from RuboCop to RuboCop Performance
when next RuboCop 0.68 will be released.
https://github.com/rubocop-hq/rubocop/issues/5977
RuboCop 0.67 is its transition period.
Since rails/rails repository uses Performance cops, This PR added
rubocop-performance gem to Gemfile.
And this PR fixes some offenses using the following auto-correct.
```console
% bundle exec rubocop -a
Offenses:
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:212:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator =
> should be surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:239:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator => should be
surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
actionview/test/template/resolver_shared_tests.rb:1:1: C: [Corrected]
Style/FrozenStringLiteralComment: Missing magic comment #
frozen_string_literal: true.
module ResolverSharedTests
^
actionview/test/template/resolver_shared_tests.rb:10:33: C: [Corrected]
Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in
default value assignment.
def with_file(filename, source="File at #{filename}")
^
actionview/test/template/resolver_shared_tests.rb:106:5: C: [Corrected]
Rails/RefuteMethods: Prefer assert_not_same over refute_same.
refute_same a, b
^^^^^^^^^^^
2760 files inspected, 5 offenses detected, 5 offenses corrected
```
This adds a bit of complexity, but is necessary for now to avoid holding
extra copies of templates which are resolved from ActionView::Digestor
after disabling cache on the lookup context.
Previously it's possible to have multiple copies of the "same" Template.
For example, if index.html.erb is found both the :en and :fr locale, it
will return a different Template object for each. The same can happen
with formats, variants, and handlers.
This commit de-duplicates templates, there will now only be one template
per file/virtual_path/locals tuple.
We need to consider virtual_path because both `render "index"`, and
`render "index.html"` can both find the same file but will have
different virtual_paths. IMO this is rare and should be
deprecated/removed, but it exists now so we need to consider it in order
to cache correctly.
This commit introduces a new UnboundTemplate class, which represents a
template with unknown locals. Template objects can be built from it by
using `#with_locals`. Currently, this is just a convenience around
caching templates, but I hope it's a helpful concept that could have
more utility in the future.
We didn't previously have many tests directly against the
OptimizedFileSystemResolver or FileSystemResolver, though usually
failures would be exposed through other tests.
It's easier to test some specifics of the behaviour with unit tests.
This also lets us test FileSystemResolver (non-optimized) which I don't
think previously had much testing (other than from classses inheriting
it).
Previously, we would discard the template source after rendering, if we
had a virtual path, in hopes that the virtual path would let us find our
same template again going through the Resolver.
Previously we discarded the source as an optimization, to avoid keeping
it around in memory. By instead just reading the file every time source
is called, as FileTemplate does, this is unnecessary.
Mainly to help with knowning which template is reponsible for the
warning.
handler.class # => Class
handler.to_s # => Coffee::Rails::TemplateHandler
Before:
Change:
>> Class#call(template)
To:
>> Class#call(template, source)
After:
Change:
>> Coffee::Rails::TemplateHandler.call(template)
To:
>> Coffee::Rails::TemplateHandler.call(template, source)
This is because we only use hash to maintain the result. So when the key
are the same, the result would be skipped. The solution is to maintain
an array for tracking every item's position to restructure the result.
Previously, when using `render file:`, it was possible to render files
not only at an absolute path or relative to the current directory, but
relative to ANY view paths. This was probably done for absolutely
maximum compatibility when addressing CVE-2016-0752, but I think is
unlikely to be used in practice.
Tihs commit removes the ability to `render file:` with a path relative
to a non-fallback view path.
Make FallbackResolver.new private
To ensure nobody is making FallbackResolvers other than "/" and "".
Make reject_files_external_... no-op for fallbacks
Because there are only two values used for path: "" and "/", and
File.join("", "") == File.join("/", "") == "/", this method was only
testing that the absolute paths started at "/" (which of course all do).
This commit doesn't change any behaviour, but it makes it explicit that
the FallbackFileSystemResolver works this way.
Remove outside_app_allowed argument
Deprecate find_all_anywhere
This is now equivalent to find_all
Remove outside_app argument
Deprecate find_file for find
Both LookupContext#find_file and PathSet#find_file are now equivalent to
their respective #find methods.
This has similar problems to render file:.
I've never seen this used, and believe it's a relic from when all
templates could be rendered from an absolute path.
The previous behaviour of render file: was essentially the same as
render template:, except that templates can be specified as an absolute
path on the filesystem.
This makes sense for historic reasons, but now render file: is almost
exclusively used to render raw files (not .erb) like public/404.html. In
addition to complicating the code in template/resolver.rb, I think the
current behaviour is surprising to developers.
This commit deprecates the existing "lookup a template from anywhere"
behaviour and replaces it with "render this file exactly as it is on
disk". Handlers will no longer be used (it will render the same as if
the :raw handler was used), but formats (.html, .xml, etc) will still be
detected (and will default to :plain).
The existing render file: behaviour was the path through which Rails
apps were vulnerable in the recent CVE-2019-5418. Although the
vulnerability has been patched in a fully backwards-compatible way, I
think it's a strong hint that we should drop the existing
previously-vulnerable behaviour if it isn't a benefit to developers.
Custom glob patterns tie the implementation (Using Dir.glob) to the API
we provide.
It also doesn't really work. extract_handler_and_format_and_variant
expects the handler, format, and variant to be at the end of the
template path, and in the same order as they are in the default pattern.
This deprecates specifying a custom path for FileSystemResolver and
removes the pattern argument of OptimizedFileSystemResolver#initialize,
which does not work with a custom pattern.
Many tests were using `render file:`, but were only testing the
behaviour of `render template:` (file: just allows more paths/ is less
secure then template:).
The reason for so many `render file:` is probably that they were the old
default.
This commit replaces `render file:` with `render template:` anywhere the
test wasn't specifically interested in using `render file:`.
This is a developer quality of life improvement, to ensure that unknown
formats aren't assigned (which it would previously accept, but wouldn't
work 100% correctly due to caching).
This reverts commit 48e44edfd0.
See discussion in #32287
For HTML content in `ajax:success` handlers, `event.detail[0]` should
be an `HTMLDocument` instance.
There was a recent change by @tenderlove to Action view which introduced
`ActionView::Base#run` [1].
We ran into an issue with our application because one of the core
concepts in our domain model is a `Run` which is exposed in most of our
views as a helper method, which now conflicts with this new method.
Although this is a public method it is not really meant to be part of
the public API.
In order to discourage public use of this method and to reduce the
chances of this method conflicting with helper methods we can prefix
this method with an underscore, renaming this method to `_run`.
[1] https://github.com/rails/rails/commit/c740ebdaf5
All actionview caches are already cleared at the start of each request
(when Resolver.caching is false) by PerExecutionDigestCacheExpiry, which
calls LookupContext::DetailsKey.clear (which clears all caches).
Because caches are always cleared per-request in dev, we shouldn't need
this extra logic to compare mtimes and conditionally reload templates.
This should make templates slightly faster in development (particularly
multiple renders of the same template)
This allows mime types in the form text/html, text/*, or */*
This required a few minor test/code changes where previously nil was
used as a mime string.
Sample example ->
Before:
prathamesh@Prathameshs-MacBook-Pro-2 blog *$ rails server thin
DEPRECATION WARNING: Passing the Rack server name as a regular argument is deprecated
and will be removed in the next Rails version. Please, use the -u
option instead.
After:
prathamesh@Prathameshs-MacBook-Pro-2 squish_app *$ rails server thin
DEPRECATION WARNING: Passing the Rack server name as a regular argument is deprecated and will be removed in the next Rails version. Please, use the -u option instead.
Turbolinks replaces the CSP nonce <meta> tag on page change, but inline scripts inserted by UJS need the nonce from the initial page load. In general, it doesn't matter to UJS if the nonce changes after the page loads: only the initial value is relevant.
A string S matches ([.]|\b)html if an only if matches \bhtml:
* If S matches [.]html, then it matches \bhtml.
* If S matches \bhtml, then it matches \bhtml.
Reciprocally:
* If S matches \bhtml, then it matches ([.]|\b)html.
The character class can be removed, and since we are on it we remove the
group too so that it is clear to a reader of the code that there is no
grouping going on.
References #35166.
`virtual_path` is calculated in the constructor when the Template object
is allocated. We don't actually need to set it in the `decorate`
method. That means we can remove the decorate method all together.
This commit ensures that locals are passed in to the template objects
when they are constructed, then removes the `locals=` mutator on the
template object. This means we don't need to mutate Template objects
with locals in the `decorate` method.
* Don't call inspect from identifier_method_name
* Add locals Template#inspect
Handler, formats, and variant are usually obvious from looking at the
identifier. However it's not uncommon to have different locals for the
same template so we should make that obvious in inspect.
* Add tests for short_identifier and inspect
[John Hawthorn + Rafael Mendonça França]
Templates only have one variant, so we should not store it in an array.
This commit converts `variants` to `variant` and deprecates the plural
accessor
Even if the template is constructed with a `nil` variant, the array it
constructs will never be `empty?`:
56b030605b/actionview/lib/action_view/template.rb (L152)
We get an array that is `[nil]`, which is not empty, so this conditional
is never true.
Templates only have one format. Before this commit, templates would be
constructed with a single element array that contained the format. This
commit eliminates the single element array and just implements a
`format` method. This saves one array allocation per template.
This commit adds a finalizer just to inline templates. We can't cache
compilation of inline templates because it's possible that people could
have render calls that look like this:
```ruby
loop do
render inline: "#{rand}"
end
```
and we would cache every one of these different inline templates. That
would cause a memory leak. OTOH, we don't need finalizers on regular
templates because we can cache, control, and detect changes to the
template source.
Fixes: #35372
I want to start reducing the calls to `lookup_context`. That method
caches the lookup context in an ivar, but I would like to cache the
lookup context on the stack. That way we aren't coupled to the behavior
of the `lookup_context` method.
Currently, `ActionView::Base.new` will raise a `NotImplementedError` when given an instance of `ActionView::PathSet` on initialization. This commit prevents the raised error in favor of a deprecation warning.
This commit introduces "rendered template" and "rendered collection"
objects. The template renderers can now return a more complex object
than just strings. This allows the framework to get more information
about the templates that were rendered. In this commit we use the
rendered template object to set the "rendered_format" on the lookup
context in the controller rather than all the way in the template renderer.
That means we don't need to check the "rendered_format" every time we
render a template, we just do it once after all templates have been
rendered.
This commit is to remove direct access to the "rendered_format"
attribute on the lookup context. "rendered_format" is an implementation
detail that we shouldn't test directly.
Since #35036, the subclasses of `ActionView::Base` requires
the `compiled_method_container`.
This is incompatible. For example, `web-console` use view class that
subclass of `ActionView::Base`, and does not work it now cause of this.
Actually, since it seems that it is only `ActionView::Base` that
`compiled_method_container` is necessary, modified the condition that
emits a warning.
This commit passes the template format to the digestor in order to come
up with a key. Before this commit, the digestor would depend on the
side effect of the template renderer setting the rendered_format on the
lookup context. I would like to remove that mutation, so I've changed
this to pass the template format in to the digestor.
I've introduced a new instance variable that will be alive during a
template render. When the template is being rendered, it pushes the
current template on to a stack, setting `@current_template` to the
template currently being rendered. When the cache helper asks the
digestor for a key, it uses the format of the template currently on the
stack.
Akira Matsuda