fxn / zeitwerk

Efficient and thread-safe code loader for Ruby
MIT License
1.99k stars 118 forks source link

`eager_load` clobers previous `eager_load_dir` and `load_file` calls? #298

Closed mscrivo closed 3 months ago

mscrivo commented 3 months ago

Hi,

Just started working with zeitwerk and it's fantastic! We have a large non-rails codebase that I've been trying to get it working with and after much heartache, I was finally able to get it all working. However, one small issue, or probably misunderstanding on my part is how the eager loading works. I had to add a bunch of loader.eager_load_dir for various things that used a lot of meta programming, ie. using .descendants, dynamically generating constants, etc. just to get the app working. While that all works fine, I then wanted to eager load the rest of the code for our web apps specifically in production, so we didn't get any slow lazy loading surprises during actual requests.

So I'm doing something like:

app.rb:

# setup loader and other common stuff
$loader.eager_load_dir("one")
$loader.eager_load_dir("two")

Then in each web app, say web_app.ru for example, I wanted to do:

$loader.eager_load

to eager load the "rest of it", at least that's how I intuitively thought of it. What seems to be happening instead is that eager_load call is invalidating all those prior specific eager_load_dir calls.

First, I assume this is by design? And if so, is there some other canonical way to do this kind of thing?

fxn commented 3 months ago

Hi! invalidating in what sense?

mscrivo commented 3 months ago

Hi! invalidating in what sense?

In the sense that it was if they were never called at all. Things that were supposed to be eager loaded in those specific eager_load_dir were in fact missing and thus things relying on them started causing errors at runtime until I removed that last eager_load. Once I removed that, it worked as expected. I will try seeing if I move the eager_load if prod_build to the top and then do the specific eager loads after it, to see if that works.

fxn commented 3 months ago

This is strange.

First, let me say that your expectations about the API were correct, you can selectively eager load directories first, and then eager load globally.

This script demonstrates this feature:

require 'tmpdir'
require 'fileutils'
require 'zeitwerk'

Dir.mktmpdir do |dir|
  Dir.chdir(dir) do
    FileUtils.mkdir_p('lib/foo')
    FileUtils.mkdir_p('lib/bar')

    File.write('lib/foo/x.rb', 'Foo::X = 1')
    File.write('lib/bar/x.rb', 'Bar::X = 1')

    loader = Zeitwerk::Loader.new
    loader.push_dir('lib')
    loader.log!
    loader.setup

    loader.eager_load_dir('lib/foo')
    loader.eager_load

    p Foo::X
    p Bar::X
  end
end

In that script, I added loader.log! so that you can see how does a normal activity look like. One thing we could do would be to enable logging in your app and reproduce, to see if anything interesting shows up.

It would be ideal to have a minimal way to reproduce.

mscrivo commented 3 months ago

Thanks for that clarification. I'll play around with it today and see if I can figure out what's going on.

mscrivo commented 3 months ago

This is user error. It seems something in our codebase, when eager loading all is changing the behaviour of one particularly sensitive piece of code that made it look like the other eager loads weren't working. Sorry for the false alarm and super appreciate the quick responses!

fxn commented 3 months ago

@mscrivo awesome, glad you finded out!