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

Move generators into ViewComponent namespace #2130

Open
wants to merge 3 commits into
base: v4
Choose a base branch
from

Conversation

paul
Copy link

@paul paul commented Oct 9, 2024

What are you trying to accomplish?

#2112

ViewComponent generators pollute the generator namespace with a bunch of top level items, and claim the generic "component" name.

Before:

$ rails g
Usage:
  bin/rails generate GENERATOR [args] [options]

General options:
  -h, [--help]     # Print generator's options and usage
  -p, [--pretend]  # Run but do not make any changes
  -f, [--force]    # Overwrite files that already exist
  -s, [--skip]     # Skip files that already exist
  -q, [--quiet]    # Suppress status output

Please choose a generator below.

Rails:
  application_record
  authentication
  benchmark
  channel
  component

...

Erb:
  erb:authentication
  erb:component

Haml:
  haml:component

Locale:
  locale:component

Preview:
  preview:component

Rspec:
  rspec:component

Slim:
  slim:component

Stimulus:
  stimulus
  stimulus:component

Tailwindcss:
  tailwindcss:component

After

$ rails g

ViewComponent:
  view_component:component
  view_component:erb
  view_component:haml
  view_component:locale
  view_component:preview
  view_component:rspec
  view_component:slim
  view_component:stimulus
  view_component:tailwindcss
  view_component:test_unit

What approach did you choose and why?

I moved the generators into a ViewComponent module, instead of sitting at the top-level, so they end up namespaced in the rails g output. I also cleaned up the references to the files and classes within themselves, and in the tests.

Anything you want to highlight for special attention from reviewers?

@reeganviljoen
Copy link
Collaborator

@paul how is this com ing along, do you need any help in getting the CI to Passs

@paul
Copy link
Author

paul commented Oct 11, 2024

@reeganviljoen I haven't really looked into it. The CI failures are due to low coverage on completely unrelated files, so I assumed it had something to do with me branching my PR off v4 like @joelhawksley told me to. 😉

@reeganviljoen
Copy link
Collaborator

@paul i diddny realize, sorry. @joelhawksley are you able to see what failures are legit?

@joelhawksley
Copy link
Member

@paul I think simple cover short-circuited due to /home/runner/.rubies/ruby-head/lib/ruby/3.4.0+0/bundled_gems.rb:76:in 'Kernel.require': cannot load such file -- rails/generators/component/component_generator

@paul
Copy link
Author

paul commented Oct 16, 2024

@joelhawksley Ah, you're right, I didn't see that error at first. Its fixed now. Looks like the errors are releated to changes in Ruby main.

Failure:
RenderingTest#test_render_inline_allocations [test/sandbox/test/rendering_test.rb:18]:
Expected 107 allocations, got 110 allocations for Ruby 3.4.0.

@paul paul force-pushed the namespace-generators branch from 1eae092 to d01779f Compare October 16, 2024 16:15
@joelhawksley
Copy link
Member

@paul I fixed CI on the v4 branch, mind resolving the merge conflicts here?

Copy link
Member

@joelhawksley joelhawksley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks good to me. Mind updating the docs with the new command syntax and including a second sentence in the changelog summarizing the syntax changes?

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

Successfully merging this pull request may close these issues.

3 participants