rubyatscale / packs-rails

packs-rails establishes and implements a set of conventions for splitting up large monoliths.
MIT License
268 stars 26 forks source link

How to add custom paths to `Packs::Rails.config.paths`? #61

Closed TastyPi closed 6 months ago

TastyPi commented 1 year ago

I'm trying to add a custom paths to Packs::Rails.config.paths, but I can't figure out the right place to do it.

Here's the relevant part of my application.rb:

Bundler.require(*Rails.groups)

Packs::Rails.config.paths << "app/once"

module MyCompany
  class Application < Rails::Application
    config.load_defaults 7.0

    config.paths.add("app/once", autoload_once: true)
  end
end

I'm trying to set up an app/once directory as a kind of app/lib directory but with autoload_once: true. This fails with the following message:

~/.local/share/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/packs-rails-0.0.3/lib/packs/rails/integrations/rails.rb:31:in `block (2 levels) in inject_paths': undefined method `<<' for nil:NilClass (NoMethodError)

              @app.paths[path] << pack.relative_path.join(path)
                               ^^

This is caused by packs-rails using a config.before_configuration block to set up config.paths, which is run when inheriting from Rails::Application (i.e. before my config.paths.add(...) call). Is there an intended way to add custom paths? Or should I give up on using a custom path for this and find another way to set autoload_once: true?

ngan commented 1 year ago

Hi @TastyPi can you see if this works? https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#config-autoload-once-paths

TastyPi commented 1 year ago

I haven't tested it, but I'm sure that would work. It's more that it's weird packs-rails allows the paths to be configured, but it's only possible to configure a subset of standard Rails paths. Maybe that's intentional, just seems odd to me.

sarahsehr commented 1 year ago

I'm having a similar issue. I'd like to have an app/services and app/queries for each pack. I'm also trying to figure out where I should configure that.

I have the following in my application.rb

config.before_configuration do
      Packs::Rails.config.paths += %w[
        app/api
        app/services
        app/queries
      ]
end

Unfortunately, the Packs::Rails Railtie seems to execute before that.

alexevanczuk commented 1 year ago

@sarahsehr I'm pretty sure any app/* pack should automatically be configured with the normal autoloading behavior. Is that not working for you or are you trying to use autoload_once as well?

sarahsehr commented 1 year ago

@sarahsehr I'm pretty sure any app/* pack should automatically be configured with the normal autoloading behavior. Is that not working for you or are you trying to use autoload_once as well?

No, not trying to use autoload_once. I don't mean to hijack this issue, but I think we're experiencing the same behavior.

I'm not super familiar with how autoloading works, but from what I have debugged so far the autoloading is limited to this list: https://github.com/rubyatscale/packs-rails/blob/ab62d6bb40ceec52d6b5874625976d43719e08ac/lib/packs-rails.rb#L25-L39

alexevanczuk commented 1 year ago

Are your packs in the packs folder? e.g. packs/my_pack/app/services? Or are they located somewhere else? Are you getting a NameError due to no initialized constant? You might also want to make sure packs-rails is properly required before boot.

sarahsehr commented 1 year ago

Are your packs in the packs folder? e.g. packs/my_pack/app/services? Or are they located somewhere else? Are you getting a NameError due to no initialized constant? You might also want to make sure packs-rails is properly required before boot.

Yes, that is the location and yes I'm getting a NameError due to uninitialized constant. I have verified that the code has executed via debugger.

I added the following to my application.rb and things seem to load correctly now:

    config.before_configuration do |app|
      additional_paths = %w[
        app/api
      ]

      Packs.all.reject(&:is_gem?).each do |pack|
        additional_paths.each do |path|
          app.paths[path] = [] if app.paths[path].blank?
          app.paths[path] << pack.relative_path.join(path)
        end
      end
    end

I copied and tweaked the code from https://github.com/rubyatscale/packs-rails/blob/ab62d6bb40ceec52d6b5874625976d43719e08ac/lib/packs/rails/integrations/rails.rb#L27-L33

sarahsehr commented 1 year ago

Hmm, must be something more at play, because I'm finding app/serializers is loading in my pack even though that is in neither list.

ETA: Okay, looking like this is a separate issue from the original issue. My issues seem to be specific to app/api. Other custom app subdirectories autoload correctly.

TastyPi commented 1 year ago

@sarahsehr I normally see this issue when attempting to reference a class in config/initializers. Only classes defined in paths that are autoload_once can be used there (and this is the use case I'm trying to cover) https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoloading-when-the-application-boots.

alexevanczuk commented 1 year ago

@TastyPi is right that if you want to reference an application constant from config/initializers, you need to have your initializer run after your application loads:

# config/initializers/my_initializer.rb
Rails.application.config.to_prepare do
  # reference app constants here
end

@sarahsehr Is that where the NameError is coming from? If not, do you think it would be possible to reproduce the issue as a test case in this repo? Would love to be able to get this fixed for you 🙏 We can also go to zoom pairing if we are stuck.

sarahsehr commented 1 year ago

@TastyPi @alexevanczuk

You are correct that it was related to to_prepare. A constant was being set to an array of classes outside of the to_prepare block. Inside the to_prepare block, I was iterating through the array. However the entires in the array were not equivalent to the constants by the same name in the to_prepare block and that was causing the loading issues.

So definitely unrelated to this issue, sorry for hijacking! Thank you for the kind offers of help. :)

alexevanczuk commented 1 year ago

No worries! Glad your issue is settled.

@TastyPi When you get a chance, let me know your status on the issue! If you feel like you have a good solution to this that lives outside of packs-rails, we can add to the README!

TastyPi commented 1 year ago

So we're currently not using packs-rails, I ran into this issue when looking at replacing our manual implementation with it. It is possible to configure it while using packs-rails, you just need to manually add the paths:

  config.before_configuration do |app|
    app.config.paths.add("app/once", autoload_once: true)
    Packs.all.each do |pack|
      app.config.paths["app/once"] << pack.relative_path.join("app/once")
    end
  end

Just seems odd/confusing that packs-rails has a config option for paths but doesn't support non-default Rails paths.

alexevanczuk commented 1 year ago

Oh gotcha, sounds like this workaround will work for now.

It sounds like what you'd be looking for (to use packs-rails) is for packs-rails to expose an API for specifying your non-default Rails paths so they can be included as well, does that sound right?

TastyPi commented 1 year ago

That would be nice, my workaround isn't a particularly big deal though, I was mostly curious how it was expected to work.