Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Project-wide config for generators #623

Closed
Tracked by #617
Spone opened this issue Feb 15, 2021 · 11 comments
Closed
Tracked by #617

Project-wide config for generators #623

Spone opened this issue Feb 15, 2021 · 11 comments
Assignees
Labels

Comments

@Spone
Copy link
Collaborator

Spone commented Feb 15, 2021

Feature request

Currently, you have to pass the flags every time you run a generator. Being able to override the default values in config/application.rb would be useful.

Here is the suggested syntax, that would follow Rails convention:

config.generators do |g|
  g.view_component preview: true, sidecar: true # this would always create a preview and a sidecar directory
end

Another example:

config.generators do |g|
  g.view_component inline: true # this would always create inline components
end

There already is a way to configure the template engine (see below) so we wouldn't change that.

Such global config option could obviously be overridden when running the generator, by using flags as usual.

Motivation

In some cases, it makes sense to use generator flags every time you generate components (such as --sidecar), but you can easily forget.

Related:

Reference

Existing generator flags

# Override the default template engine
bin/rails generate component Example title content --template-engine slim

# Generate a preview
bin/rails generate component Example title content --preview

# Place the view and assets in a sidecar directory
bin/rails generate component Example title content --sidecar

# Use inline rendering (no template file)
bin/rails generate component Example title content --inline

By the way, the --inline flag appears to be only taken into account by the ERB generator, it should be added to Slim and Haml as well. I opened a separate issue for that: #625

Also, what about a --stimulus flag to generate the Stimulus controller? I opened another separate issue: #627

Existing configuration settings

Here is a quick recap of the current configuration settings in ViewComponent.

The generators.template_engine option from Rails is taken into account.

Apart from that, most of the ViewComponent-specific settings are for now related to previews.

# config/application.rb

# Use your template engine of choice (reusing the existing Rails option)
config.generators.template_engine = :slim

# Set the default layout for previews
config.view_component.default_preview_layout = "component_preview"

# Set the path to previews
config.view_component.preview_paths << "#{Rails.root}/lib/component_previews"

# Set the route for previews
config.view_component.preview_route = "/previews"

# Set the controller to use for previews
config.view_component.preview_controller = "MyPreviewController"

# Set the controller to use for tests
config.view_component.test_controller = "BaseController"

# Disable the render monkey patch (Rails < 6.1)
config.view_component.render_monkey_patch_enabled = false # defaults to true
@Spone Spone mentioned this issue Feb 15, 2021
23 tasks
@Spone Spone added the v3 label Feb 15, 2021
@joelhawksley
Copy link
Member

@Spone fantastic! I'm 👍🏻 to going this route.

I'll have to give the Stimulus bit some more thought, but in the meantime let's definitely add these options for all of the existing generator options.

@elia
Copy link
Collaborator

elia commented Apr 17, 2021

Currently, you have to pass the flags every time you run a generator.

For this same reason, we ended up writing our own generators in all apps we introduced components into. The generators included a custom ERB, sidecar CSS, JS, and translations, in one case the preview was changed to use ERB templates instead of the class, on another the JS was a custom controller class for AlpineJS.

Not sure what's the best way to facilitate all of this. Maybe just suggesting users to generate their generator is enough, or explaining how to override generator templates, or a way to add/replace templates in the current generator.

@Spone
Copy link
Collaborator Author

Spone commented Apr 21, 2021

It makes sense to recommend using a custom generator tailored for each project, instead of adding more configuration flags (by the way it's also @palkan's approach in view_component-contrib). It's the most sensible approach to cater all needs without adding complexity to ViewComponent.

But I also think that each flag available in the standard generator provided by ViewComponent should at least be available as a global configuration setting as well. It makes it easier to get started with the gem, then you can go further and write your own generator (ViewComponent could provide a easy path to do that).

@Spone
Copy link
Collaborator Author

Spone commented Feb 11, 2022

In preparation for v3 (#617) Let's sum up the progress on this. Here are the relevant PRs:

Thanks again to everyone who contributed :)

The syntax is currently:

config.view_component.generate.preview = true
config.view_component.generate.sidecar = true

