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

Defer including PdfHelper into ActionController::Base until it's loaded #790

Closed

Conversation

cgunther
Copy link

@cgunther cgunther commented Jan 9, 2019

ActionController::Base is lazily loaded by Rails and we don't want to prematurely load it, otherwise it may be loaded before initialization runs, then any configuration set in initialization is not copied over to ActionController::Base.

For example, the new Rails 5.2 config for Rails.application.config.action_controller.default_protect_from_forgery is assigned in an initializer, but ActionController::Base was loaded too early by the railtie, prior to running the initializers, so the value set was never set on ActionController::Base itself, thus never triggering the default protection.

Rails 3 is the first version where load hooks were introduced, and Rails 5.2 is the first version where an explicit hook for ActionController::Base was introduced.

ActionController::Base is lazily loaded by Rails and we don't want to prematurely load it, otherwise it may be loaded before initialization runs, then any configuration set in initialization is not copied over to ActionController::Base.

For example, the new Rails 5.2 config for `Rails.application.config.action_controller.default_protect_from_forgery` is assigned in an initializer, but ActionController::Base was loaded too early by the railtie, prior to running the initializers, so the value set was never set on ActionController::Base itself, thus never triggering the default protection.

Rails 3 is the first version where load hooks were introduced, and Rails 5.2 is the first version where an explicit hook for ActionController::Base was introduced.
@cgunther
Copy link
Author

cgunther commented Jan 9, 2019

The test runs that failed are all ruby 2.3 or later and failed because bundler v2 was installed, but the gemfile calls for bundler v1. I haven't had a chance to read up on the compatibility between bundler v1 and v2 yet to say what the right resolution here is.

For comparison, sidekiq recently changed their Travis config to force bundler v1 across the board:
https://github.com/mperham/sidekiq/blob/c2e819769a9d765b93c66f8ae0e2737885303ae1/.travis.yml#L7

I'll be running this patch in production shortly on a Rails v5.2.2, Ruby v2.5.3 app, so I'll report back if there's any compatibility issues.

@cgunther
Copy link
Author

Fixed by #869.

@cgunther cgunther closed this Jun 19, 2020
@cgunther cgunther deleted the lazy-load-include-pdf-helper branch June 19, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants