heartcombo / devise

Flexible authentication solution for Rails with Warden.
http://blog.plataformatec.com.br/tag/devise/
MIT License
23.95k stars 5.55k forks source link

Rails6 without ActionMailer won't boot with zeitwerk eager-loading #5140

Open sbull opened 5 years ago

sbull commented 5 years ago

Environment

Current behavior

In a Rails 6 app without ActionMailer included (rails new testapp --skip-action-mailer), the default zeitwerk loader errors when eager-loading due to mailers/devise/mailer.rb not defining Devise::Mailer. Output from rails zeitwerk:check: expected file /usr/local/bundle/gems/devise-4.7.1/app/mailers/devise/mailer.rb to define constant Devise::Mailer,

Expected behavior

Zeitwerk eager-loading should work without ActionMailer. Suggested options:

fxn commented 5 years ago

Idiomatically, you want to ignore that file. It could done with something like this in Devise (off the top of my head):

initializer "devise.configure_zeitwerk_if_enabled" do
  if Rails.autoloaders.zeitwerk_enabled? && !defined?(ActionMailer)
    Rails.autoloaders.main.ignore("#{__dir__}/relative/path/to/devise/mailer.rb")
  end
end
mracos commented 5 years ago

@fxn I would only add a check for Rails::Version::MAJOR >= "6" so it does not break on older versions of Rails where the method zeitwerk_enabled? does not exist, wdyt?

fxn commented 5 years ago

Oh yes, totally.

jeppester commented 4 years ago

In case others come here in search for a quick solution:

It's possibly to work around the issue by simply adding ActionMailer in config/application.rb along with the other frameworks:

require "action_mailer/railtie"
gkats commented 4 years ago

For what it's worth, in a Rails 6.0.1 app without the ActionMailer railtie, it seems that defining Devise::Mailer as an empty class does not break anything. The app sends emails via a 3rd party service.

# <path-to-gems>/devise-4.7.1/app/mailers/devise/mailer.rb

if defined?(ActionMailer)
  class DeviseMailer < Devise.parent_mailer.constantize
    # whatever was already there
  end
else
  class DeviseMailer
  end
end

Tested authentication, registration, validation, forgot password flows.

Obviously inferior solution to explicitly ignoring the file in an initializer, but thought I'd share.

NeimadTL commented 4 years ago

Hi everyone, I'm working on this and I was wondering how can I write a test to check whether the file has been ignored or not please as I don't have an in-depth Rails API knowledge ?

This is what I added so far (else statement) :

if defined?(ActionMailer)
  class Devise::Mailer < Devise.parent_mailer.constantize
    # whatever was already there
else
  if Rails::Version::MAJOR >= "6"
    Rails.autoloaders.main.ignore("#{__dir__}/relative/path/to/devise/mailer.rb")
  end
end

Thank you for your help.

rusterholz commented 3 years ago

Out of curiosity, why isn't the preferred solution here simply for Devise to start using "zeitwerk-correct" file structure and class naming?

carlosantoniodasilva commented 3 years ago

Fun story, I just hit this while building a small test app to validate a few other things I'm working on here. I'll get a patch ready for Devise. Thanks everyone!

@rusterholz would you mind clarifying what "using "zeitwerk-correct" file structure and class naming?" means? Devise is using the correct naming structure for that mailer for example, the issue is that it doesn't declare the constant name if ActionMailer isn't loaded (otherwise it'd cause a possible load error), resulting in a devise/mailer.rb file being declared empty, and Zeitwerk doesn't like it, it expects the file to have a matching constant name.

The fix is to either declare that constant name "empty", or to have Zeitwerk ignore that file like mentioned above. (alternatively, loading ActionMailer works too because it makes Devise define the "missing" constant.)

fxn commented 3 years ago

The main autoloader is setup late in the boot process. An initializer could do something like this (untested)

if Rails.autoloaders.zeitwerk_enabled?
  Rails.autoloaders.main.ignore("#{root}/app/mailers")
end

While classic is gone in Rails 7, zeitwerk_enabled? still exists for engines.

rafaelfranca commented 1 year ago

Would devise even work without Action Mailer? So many of its default modules requires a mailer to exist.

simonc commented 7 months ago

@rafaelfranca If you provide a class that provides the necessary interface and responds with objects that respond to the deliver_* methods, it should work properly. For instance I do so with a class that will then call the Sendgrid API to use templates hosted on it.

I just faced the present issue when removing the call to require "action_mailer/railtie" in application.rb.