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

Updated Railtie to be Zeitwerk compatible when Rails version >= 6 #53

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

dmorehouse
Copy link

This fixes Zeitwerk detecting constants loaded during initialization.

@pokonski
Copy link
Contributor

Sorry for the delay with reviewing this! I'll have a look as soon as possible, somehow missed a notification about this 😓

@pokonski
Copy link
Contributor

pokonski commented Jul 30, 2021

@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?

@dmorehouse
Copy link
Author

@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.

@dmorehouse
Copy link
Author

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 rails _6.0.4_ new project and produces the following message (which goes to development.log) after adding access_granted to the Gemfile.

DEPRECATION WARNING: Initialization autoloaded the constants ActionText::ContentHelper and ActionText::TagHelper.

Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload ActionText::ContentHelper, for example,
the expected changes won't be reflected in that stale Module object.

These autoloaded constants have been unloaded.

Please, check the "Autoloading and Reloading Constants" guide for solutions.
 (called from <main> at /redacted/ag_rails_6.0/config/environment.rb:5)

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.
mileszs/wicked_pdf@6fa6599#diff-872e0a26631d6103ce120b7e32313dbec795e73994f865312451a08023cb6a99
ErwinM/acts_as_tenant@fea563c#diff-c44e52750815152c39879f43563fd7a265f7f031602b0ea3f6e986403ae159ea

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.

@pokonski
Copy link
Contributor

pokonski commented Aug 2, 2021

Awesome, thanks for the detailed explanation :)

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.

Good call, but I am also okay with making this a breaking change and bumping version number. I'll have a look :)

@pokonski pokonski merged commit bdd02c7 into chaps-io:master Aug 2, 2021
@pokonski
Copy link
Contributor

pokonski commented Aug 2, 2021

I merged this and will prepare a Rubygems release shortly, once again thanks for the help!

@dmorehouse
Copy link
Author

Great thanks so much for working with me on this. I look forward to updating my Gemfile to use the latest version :-)

@pokonski
Copy link
Contributor

@dmorehouse this is now live as 1.3.3, thanks for the reminder!

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.

2 participants