-
Notifications
You must be signed in to change notification settings - Fork 41
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
Updated Railtie to be Zeitwerk compatible when Rails version >= 6 #53
Conversation
Sorry for the delay with reviewing this! I'll have a look as soon as possible, somehow missed a notification about this 😓 |
@dmorehouse can you share what error you see in Rails 6 without your patch? I just tested the Rubygems latest version with Rails 6.1 and I see no errors during initialization. Maybe something changed in 6.1? |
@pokonski I suspect the message was a deprecation warning in 6.0 and they might have removed the warning (but not the problematic behavior) in 6.1. To be sure let me work up a Rails 6.0 project and a 6.1 project and I'll report back. |
After digging in, I've found that the deprecation warning happens in a new Rails 6.0 project but not a Rails 6.1 project because Rails 6.1 appears to have made subtle changes to the autoloader (I debugged autoloading and processing the access_granted/railtie.rb was in the same call stack and called in the same initializer order in both 6.0 and 6.1). This issue happens in a brand new Rails 6.0 project created using
Per the Rails 6.0 upgrade guide about autoloading - https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoloading-when-the-application-boots. Initializers can not autoload constants (class names) the solution is to use Rails.application.reloader.to_prepare however in your case you want to wait until ActionController::Base and ActionController::API are loaded. When I upgraded my project from Rails 5.2 -> 6.0, I noticed I had to upgrade a bunch of gems for Rails 6 / zeitwerk compatibility. Here's a couple gems that I needed to update and how they handled the Rails 6 railtie lazy initialization which I think are comparable to access_granted's railtie. FYI - The on_load hooks I use have been around for 10+ years, however to make this a no impact fix for < Rails 6.0 I chose to only use the hooks for Rails >= 6 so that if an existing project using access_granted counted on a side effect of loading ActionController::Base or ActionController::API during initialization the behavior would be the same. Let me know if there's any other details I can provide to help you assess this change. |
Awesome, thanks for the detailed explanation :)
Good call, but I am also okay with making this a breaking change and bumping version number. I'll have a look :) |
I merged this and will prepare a Rubygems release shortly, once again thanks for the help! |
Great thanks so much for working with me on this. I look forward to updating my Gemfile to use the latest version :-) |
@dmorehouse this is now live as 1.3.3, thanks for the reminder! |
This fixes Zeitwerk detecting constants loaded during initialization.