wrap initialization in reloader.to_prepare to avoid autoload deprecation warning in some cases

[jrochkind]

My Rails 6.1.3.2 app that uses qa gem has this deprecation warning on startup in development mode:

DEPRECATION WARNING: Initialization autoloaded the constants Qa::LinkedData and Qa::LinkedData::AuthorityService.


Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload Qa::LinkedData, for example,
the expected changes won't be reflected in that stale Module object.

These autoloaded constants have been unloaded.

In order to autoload safely at boot time, please wrap your code in a reloader
callback this way:

    Rails.application.reloader.to_prepare do
      # Autoload classes and modules needed at boot time here.
    end

That block runs when the application boots, and every time there is a reload.
For historical reasons, it may run twice, so it has to be idempotent.

Check the "Autoloading and Reloading Constants" guide to learn more about how
Rails autoloads and reloads.
 (called from <main> at /Users/jrochkind/code/scihist_digicoll/config/environment.rb:5

/Users/jrochkind/code/scihist_digicoll/config/environment.rb:5 is just Rails.application.initialize!.

I haven't been able to reproduce this in a new fresh Rails 6.1.3.2 app -- I'm not sure what settings in my actual app are triggering this warning, when I can't get a fresh app too. I've spent some time trying.

However, it seems like this is a legit problem, that could affect other apps, and should be fixed. Here is the Rails Autoloading and Reloading Constants Guide.

The small change in this PR eliminates the deprecation warning for me. And I believe is appropriate for Rails.

Other possible changes could have been:

• removing these initializers, doing the loading currently in these initializers on-demand lazily instead, at some point after initialization
• moving the classes referenced in the initializers somewhere will they will not be auto-loaded -- This gem has a lot of manual extend ActiveSupport::Autoload in various places (they've been there for a long time), to opt-in to Rails autoloading for classes that would ordinarily not be auto-loaded. (I think engine classes are not autoloaded by default at all). i think that's the root of the problem, and I'm not sure if it's really a good idea or necessary -- but seemed like a bigger change than I wanted to make for now.

Those might actually make sense, but I was nervous to try to make more than the smallest change that seemed to resolve the problem, which this is.

I have also confirmed:

• the code called in these initializers looks idempotent to me, as is required for anything in a to_prepare block
• The Rails.application.reloader API was introduced in Rails 5.0.0, which is the oldest version of Rails that this gem currently supports, so it is available in all versions of this gem.

The only thing that makes me nervous is I don't understand why I can't reproduce in stock from scratch app. Still, I think this change should be harmless and appropriate and will fix this problem -- unless someone would prefer we go removing all the extend ActiveSupport::Autoload sprinkled throughout the gem, which may actually be a more fundamental fix.

[maxkadel]

Hrm, looks very reasonable to me, but I'm not sure I understand rails loading well enough to approve it with confidence.

[escowles]

This looks reasonable to me, and since CI passes with Rails 5.1/5.2/6.0, I'm comfortable merging this.

[cjcolvar]

I'm running into the following error when trying to build Hyrax's docker images.

NoMethodError: undefined method `[]' for nil:NilClass
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:20:in `subauthorities_path'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:29:in `names'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:61:in `register_defaults'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:41:in `block in registry'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local/registry.rb:6:in `initialize'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:40:in `new'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:40:in `registry'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:51:in `register_subauthority'
/app/samvera/hyrax-webapp/config/initializers/hyrax.rb:55:in `<main>'

This is happening when bundle exec rake assets:precompile is run.

I played with different forms and the only I was seeing as working were Qa::Engine.config.before_initialize and the original unwrapped form. This might have to do with the rake command being run in a production rails environment where cache_classes is set to true.

[jrochkind]

Sorry I'm off on Fridays so just going through all my messages and alerts and getting to this.

While we've already reverted the change thinking we didn't have the bandwidth...

For future reference, I think the answer would/could have been that the thing in your local hyrax-webapp/config/initializers/hyrax.rb:55 also needed to be wrapped in to_prepare.

• should work fine Rails 5.2, and with or without zeitwerk
• without to_prepare in an initialier when referencing autoloaded content was always a race condition that could fail unpredictably, but only with zeitwerk autoloader is it flagged and given a deprecation notice, with future zeitwerk to make it fail altogether. So this will have to be fixed as people upgrade to a zeitwerk that requires it.