Support controllers_path generator option for controller and manifest tasks. #152

jhirn · 2025-02-03T20:34:00Z

This PR aims to support paths other than app/javascript for the root path in generator and manifest tasks. Similar attempts have been made/requested, but this PR uses config.generators instead of an initializer to set controllers_path. A small drawback is the manifest tasks load :environment to access configuration from application.rb, but it plays nicely with generator help message and doesn't pollute the Stimulus namespace. Here's the usage:

module Rails4Lyfe
  class Application < Rails::Application
    ...

    config.generators do |g|
      ...
      g.stimulus :stimulus, skip_manifest: true, controllers_path: 'app/frontend/controllers'
      ...
    end
  end
end

I like using the generator because it keeps the controller consistent with the // Connects to data-controller=.. comment and naming conventions which get tricky with camelCase, under_score and dash-erized in play. Because the path is hard coded in the generator, the only option is to monkey patch it. Monkey patching the generator in my project made it difficult to reference the template from inside the gem requiring me to copy template as well.

If there's support for this option I'd be happy to proceed with updating docs and tests, although I had a pretty tough time getting the suite to run locally.

Thank you for considering this PR.

dhh · 2025-02-04T12:35:12Z

lib/tasks/stimulus_tasks.rake

    end

+    def controllers_path
+      js_root = Rails.application.config.generators.options.dig(:stimulus, :js_root) || "app/javascript"


Wouldn't js_root always be set, given that you have a default for the class_option? If so, I don't think we need to repeat the default here. And in fact, I'd just inline the entire call in the Rails.root.join.

I love the idea of using the class_option, however the value in Rails.application.config.generators is not set on the class without instantiating an instance with the options. I actually quite like this approach as the default is only in one place:

def controllers_path Rails.root.join(StimulusGenerator.new(['stimulus'], generator_options, {}).options.js_root, "controllers") end def generator_options Rails.application.config.generators.options.dig(:stimulus) end

I also had to add the following require statement to load StimulusGenerator.

# stimulus_tasks.rake require "generators/stimulus/stimulus_generator" # This worked in the rake task, but perhaps more appropriate in stimulus_generator.rb # Let me know if its better to have them in the rake task instead of the generator. require 'rails/generators' require 'rails/generators/actions' require "rails/generators/named_base" # already present

This also eliminates needing to add :environment to the rake tasks for a reason I do not fully grok, but I'm totally ok with.

I pushed up this change. LMK what you think.

However this pans out, thanks for the code review 👍

dhh · 2025-02-04T12:35:22Z

lib/generators/stimulus/stimulus_generator.rb

  source_root File.expand_path("templates", __dir__)

  class_option :skip_manifest, type: :boolean, default: false, desc: "Don't update the stimulus manifest"
+  class_option :js_root, type: :string, default: 'app/javascript', desc: "Root path for javascript files"


Always use " instead of ' for strings.

dhh · 2025-02-24T09:54:37Z

lib/tasks/stimulus_tasks.rake

+
    task :display do
-      puts Stimulus::Manifest.generate_from(Rails.root.join("app/javascript/controllers"))
+      Stimulus::Manifest.generate_from(Stimulus::Tasks.controllers_path)


Why remove the puts here? That would remove the actual display of the Manifest.

Habit because I'm usually beaten with a stick for leaving puts in code, but realize that was there to begin with.

puts for debugging is definitely not something you want to ship, but this puts is in service of what the task does: Display the manifest to the user on the terminal.

@dhh What are your thoughts adding a puts to stimulus:manifest:update to avoid surprise when js_root is not default?

desc "Update the Stimulus manifest (will overwrite controllers/index.js)" task :update do puts "Updating stimulus manifest in #{Stimulus::Tasks.controllers_path.relative_path_from(Rails.root)}" manifest = Stimulus::Manifest.generate_from(Stimulus::Tasks.controllers_path) ....

➜ rails stimulus:manifest:update 08:55:33 Updating stimulus manifest in app/frontend/controllers

dhh · 2025-02-24T09:56:30Z

lib/tasks/stimulus_tasks.rake

