fxn/zeitwerk

Let loaders eager load namespaces

Closed this issue · 14 comments

fxn commented

(Followup to #232.)

The idea would be

loader.eager_load_namespace(Some::Namespace)

@byroot could you describe a use case for it?

Sure.

# app/models/experiments.rb
module Experiments
  class Base
  end

  class << self
    def all
      Rails.autoloaders.main.eager_load_namespace(Some::Namespace)
      Base.descendants
    end
  end
end
# app/models/experiments/something.rb
module Experiments
  class Something < Base
  end
end
# app/views/experiments/index.html.erb
<ul>
  <% Experiments.all.each do |experiment| %>
    <li><%= experiment.name %></li>
  <% end %>
</ul>

Let me know if that requires further clarification.

Also I think it would make more sense to have Zeiwerk.eager_load_namespace(Experiments), as that namespace might not be handled by a single loader, but that's up to you.

fxn commented

Good idea. That makes sense, because a namespace is a global object (while directories are per loader). The curved ball comes from eager load exclusions, since they are file system-based. I'll give it a thought.

👍 no rush. I think that would help us cleanup a few things, but it's not a blocker or anything for us.

fxn commented

We could also say that eager load exclusions apply only to file system-based interfaces like loader.eager_load and loader.eager_load_dir. These indeed have some edge-cases that already show asymmetry in the opposite direction, for example see this test:

test "eager loads collapsed directories, ignoring the rest of the namespace" do
  files = [["x.rb", "X = 1"], ["collapsed/y.rb", "Y = 1"]]
  with_files(files) do
    loader.push_dir(".")
    loader.collapse("collapsed")
    loader.setup
    loader.eager_load_dir("collapsed")

    assert !eager_loaded?(files[0])
    assert eager_loaded?(files[1])
  end
end

Conceptually, that is a flat namespace, but you asked to load a directory, so we are eager loading the directory, not the namespace.

fxn commented

Another one, simpler:

test "eager loads all files, ignoring other directories (same namespace)" do
  files = [
    ["a/m/x.rb", "M::X = 1"],
    ["b/m/y.rb", "M::Y = 1"],
  ]
  with_files(files) do
    loader.push_dir("a")
    loader.push_dir("b")
    loader.setup
    loader.eager_load_dir("a/m")

    assert eager_loaded?(files[0])
    assert !eager_loaded?(files[1])
  end
end

Same idea. There are a few others that show the asymmetry.

fxn commented

Anyway, I'll think about it.

fxn commented

The instance method is in main (docs).

After thinking about this feature these days, I am not very convinced about the class method. I find suspcious that we are sending messages to loaders that have no relationship with the namespace whatsoever.

Let's imagine you implement a group of gems that work together, and a project depends on them and on Karafka. If that group of gems implement MyFramework::Services that they want to eager load, or the parent application wants to eager load, why should we send that message to something totally unrelated like Karafka?

When I design this library, I work with the vision that there are multiple loaders in the process managing unrelated projects. Whether a gem loads with Zeitwerk or not is not even public interface.

In particular contexts, however, some of those loaders may be related (like main and once are in Rails), but that belongs to a higher level, and such relationship has to be coordinated ad-hoc by the unit to which that higher level belongs.

I wonder if the use case you have in mind could be addressed by introducing a group of loaders to which you know it makes sense to broadcast:

# Every gem/loader in MyFramework.
MyFramework.register_loader(loader)

And later:

MyFramework.loaders.each do |loader|
  loader.eager_load_namespace(MyFramework::Services)
end

What do you say?

could be addressed by introducing a group of loaders to which you know it makes sense to broadcast:

I think it couple the caller with your setup.

Imagine I have a Components:: namespace that is populated by multiple loaders coming from multiple sub-gems. Every time I introduce a new gem that publish some new components, I'd need to go update that loader list.

IMHO, when we're dealing with namespaces (and not paths), it doesn't concern a specific loader.

That being said, the feature I want don't have to be implemented by Zeitwek, it could perfectly be done in pure ruby by listing and accessing the autoloads defined on a module:

def eager_load_namespace(mod)
  mod.constants.each do |const|
    if mod.autoload?(const) }
      value = mod.const_get(const)
      eager_load_namespace(value) if value.is_a?(Module)
    end
  end
end
fxn commented

Every time I introduce a new gem that publish some new components, I'd need to go update that loader list.

No, no, the gem does it in my mind.

You say: Hey component extensions, you have to use Zeitwerk in your project (to have things eager loadable as desired), and in your entrypoint you are responsible for registering your loader (so I can eager load):

# lib/component_foo.rb

loader = Zeitwerk::Loader.for_gem
loader.setup

MyComponentsFramework.register_loader(loader)
fxn commented

Alternatively, the gem could be required to eager load the namespace.

fxn commented

Well, nuances depend on the exact situation, let's discuss later :).

fxn commented

We discussed this further with @byroot out-of-band.

It is one of those APIs for which you don't have a definitive answer, only pros and cons and intuitions.

On one hand, there's the objections I commented above.

On the other hand, if you compare the concept of "registering loaders" and the simplicity of the contract "just toss your stuff under this shared namespace", the convenience of the latter is arguably superior.

Also, we share a vision, which is that Zeitwerk should be as discreet as possible. You do the bare minimum to set things up, and the lib gets out of the way and just works. The registering contract resurfaces the presence of the library in a sense.

So, all things considered, let's bet on the class method. It is already written (docs).

fxn commented

Version 2.6.2 is out, includes this feature.