Do you think we should aim for the more conventional syntax described above?

config.generators do |g|
  g.view_component preview: true, sidecar: true # this would always create a preview and a sidecar directory
end

If we do, it would be great to ship it in v3 to minimize disruption.

@Spone Spone self-assigned this Feb 11, 2022
@boardfish
Copy link
Collaborator

The config.generators block isn't responsible for configuring things like template engines and test frameworks themselves, only choosing which ones to use. So I'd argue this is fairly nonstandard. I feel it makes sense for the config to be where it is now, because it's configuration for something provided by ViewComponent — I feel the relation to generators comes second. If someone were to remove ViewComponent from their project, this would mean they'd have to remove configuration from another place potentially.

This feels like a similar discussion to where the config option landed in rails/rails#42279 — in that, we chose to isolate the config option to the most common thread (the ERB template handler) even though the user's perception of what this would affect differed (Action View).

@Spone
Copy link
Collaborator Author

Spone commented Feb 14, 2022

The config.generators block isn't responsible for configuring things like template engines and test frameworks themselves, only choosing which ones to use.

I have a few counter-examples:

  • g.test_framework :test_unit, fixture: false: the fixture option is related to the generator behavior
  • g.orm :data_mapper, migration: true
  • g.force_plural allows pluralized model names

If someone were to remove ViewComponent from their project, this would mean they'd have to remove configuration from another place potentially.

I agree with this argument, even if in pratice, they are most of the time colocated in config/application.rb.

@boardfish
Copy link
Collaborator

Seems like the pattern with those examples is to toggle whether or not to run related generators (e.g. with hooks). Running on that basis, sidecar, stimulus_controller, and locale could be configurable this way, but distinct_locale_files might seem out of place.

If I could choose between either I'd personally lean towards configuring alongside the rest of view_component rather than with generators, but I can see reasons for both sides now 👍

@Spone
Copy link
Collaborator Author

Spone commented Feb 14, 2022

There is also the argument that the config.view_component.generate status quo does not need any additional work :)

@Spone
Copy link
Collaborator Author

Spone commented Feb 15, 2022

Closing now that the last PR (#1194) is merged!

Feel free to reopen if you want to continue the above discussion about config syntax.

@Spone Spone closed this as completed Feb 15, 2022
@Spone
Copy link
Collaborator Author

Spone commented Feb 22, 2022

@boardfish have you used the config.view_component.generate.sidecar = true syntax with success?

I just tried in the replicate-bug folder: if you add the following

config.view_component.generate.sidecar = true

and start the server, you get:

$ rails s
/workspaces/view_component/replicate-bug/config/application.rb:37:in `<class:Application>': undefined method `sidecar=' for nil:NilClass (NoMethodError)
        from /workspaces/view_component/replicate-bug/config/application.rb:23:in `<module:ReplicateBug>'
        from /workspaces/view_component/replicate-bug/config/application.rb:21:in `<top (required)>'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/zeitwerk-2.5.2/lib/zeitwerk/kernel.rb:36:in `require'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/zeitwerk-2.5.2/lib/zeitwerk/kernel.rb:36:in `require'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/railties-7.0.0/lib/rails/commands/server/server_command.rb:137:in `block in perform'
        from <internal:kernel>:90:in `tap'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/railties-7.0.0/lib/rails/commands/server/server_command.rb:134:in `perform'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/thor-1.1.0/lib/thor/command.rb:27:in `run'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/thor-1.1.0/lib/thor/invocation.rb:127:in `invoke_command'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/thor-1.1.0/lib/thor.rb:392:in `dispatch'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/railties-7.0.0/lib/rails/command/base.rb:87:in `perform'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/railties-7.0.0/lib/rails/command.rb:48:in `invoke'
        from /usr/local/rvm/gems/ruby-3.0.3/gems/railties-7.0.0/lib/rails/commands.rb:18:in `<top (required)>'
        from bin/rails:4:in `require'
        from bin/rails:4:in `<main>'

@boardfish
Copy link
Collaborator

Can confirm this is an issue. I've found the fix and I'll put a PR in now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants