Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow ApplicationController and helpers to reload in development environment #3927

Closed
wants to merge 1 commit into from

Conversation

jvaill
Copy link

@jvaill jvaill commented May 3, 2015

This fixes #697.

The problem is that PageController and its ancestors (up until ::InheritedResources::Base) don't reload by default and thus don't end up inheriting from future changes made to ApplicationController. It turns out that autoloading a class with ActiveSupport::Autoload does nothing to hook into Rails' reloading mechanism.

This PR moves the controllers to app/controllers/ which allows Rails to take over its autoloading and its reloading.

A good way to test this is with @timoschilling's minimal test case:
https://github.com/activeadmin/activeadmin_helper_reload_bug

🎉

@seanlinsley
Copy link
Contributor

It seems like there should be a way to allow these files to be reloaded without moving them out of the lib directory.

By the way, last year there was a PR that did something similar, that didn't end up being merged: #2906.

@jvaill
Copy link
Author

jvaill commented May 4, 2015

Just quickly brainstorming, off the top of my head I can think of a few other feasible solutions:

  1. Add the entire lib/ directory to ActiveSupport::Dependencies.autoload_paths and let Rails handle autoloading and reloading. This has the disadvantage of unloading all files inside of lib/ on each request.
  2. Move the controllers to a lib/controllers/ directory and add that to ActiveSupport::Dependencies.autoload_paths.
  3. See if we can use ActiveSupport::Dependencies.explicitly_unloadable_constants to get Rails to explicitly unload the controllers after each request.
  4. ? 😄

If we'd rather not shuffle files around, then looking into the third option might be worth it.

@timoschilling
Copy link
Member

I agree with @seanlinsley, so I'm closing this, but I will copy your ideas over to #697.

@javierjulio
Copy link
Member

@jvaill thanks. Your changes have been reincorporated in #8180 for now. I've confirmed they fix the helper bug reproductions locally. Just encountering another issue I'd need help with. We'll see if we can finally resolve this. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ApplicationController and Helpers are not being reloaded in Dev Env
4 participants