fxn / zeitwerk

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

Using Zeitwerk in gem development #271

Closed pepawel closed 1 year ago

pepawel commented 1 year ago

Hi,

Thank you for this amazing gem!

I use Zeitwerk in my own gem. It greatly improves my development experience. However, I found it's not much needed in runtime, i.e. when someone uses my gem in his/her app/gem. In this situation, everything required is to load all the gem files. Zeitwerk features are not much needed then.

I believe in many situations it is best to avoid using Zeitwerk in runtime to minimize dependencies. Zeitwerk is a rock-solid gem, but still when included the attack surface is larger, complexity grows, and the potential to break increases just a little bit.

I would like to suggest mentioning this in README. This way less gem authors will choose to depend on Zeitwerk if it is not needed. What do you think?

I published a post about this specific problem here: https://blog.pawelpokrywka.com/p/gem-with-zeitwerk-as-development-only-dependency

fxn commented 1 year ago

Ruby just executes code. It loads your gem files lazy or eagerly, and executes whatever client code wants to execute.

In order to interpret the gem's files you are using Zeitwerk, and this is so regardless of whether your gem is being executed in your laptop, or CI, whether you load it in a console, or via the test suite, or as someone else dependency. For Ruby, all is "executing" your code.

So, there is no way to ship your gem without Zeitwerk and develop with Zeitwerk at the same time. Because constants are not going to be found by client code.

The alternative is to not use Zeitwerk and write manual require or autoload calls.

fxn commented 1 year ago

My reply above is a statement about what I consider to be in the scope of the library. But let me add thoughts about the trick in the blog post.

One way to load a library without having require calls scattered through the gem files is to issue require statements in the entry point. Eager loading, in practice. This is, for example, what nanoc did before migrating to Zeitwerk (see https://github.com/nanoc/nanoc/commit/bc8f0cab39131f74568449e557f89e75961b3e8b). The post wants to use Zeitwerk to generate such a list of ordered require calls.

This approach has caveats. It is the kind of thing you can do as a user if you control your project and have your desire to ship with one dependency less (not everyone has this goal), but that has caveats.

For example, if your code has

class Foo
  if ...
    include Foo::Bar
  else
    include Foo::Baz
  end
end

the order of execution depends, and the consequences on transitive dependencies do too.

Zeitwerk allows you also to opt-out of eager loading some parts of the project, those files could be autoloaded on demand, but would not necessarily appear in the generated require list.

Additionally, your gem is being developed in a way, and ships a different code base. I personally wouldn't do it this way. I would use Zeitwerk to generate the file with requires always, and run the test suite against that. That is, the gem itself would not even have the Zeitwerk constant, a utility in bin would use Zeitwterk as a generator and done, out of the way. This utility does not ship with the gem.

In this apprach, you need to regenerate always, yes. But at some point there has to be a trade-off. My trade-off would be this other one. Point being, the test suite would test exactly what the gem ships.

So, I believe the trick in your blog post is cool: You can use Zeitwerk to generate a require list assuming your gem does not do anything "special". Defining "special" is difficult but may be enough for a post. However, not so in official docs and support, I don't see Zeitwerk shipping with something built-in for this.

fxn commented 1 year ago

Another example of something less "special". In this pretty common pattern:

# lib/foo.rb
module Foo
  include Bar
end

# lib/foo/bar.rb
module Foo::Bar
end

you cannot really load lib/foo/bar.rb before lib/foo.rb, because it needs Foo to be defined. Of course, you cannot load them the other way around either. In this project you cannot generate the require calls.

(Zeitwerk does support that chicken-and-egg use case, you surely know it.)

fxn commented 1 year ago

Yeah, so that is the resolution.

Zeitwerk cannot ship "use at your own risk, may or may not work" features. If it ships, it works robustly.

Concrete projects may have more control and may use that trick in the post.

pepawel commented 1 year ago

Wow, that was great feedback!

To sum things up, you observed my approach won't work when:

I haven't thought about those use cases. Thank you for making me aware of them!

I accept those tradeoffs for my particular needs. At the same time, I understand there may be projects depending on them, which disqualifies my approach entirely in those cases.

Also, you noticed the difference between in-development code and released code. That's also a good point.

To address it, I've adjusted my code to make sure the file containing "require" statements is included in the git repository. And it is regenerated before each test run. This way I can make sure I test the exact same code as the one to be released. As the file with "require" statements is available, I can quickly check if it has problems to resolve.

When working on code I still use Zeitwerk for convenience. But this time, if I introduce conditional/delayed code loading or circular references, my tests will tell me. Given I test before I release, the gems I build should be fine.

I'm going to update my post soon. For now, I pushed my changes. I'm happy, as the discussion resulted in the code being much simpler than before. The updated repository may be handy if my explanations were not clear enough. Here is a diff for interested parties: https://github.com/pepawel/zeitgeist/commit/88bef86e56e72ad0bdb4061604c7e368239c023f

Given my approach is not universal, I agree it doesn't need to be recommended in official docs. However, I believe there are projects that accept the tradeoffs you listed and at the same time try to limit runtime dependencies. Showing the potential direction while informing them of consequences could help them achieve their goals. But of course, I don't want to suggest it's Zeitwerk's responsibility. Also from your last comment, I conclude it doesn't fit well into project policy which I fully respect.

Again, thank you for your comments Xavier, I appreciate it.

fxn commented 1 year ago

Awesome, the trick is cool indeed.

Please, note that is not an exhaustive list of caveats. There might be more. Another one is that you'd need to create implicit namespaces by hand because

class Admin::UsersController < ...
end

won't have the Admin constant defined otherwise (assuming there is no admin.rb).

Bottom line, there will be dragons. Enumerating them all is tricky, and evaluating if your project has any of them is too. And it is a moving target.

It's brittle, but if you at least generate and have test coverage, at least you can tell that for now your project seems to be good.

(On my phone, quick reply waiting for a driver.)

fxn commented 1 year ago

@pepawel Hey, fact correction in your post (did not find a way to contact you directly other than LinkedIn).

Please don’t get me wrong. Zeitwerk is a rock-solid, actively developed gem with a huge contributor base.

While Zeitwerk has received awesome help and inspiration from several people (see https://www.youtube.com/watch?v=DzyGdOd_6-Y), if you look at the Git log, you'll see it is essentially a one-man project.

pepawel commented 1 year ago

I updated the post. I added the "Limitations" section and adjusted the code as I mentioned earlier. I also updated the caption on the last image. Thank you once again for your help.