Compatibility with Zeitwerk

[mhw]

I just tried adding this to a new Rails 6 project, and it doesn't seem to be raising an error for a migration that should be getting flagged.

My first guess is that the Zeitwerk loader doesn't use the code path that good_migrations hooks in to (ActiveSupport::Dependencies.load_file). I've added puts statements in the right places and they're not getting hit, while adding config.autoloader = :classic to application.rb gets the good_migrations error I'd expect.

I've not dug further to see if there's an equivalent place in Zeitwerk to hook in to, but thought I'd raise the issue for others that may run in to it. (Or for others to say it works for them and I must be doing something wrong...)

[searls]

Thanks for the heads up -- any additional insight you're able to dig up would be hugely appreciated!

[searls]

Hey @fxn, could you take a quick look at this source and advise what we might do to support Zeitwerk? Thanks if you get a chance!

[fxn]

Hi!

The thing with AS::Dependencies is that most of its methods were implementation. This is tricky, because since it has been there since the beginning, the line was not well-defined. When the integration with Zeitwerk was done, you had to take a decision, and this conservative selection was made.

Zeitwerk does not load the files itself, it basically sets autoloads and delegates loading to Ruby. Therefore load_file in that mode is really Ruby's require. However, that is implementation too.

The public API is the one linked above. Could the gem be based on checking if autoloaded_constants changed, perhaps? (That collection is empty if reloading is disabled.)

[searls]

I don't have time to try to figure out a solution but I at least got to the point of getting a failing build assembled (requires setting a RAILS_VERSION env var to 6.x) here: #11

[bf4]

let's add docs that this don't work under zeitwerk

[searls]

@bf4 good point, thanks 14a7730

[fxn]

Hey!

In zeitwerk/main there is a catch-all on_load callback that I believe could be helpful. You'd need to configure the loaders before migrations run. Something like this:

Rails.autoloaders.each do |loader|
  loader.on_load do |cpath, value, abspath|
    # Check if abspath is under Rails.root.
  end
end

There, cpath is the constant path as a string (like "UsersController"), value would be the value stored in the constant, which is normally a class or module object (but not necessarily), and abspath is the absolute path to the corresponding file or directory.

That will ship with the forthcoming 2.5.0.

[searls]

That's awesome, @fxn! Thank you so much for the heads-up!

I don't have a great sense for what percentage of Rails users are using which loader. Is there a certain version of rails on-or-after which it will for-sure be using zeitwerk?

[fxn]

Rails 7 won't include classic and the vast majority of code in dependencies.rb will be deleted. I am already gradually deleting the classic autoloader in rails/main. Rails 7 is going to depend on Zeitwerk 2.5.0 or more, so that callback will be available.

The predicate Rails.autoloaders.zeitwerk_enabled? was introduced in Rails 6.0 and could also be useful, perhaps. That predicate will also exist in Rails 7, and will return true unconditionally.

[mhw]

@fxn, that's perfect!

I went back to the change I suggested that hooked in to a private API. It was simple to update it to use the on_load callback. Updated branch here.

[searls]

Ok, this was just released in 0.1.0

@fxn, do you have an estimate for when zeitwerk 2.5 will be released?

[fxn]

@searls Not really. I am in the process of removing the classic autoloader in rails/main and am holding a bit in case some new use case arises. Plan would be to require 2.5 in Rails 7, but won't wait till Rails 7 for the release, I'll publish it when the work is done.

[searls]

Thanks @fxn!