Defer modification of ActionController::Base to when it is loaded

[cjlarose]

The railtie for this gem adds an initializer that loads the class ActionController::Base in order to add a before_action to it.

The initializer references the ActionController::Base constant directly, which, if it isn't already loaded, will be loaded at that moment. loading that class causes other initializers run, including, for example, the initializer defined by actiontext's engine, as well as various initializers defined by third-party gems in a given application.

The effect is that adding the config gem to a Rails application that didn't have it causes that application to load ActionController::Base (and possibly a bunch of related stuff) during initialization, even in contexts where usage of ActionController::Base isn't expected, like when launching a console through rails console.

The solution here is to use ActiveSupport::LazyLoadHooks to add a hook that runs only after the ActionController::Base has been loaded.

[rdubya]

Seems like a good idea to me, but can you look into the failing travis build and see if it is related to your change? It doesn't seem like it would be, but I don't see those failures in the last couple of commits to master.

[pkuczynski]

I agree, and it seems like a very good change! We can merge it and release it as soon as the build is green and CHANGELOG entry is added. @cjlarose can u have a look at that?

[cjlarose]

I'll take a look and add the changelog entry. Thanks!

[cjlarose]

The test failures ended up being unrelated to my change, but I fixed it anyway.

The most recent build on master happened to install sprockets version 3.7.2 when installing the sample Rails 4.2 application. But when my PR ran a month later, sprockets had published a new major version (4.0.0). For whatever reason, the sample Rails 4.2 application in this project is incompatible with sprockets 4.0.0 (not surprising, considering it's a major version release). All I did was modify the gemfile for the Rails 4.2 application so that it specifies that we want sprockets 3.7.x.

[rdubya]

Awesome, thanks for finding/fixing the sprockets issue.

[jrafanie]

@cjlarose I didn't see your fix for tests with rails 4.2 with sprockets 4 so I extracted a single PR to do it, #254

[pkuczynski]

Can you dbl check which version is correct: yours or https://github.com/railsconfig/config/pull/258/files#diff-e1b40370ec751eed3246e103922adc0eR26 ?

[pkuczynski]

@amatsuda @Fryguy which version is better? This one or the one from #258?

[Fryguy]

I think this version is better. I don't think the class_eval is needed because on_load takes care of it being in the right context.

[pkuczynski]

Yeah, that's what I thought too! Then I will merge this one!

[pkuczynski]

Thank you @cjlarose!