+      Rails.root.join(StimulusGenerator.new(["stimulus"], generator_options, {}).options.js_root, "controllers")
+    end
+
+    def generator_options


Since this isn't called anywhere else, I'd just inline it in controllers_path.

Not sure if there's line length limit enforced but we're at 154 as a oneliner:

def controllers_path Rails.root.join(StimulusGenerator.new(["stimulus"], Rails.application.config.generators.options.dig(:stimulus), {}).options.js_root, "controllers") end

This feels ok to me.

def controllers_path generator_options = Rails.application.config.generators.options.dig(:stimulus) Rails.root.join(StimulusGenerator.new(["stimulus"], generator_options, {}).options.js_root, "controllers") end

I do like this because the arguments to .join pop and isnt' any longer than line 8.

def controllers_path Rails.root.join( StimulusGenerator.new(["stimulus"], Rails.application.config.generators.options.dig(:stimulus), {}).options.js_root, "controllers" ) end

Not technically inline but easier to work with (e.g. debug) assigning generator_options since it's a pretty long method chain.

def controllers_path generator_options = Rails.application.config.generators.options.dig(:stimulus) Rails.root.join( StimulusGenerator.new(["stimulus"], generator_options, {}).options.js_root, "controllers" ) end

Wanted to provide some options for you to view. To avoid back and forth I'll push up option 4 since it's easiest on the eyes and avoids the super long method chain. Happy to change if you prefer one of the others.

I like:

def controllers_path generator = StimulusGenerator.new(["stimulus"], Rails.application.config.generators.options.dig(:stimulus), {}) Rails.root.join generator.options.js_root, "controllers" end

dhh · 2025-02-25T09:43:44Z

Made some changes. Think we probably need to add a test for this, though.

jhirn · 2025-02-25T14:04:52Z

Thanks for the updates. I've updated the title and original post to reference controllers_path in favor of js_root. I can add some tests as well.

How can I avoid uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError) when running bundle exec rake test? I installed Ruby 3.1.7 and added require "logger" to test_helper.rb which gets me running but, I probably shouldn't have to and still get a warning:

/Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:16:in `<module:LoggerThreadSafeLevel>': uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError)

    Logger::Severity.constants.each do |severity|
    ^^^^^^
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:9:in `<module:ActiveSupport>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:8:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger_silence.rb:5:in `require'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger_silence.rb:5:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger.rb:3:in `require'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support/logger.rb:3:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support.rb:29:in `require'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/activesupport-6.1.7.10/lib/active_support.rb:29:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/railties-6.1.7.10/lib/rails/command.rb:3:in `require'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/railties-6.1.7.10/lib/rails/command.rb:3:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/railties-6.1.7.10/lib/rails/cli.rb:12:in `require'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/railties-6.1.7.10/lib/rails/cli.rb:12:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/railties-6.1.7.10/exe/rails:10:in `require'
        from /Users/jhirn/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/railties-6.1.7.10/exe/rails:10:in `<top (required)>'
        from /Users/jhirn/.rbenv/versions/3.1.6/bin/rails:25:in `load'
        from /Users/jhirn/.rbenv/versions/3.1.6/bin/rails:25:in `<main>'
.

Two follow up questions:

Should we add something to the readme? I didn't see a section for generator configuration otherwise I'd amend it.
Should we observe controllers_path for stimulus:install:* tasks?

dhh · 2025-02-27T15:40:13Z

I've fixed the test issue. Merge with main, and then you can run bin/test.

Yes, we should document this.

Not sure what you mean about observing controllers_path?

jhirn · 2025-02-28T07:52:06Z

Not sure what you mean about observing controllers_path?

The templates use by rails stimulus:install are hard coded and do not respect controllers_path. Should they?

dhh · 2025-03-01T17:35:27Z

Don't think that matters. Since this is being run on "rails new" before you even get a chance to change the setting. I mean, of course, if you opted out of Hotwire, and then added it back later, then ok. But seems unlikely.

jhirn · 2025-03-05T15:43:17Z

Got side tracked with life but I should have some tests up by before next week.

jhirn · 2025-03-10T16:58:31Z

@dhh Pushed up some tests and added validations for blank and absolute controller path ( blank path becomes absolute because we concatenate with /#{controller_name}. Both cases would fail with Read-only file system @ rb_sysopen so I added specific RuntimeError messages to be a bit more friendly.

I could also see using default when blank and converting absolute paths to relative by removing any leading file system separators. It's a trade off between strictness and ergonomics. I personally prefer the latter.

As far as documentation, I only updated USAGE to give an example of controllers-path. I feel like the README should have a section detailing generator configuration that includes skip_manifest in addition to controllers_path but didn't want to overstep. Not sure if that should be part of this or a separate PR to fully document generator configuration.

tomu123 · 2025-09-08T01:45:30Z

lib/tasks/stimulus_tasks.rake

    desc "Update the Stimulus manifest (will overwrite controllers/index.js)"
-    task :update do
-      Stimulus::Manifest.write_index_from(Rails.root.join("app/javascript/controllers"))
+    task update: :environment do


Why do we need to add the environment task as a dependency? It doesn't seem necessary.

tomu123 · 2025-09-08T01:47:14Z

lib/tasks/stimulus_tasks.rake

    end

+    def controllers_root
+      Rails.root.join \


It's a little hard to read and still too long in my opinion, I believe the suggestion from @dhh should be good

def controllers_path generator = StimulusGenerator.new(["stimulus"], Rails.application.config.generators.options.dig(:stimulus), {}) Rails.root.join generator.options.js_root, "controllers" end

tomu123 · 2025-09-08T01:48:46Z

amazing work @jhirn, I really would like to see this merged since i have a use case for this in one of the app I am currently working on. Do you know when it may be merged, @dhh?

dhh reviewed Feb 4, 2025

View reviewed changes

jhirn force-pushed the stimulus-generator-path-option branch from 484cc45 to 66187f7 Compare February 4, 2025 19:00

dhh reviewed Feb 24, 2025

View reviewed changes

jhirn changed the title ~~Support js_root generator option for controller and manifest tasks.~~ Support controllers_path generator option for controller and manifest tasks. Feb 25, 2025

jhirn and others added 10 commits March 10, 2025 10:54

Support js_root generator option for controller and manifest tasks.

def4f67

Use StimulusGenerator .class_options for js_root in rake task.

8e7afca

Use .js_root instead place of hash accessor

2b4381d

Inline generator_options as they aren't used elsewhere

821174b

add puts back to stimulus:manifest:display

280e31a

Style changes

99cd4b0

Use a fully formed controllers_path

98e8a2c

Use controllers_path

f2dbb1e

Style

e90a3d5

Fix rebase: controller_path => controller_root

f057e18

jhirn force-pushed the stimulus-generator-path-option branch from f761714 to f057e18 Compare March 10, 2025 16:00

Add tests. Validate controller_path is set and not absolute directory.

496209e

tomu123 reviewed Sep 8, 2025

View reviewed changes

Support controllers_path generator option for controller and manifest tasks. #152

Are you sure you want to change the base?

Support controllers_path generator option for controller and manifest tasks. #152

Conversation

jhirn commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhirn Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhirn Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhirn Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhirn Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhirn Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhh commented Feb 25, 2025

Uh oh!

jhirn commented Feb 25, 2025

Uh oh!

dhh commented Feb 27, 2025

Uh oh!

jhirn commented Feb 28, 2025

Uh oh!

dhh commented Mar 1, 2025

Uh oh!

jhirn commented Mar 5, 2025

Uh oh!

jhirn commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomu123 commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

jhirn commented Feb 3, 2025 •

edited

Loading

jhirn Feb 4, 2025 •

edited

Loading

jhirn Feb 4, 2025 •

edited

Loading

jhirn Feb 24, 2025 •

edited

Loading

jhirn Feb 24, 2025 •

edited

Loading

jhirn Feb 24, 2025 •

edited

Loading

jhirn commented Mar 10, 2025 •

edited

Loading

tomu123 commented Sep 8, 2025 •

edited

Loading