Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

engine_cart generated app has something in ./lib/generators which disturbs Rails 7.1 zeitwerk #117

Open
jrochkind opened this issue Nov 7, 2023 · 1 comment

Comments

@jrochkind
Copy link

jrochkind commented Nov 7, 2023

Engine cart seems to put something in ./lib/generators/test_app_generator.rb in the generated app?

I am not sure why the generated app needs to have a generator file placed in it, is this necessary and intentional?

When trying to use with Rails 7.1 -- which also by default autoloads ./lib for the first time, I think -- this winds up disturbing Rails 7.1 zeitwerk.

/home/circleci/.rubygems/gems/zeitwerk-2.6.12/lib/zeitwerk/loader/callbacks.rb:33:in `on_file_autoloaded': expected file /home/circleci/project/.internal_test_app/lib/generators/test_app_generator.rb to define constant Generators::TestAppGenerator, but didn't (Zeitwerk::NameError)

There are ways to tell zeitwerk to ignore this directory... but they are difficult to use in the engine_cart "remote control create an app on the fly as part of CI" use case.

What is the reason engine_cart is generating a generator into the generated app (phew!) in the first place, is it really necessary? If so... we need to figure out the best way to get zeitwerk to be okay with it.

@jrochkind
Copy link
Author

jrochkind commented Nov 8, 2023

The immediate cause of this is that Rails 7.1 by default adds RAILS_ROOT/lib as an autoload path, by generating apps with config.autoload_lib(ignore: %w(assets tasks)) into config/application.rb. (note that generators is not included in the ignore, if were by default we'd be good). See here, and I further note that fresh generated Rails 7.1 apps do have that config.autoload_lib in them.

This then causes problems for zeitwerk, to have things in RAILS_ROOT/lib, not zeitwerk-ignored, that aren't standardly directory mapped -- ./lib/generators/test_app_generator.rb being ::TestAppGenerator instead of Generators::TestAppGenerator.

The zeitwerk README actually talks about the ./lib/generators case specifically in the context of a GEM_ROOT/lib directory, and talks about adding that directory to the zeitwerk ignore path.

We have a somewhat different issue, because it's not inside gem source, but a generator at local app RAILS_ROOT/lib/generators -- and an automatically generated local app, so changing it's configuration involves an extra layer of indirection.

The simple solution I've found is to have the engine gem configuration tell the local app zeitwerk to ignore RAILS_ROOT/lib/generators.

    # eg in blacklight/lib/blacklight/engine.rb

    config.before_configuration do
      # see https://github.com/fxn/zeitwerk#for_gem
      # Blacklight puts a generator into LOCAL APP lib/generators, so tell
      # zeitwerk to ignore the whole directory? If we're using zeitwerk
      if Rails.autoloaders&.main.respond_to?(:ignore)
        Rails.autoloaders.main.ignore(Rails.root.join('lib/generators'))
      end
    end

https://github.com/projectblacklight/blacklight/blob/3dd752b05c424874c1c7ce9033247e4a5ab91680/lib/blacklight/engine.rb#L9-L16

What I don't love about this solution

  • We are configuring this for any app using the gem, when we really only need it for engine_cart-generated test apps in CI.

  • We are excluding the entire RAILS_ROOT/lib/generators, which could apply to things other than our own test_app_generator.rb. If a local app that happens to be using the gem is trying to use lib/generators in a "normal" way (with a Generators:: namespace), this will mess it up -- a pretty unlikely use case, but it would be very confusing to debug and I don't like leaving this trap.

But I am unable to find a better solution at present, that does not require engine_cart changes, and want to get several gems on Rails 7.1 without waiting for engine_cart changes.

Better solutions?

Ideally engine_cart would somehow take care of this itself, without requiring additional custom code to be added to a gem using engine_cart.

  • Does the test_app_generator.rb actually need to be generated into the .internal_test_app at all?

    • If not, this would be ideal. Why do we need to generate a generator into the generated app? But I don't understand the architecture or intent enough to have the answer to this question.
  • Can test_app_generator.rb be generated into some other location, that doens't cause a problem for zeitwerk? But is still available as a generator?

    • i can't figure out such a place
  • Can engine_cart, when generating an .internal_test_app, generate into it the configuration to ignore the local RAILS_ROOT/lib/generators?

    • Adding complexity, when added complexity is the last thing engine_cart needs
    • But at least it only applies to generated .internal_test_app, and not to all apps using an engine_cart-using gem.

Other ideas, or other understanding of what's going on?

jrochkind added a commit to samvera/questioning_authority that referenced this issue Nov 8, 2023
jrochkind added a commit to samvera/questioning_authority that referenced this issue Nov 8, 2023
jrochkind added a commit to samvera/questioning_authority that referenced this issue Nov 8, 2023
jrochkind added a commit to samvera/questioning_authority that referenced this issue Nov 8, 2023
cjcolvar added a commit to samvera/noid-rails that referenced this issue Jan 26, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant