Load hooks for generators

[rwz]

I'm trying to fix this bug: rails/jbuilder#100

The only way I see I can fix that is to redefine the ScaffoldControllerGenerator template and add hooks to it. And to do that I need to require a lot of code including rails/generators.

The problem is that if I do that in jbuilder gem, it'll create Rails::Configuration::Generators instance and skip all custom generator settings specified in other gems.

For example, if I have jbuilder and haml-rails gems enabled, jbuilder requires rails/generators just for template/hook, and then, later haml-rails is doing config.generators.template_engine :haml which never get into Rails::Generators.options because it was already initialized earlier.

The other problem is that it loads a lot of generators code even if generators are not needed.

I'm not sure how to proceed here, but one solution would be to add load hook for generator. It'll allow me to augment the generator only when it actually loads, which kinda solves both problems stated above.

[josevalim]

I am slightly confused by the need of this patch.

You don't need to require rails generators by hand. If you generator follows the proper naming convention, it will be loaded just when required. What am I missing?

[schneems]

Agreed, I read through the linked issue and through this description, i'm not sure I fully understand the problem or this solution, could you give me some more information?

[rwz]

Here's the situation. We need to redefine the default scaffold_controller_generator. One way to do that is something like this:

class MyEngine < ::Rails::Engine
  config.generators.scaffold_controller = :my_scaffold_controller
end

So, now when you invoke scaffold, which hooks for scaffold_controller, my_scaffold_controller gets invoked instead — just what we need. BUT, when you try to invoke scaffold_controller directly, the override doesn't work, and rails run original scaffold_controller

So, the other solution would be to augment the original scaffold_controller to do what we need. I've found the only way to do that, which is something like that:

require 'rails/generators'
require 'rails/generators/rails/scaffold_controller/scaffold_controller_generator'

module Rails
  module Generators
    class ScaffoldControllerGenerator

      # redefining methods and adding hooks

    end
  end
end

And that approach breaks a lot of things. First, it requires generators all the time, even when we don't need them .Second, it preemptively triggers generators configuration to be build, so other gems trying to change generators config later fail, because the configuration does not pick changes from engines after being loaded.

[rwz]

So, I think we might need a load hook for generators. I understand that it might be silly to add a load hook for each particular generator, but what about load hook for all generators? For example for rails/generators. That would solve the problem I've stated above.

[schneems]

I've never been involved in any of the decision making regarding any of the AS hooks, so i'm not sure if this is the best place to address this issue, or somewhere else. Seems like adding one hook after all generators are loaded would be more useful. @josevalim, what do you think?

[rwz]

Changed the pull request to actually run hook on rails/generators load.

[rwz]

Ok, I've figured out how to trick the configuration without load hook. Seen how it's implemented in active_model_serializers gem. This code does the trick:

class MyRailtie < ::Rails::Railtie
  generators do |app|
    Rails::Generators.configure! app.config.generators
    Rails::Generators.hidden_namespaces.uniq!
    require 'file_with_generator_overrides'
  end
end

...but it is quite untrivial code that relies on undocumented (AFAIK) private APIs. I'd rather have a load hook for rails/generators instead of doing this.

[parndt]

Do we want to add this hook?

[rwz]

Well I've solved my problem without this hook. Yet I think it might be helpful in the future.
—
Pavel Pravosud

On Sun, Apr 14, 2013 at 2:52 AM, Philip Arndt notifications@github.com
wrote:

Do we want to add this hook?

Reply to this email directly or view it on GitHub:
#9647 (comment)