Add some information about the eager load paths in the upgrade docs

[jvanbaarsen]

It is not really clear that config.autoload_paths has been removed. Make it
more explicit by adding it to the upgrade guide.

Related: #24012

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @senny (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[senny]

@jvanbaarsen what code change are you referring to? The linked Issue points to 212ef52 which was part of Rails 4.0. Did something change from 4.2 to 5.0 ?

[jvanbaarsen]

@senny I feel something has changed, since my app worked on 4.2, but I upgraded it to 5.0.beta3 and the autoloading has failed for me (It's in the autoload path, but I suddenly need to require the lib file). By changing it to eager_auto_load_paths it worked again

[pixeltrix]

@jvanbaarsen something has changed - we no longer enable autoloading in production by default.

a71350c

[jvanbaarsen]

@pixeltrix Ok, so is this the correct way of enabling it again (That I use now), since that made the app work again. Or am I missing something?

[pixeltrix]

@jvanbaarsen if you want to load all of /lib at application boot time then yes, that's the way to fix it. However you may have stuff in there you don't want to load in which case use ActiveSupport::Dependencies.hook! to re-enable autoloading in production.

[jvanbaarsen]

Ok, I see.

This leaves open the situation that potentially a lot of apps will break when they upgrade to rails5, how do you think we should communicate this breaking change?

[pixeltrix]

We can obviously add something to the upgrading guide that explains the change, the question is whether it needs something more like leaving it enabled by default and unhooking it in an intializer for new apps. Then we can flip the default in Rails 5.1 and remove the initializer.

[jvanbaarsen]

@pixeltrix So if we add an initializer with the content: ActiveSupport::Dependencies.hook! it will work as before? If that is the case I can create a new PR adding that initializer?

[pixeltrix]

@jvanbaarsen no, please don't create that PR - if we were to do one it'd be the other way around. However we need to have a discussion first.

[jvanbaarsen]

👍

[vipulnsward]

Thanks for Pull Request. Going to give this a close since a more detailed version is now up at
http://guides.rubyonrails.org/upgrading_ruby_on_rails.html#autoloading-is-disabled-after-booting-in-the-production-environment

[fxn]

The undelying problem here is a mismatch that has been out there for a long time due to historical reasons.

The mismatch is that code in custom autoloaded directories is not eager loaded. As a user I'd expect it to be eager loaded because autoloading is not thread-safe and that is one of the reasons Rails eager loads while booting. So, I would expect Rails to automatically eager load what has been explicitly marked, plus any remaining stuff in config.autoload_paths.

Rails 5 disables autoloading after boot, and this now becomes an actual problem. This code no longer works in production (untested, written directly here):

# config/application.rb
config.autoload_paths += "#{Rails.root}/lib"

# app/models/user.rb
class User < ActiveRecord::Base
  def boom
    p X
  end
end

# lib/x.rb
X = 1

# executed later at runtime somewhere
User.new.boom # NameError

and it did before, except it was doing it in a non-thread-safe way (so the previous situation also had a problem).

The historical reason goes back to the Rails 3 days IIRC. The lib directory was often added to the autoload paths of applications because it was there in the early days of Rails, so it had an inertia via upgrades and customs. People had all kind of stuff in lib and eager loading the entire directory would have been a bad idea in many cases. So, it was decided to not do it.

Nowadays we need to address this some way. For example, some options are:

1. Rails could add autoload paths to eager load paths for coherence. Document it and ask the user to consider the consequences. Mention it in the upgrade guide.
2. Rails could check if autoload paths are a subset of eager load paths and err if they are not on booting, on any environment. Explain why, suggest to add the directory to the eager loaded paths and ask the user to consider the consequences. This is the defensive version of 1.

At some point, I think we need to move forward here and go with 1, because the reaction to "if you add to autoload paths then you need to add to eager load paths" is, "if so, please do it automatically for me!"

/cc @tenderlove @matthewd

[jvanbaarsen]

@fxn Thanks for your extended reply! I would really love for the first option. If there is anything I can do to help with this (Contribute docs or code), please let me know 👍

[rafaelfranca]

Rails could add autoload paths to eager load paths for coherence. Document it and ask the user to consider the consequences. Mention it in the upgrade guide.

This option is not possible with lib, in lib can live code that should not be eager loaded like generators, development specific patches, etc.

Rails could check if autoload paths are a subset of eager load paths and err if they are not on booting, on any environment. Explain why, suggest to add the directory to the eager loaded paths and ask the user to consider the consequences. This is the defensive version of 1.

This one is the best direction. Like I said above you can't add the whole lib in eager_load_paths and you will need to add subfolders of lib, so we should err and tell users what are the alternatives.

[fxn]

@rafaelfranca not totally sure. We are not talking about what to do with lib, but what to do with autoload paths and their relation with eager load paths.

If an application adds lib/utils to autoload paths and we err saying "hey, you added something to autoload paths, this should also be in eager load paths". The reaction of the user would be: "if everything in autoload paths should be in eager load paths, please do so for me! Why do I need to remember to modify two settings?"

[fxn]

I have the feeling that we have two conflicting issues going on here:

1. It makes no sense to have stuff in autoload paths that it is not present in eager load paths. For thread-safety in the first place, and now because it just does not work as shown above.
2. Therefore, Rails should be expected to consider that stuff in autoload paths has to be eager loaded.
3. We have a historical issue specifically with the lib directory.

I wonder if we could get 2 and have hard-coded treatment for 3?

[rafaelfranca]

By my non-data based research 80% of times people changes autoload_paths is to add the whole lib to it. All the applications I worked did this. It was the default in 3.x era, and this search show a lot of examples https://github.com/search?l=ruby&p=3&q=autoload_paths&type=Code&utf8=%E2%9C%93.

Yeah, my main concern is about the case 3, it is 80% of the cases. 1 and 2 makes sense, we tried it in Rails 4.0 but we reverted exactly because of 3. To make 1 and 2 possible we should kill the bad practice that is 3.

[jayjlawrence]

I'd like to add a comment about a nuanced behavioiur of the autoloading mechanism in hopes it might find its way into the documentation as well.

In my lib directory I have a class that provides configuration data for my app. Under this class are lots of subclasses for app features. So I'd like to have the configuration class available as the application.rb file evaluates.

What seems to happen is that adding lib to 'config.autoload_paths' is that it is only processed after the application.rb is completed. To make the autoloaded path available earlier I am current using a direct call to ActiveSupport::Dependencies.autoload_paths and this seems to resolve the issue.

So the following works:

module Amp2
  class Application < Rails::Application
    # Add lib to the autoload path
    ActiveSupport::Dependencies.autoload_paths << "#{Rails.root}/lib"

    # Application configuration class
    config.time_zone = MyAppLibClass.config('locale.time_zone')
    config.i18n.available_locales = MyAppLibClass.config('locale.locales')

the file "./lib/my_app_lib_class.rb" is autoloaded and available for the 'config.time_zone' assignment.

This does not work:

module Amp2
  class Application < Rails::Application
    # Add lib to the autoload path
    config.autoload_paths << "#{Rails.root}/lib"

    # Application configuration class
    config.time_zone = MyAppLibClass.config('locale.time_zone')
    config.i18n.available_locales = MyAppLibClass.config('locale.locales')

However after 'application.rb' is loaded then I can refer to MyAppLibClass as expected - like in the production and development.rb files. This really just helps me keep my app/environment code dry.

And if there is a better way to do this it would be a good thing to put in the guide.

Hope this helps
J

[pixeltrix]

@jayjlawrence we already go into great detail in The Rails Initialization Process - if you think there's something missing from there please submit a PR.

As for your issue, the problem is that Rails boots in two phases - the configuration phase and then the initialization phase and you're trying to utilize that config change whilst still in the config phase. There are initialization event callbacks that you can hook into that can implement what you need, e.g:

module Amp2
  class Application < Rails::Application
    config.before_initialize do
      config.time_zone = MyAppLibClass.config('locale.time_zone')
    end
  end
end

I can't be sure that every config option would be setup properly in before_initialize though - check each framework component's railtie class for the gory details.

[jayjlawrence]

Hey @pixeltrix thanks for the excellent reference. In fact I was not aware of this document. Is it possible to have http://guides.rubyonrails.org/initialization.html added to the 'Guides Index'. I see it listed on the main index page further down ... I never thought to scan through the main index page of Rails Guides because I aways just went to the drop down for Guides Index .... I see that it is listed as work in progress - that caveat can be repeated in the guide itself to cover off this warning. HTH. Jay