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

Correctly hook on Action Controller #338

Merged
merged 1 commit into from
Jan 23, 2017

Conversation

rafaelfranca
Copy link
Contributor

When an initializer load the ActionController::Base constant all the on_load triggers for :action_controller are executed. This cause a lot of load order issues.

Gems should always use ActiveSupport.on_load to extent behavior of the Rails frameworks.

Also using ActiveSupport.on_load(:action_controller) we can make sure that both ActionController::Base and ActionController::Api are extended.

This is related with #337 and #314.

When an initializer load the `ActionController::Base` constant all the
on_load triggers for `:action_controller` are executed. This cause a lot of
load order issues.

Gems should always use `ActiveSupport.on_load` to extent behavior of the
Rails frameworks.

Also using `ActiveSupport.on_load(:action_controller)` we can make sure
that both `ActionController::Base` and `ActionController::Api` are extended.
@emaiax
Copy link

emaiax commented Dec 7, 2016

I hope this can be merged soon. :shipit:

@cupakromer
Copy link

ping 💙

Hoping this gets merged soon. Just ran into a strange issue with Rails 5 ActionController::API and render :action (I am using Jbuilder for the renderer). All requests return the string body of " ". Applying this patch resolves the issue (as does removal of the bugsnag gem).

This can easily be reproduced with a new Rails 5.0.1 app, Jbuilder 2.6.1, and bugsnag 5.0.1. Here is the complete controller and view I used:

# app/controllers/widgets_controller.rb
class WidgetsController < ActionController::API
  def index
    render :index
  end
end
# app/views/widgets/index.json.jbuilder
json.data "API Testing"

@kattrali kattrali merged commit 514cd6a into bugsnag:master Jan 23, 2017
@kattrali
Copy link
Contributor

Thank you! This will be in a release shortly.

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.

4 participants