Skip to content
Browse files

Add preview_path to autoload_paths in after_initialize

Only config.autoload_paths is frozen, so add the preview_path
to ActiveSupport::Dependencies.autoload_paths directly in an
after_initialize block. Also protect against a blank preview_path
being added to autoload_paths which can cause a serious slowdown
as Dir[] tries to load all *_preview.rb files under /

Fixes #13372
  • Loading branch information...
1 parent 81066b1 commit 3713e433667ee95caccb53a4062f540405272234 @pixeltrix pixeltrix committed
View
8 actionmailer/lib/action_mailer/preview.rb
@@ -56,12 +56,18 @@ def preview_name
protected
def load_previews #:nodoc:
- Dir["#{preview_path}/**/*_preview.rb"].each{ |file| require_dependency file }
+ if preview_path?
+ Dir["#{preview_path}/**/*_preview.rb"].each{ |file| require_dependency file }
+ end
end
def preview_path #:nodoc:
Base.preview_path
end
+
+ def preview_path? #:nodoc:
+ Base.preview_path?
+ end
end
end
end
View
12 actionmailer/lib/action_mailer/railtie.rb
@@ -19,6 +19,10 @@ class Railtie < Rails::Railtie # :nodoc:
options.javascripts_dir ||= paths["public/javascripts"].first
options.stylesheets_dir ||= paths["public/stylesheets"].first
+ if Rails.env.development?
+ options.preview_path ||= defined?(Rails.root) ? "#{Rails.root}/test/mailers/previews" : nil
+ end
+
# make sure readers methods get compiled
options.asset_host ||= app.config.asset_host
options.relative_url_root ||= app.config.relative_url_root
@@ -41,11 +45,9 @@ class Railtie < Rails::Railtie # :nodoc:
end
end
- initializer "action_mailer.configure_mailer_previews", before: :set_autoload_paths do |app|
- if Rails.env.development?
- options = app.config.action_mailer
- options.preview_path ||= defined?(Rails.root) ? "#{Rails.root}/test/mailers/previews" : nil
- app.config.autoload_paths << options.preview_path
+ config.after_initialize do
+ if ActionMailer::Base.preview_path?
+ ActiveSupport::Dependencies.autoload_paths << ActionMailer::Base.preview_path
end
end
end
View
42 railties/test/application/mailer_previews_test.rb
@@ -225,6 +225,46 @@ def foo
assert_no_match '<li><a href="/rails/mailers/notifier/bar">bar</a></li>', last_response.body
end
+ test "mailer previews are reloaded from a custom preview_path" do
+ add_to_config "config.action_mailer.preview_path = '#{app_path}/lib/mailer_previews'"
+
+ app('development')
+
+ get "/rails/mailers"
+ assert_no_match '<h3><a href="/rails/mailers/notifier">Notifier</a></h3>', last_response.body
+
+ mailer 'notifier', <<-RUBY
+ class Notifier < ActionMailer::Base
+ default from: "from@example.com"
+
+ def foo
+ mail to: "to@example.org"
+ end
+ end
+ RUBY
+
+ text_template 'notifier/foo', <<-RUBY
+ Hello, World!
+ RUBY
+
+ app_file 'lib/mailer_previews/notifier_preview.rb', <<-RUBY
+ class NotifierPreview < ActionMailer::Preview
+ def foo
+ Notifier.foo
+ end
+ end
+ RUBY
+
+ get "/rails/mailers"
+ assert_match '<h3><a href="/rails/mailers/notifier">Notifier</a></h3>', last_response.body
+
+ remove_file 'lib/mailer_previews/notifier_preview.rb'
+ sleep(1)
+
+ get "/rails/mailers"
+ assert_no_match '<h3><a href="/rails/mailers/notifier">Notifier</a></h3>', last_response.body
+ end
+
test "mailer preview not found" do
app('development')
get "/rails/mailers/notifier"
@@ -366,7 +406,7 @@ def foo
private
def build_app
super
- app_file 'config/routes.rb', "Rails.application.routes.draw do; end"
+ app_file "config/routes.rb", "Rails.application.routes.draw do; end"
end
def mailer(name, contents)

4 comments on commit 3713e43

@vipulnsward

@pixeltrix not sure, but would not having preview_path in autoload_paths cause issues down the line, with dependency clearance/ reloads in future.

btw :heart: for fixing this.

@pixeltrix
Ruby on Rails member

@vipulnsward there are tests to make sure that they reload across requests and that methods added and deleted are picked up

@sadjow

The problem is still happening when upgrading from Rails 4.0.4 to 4.1. I'm investigating. :shipit:

cc @vipulnsward @pixeltrix

@sadjow

The problem only occurs in test environment with capybara. Investigating ...

Please sign in to comment.
Something went wrong with that request. Please try again.