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

Fix component generator inconsistencies #155

Merged
merged 2 commits into from
Mar 16, 2024

Conversation

justinfrench
Copy link
Contributor

I'm new to Phlex, but I noticed a few inconsistencies with the expected location of components. They're generated into app/views/components, but the component generator USAGE Tailwind portion of phlex:install were referencing app/components.

I think the changes here match what the code does today, but if it is possible to use app/components, I'd be happy to work on a PR to make the clearer and configurable.

The component is generated in `app/views/components/`, not `app/components/`.
Components are generated into `app/views/components`, so this is unneccessary.
@joeldrapper
Copy link
Collaborator

Thanks for the PR! I think it was originally app/components and we changed it to app/views/components but must have missed this. I’m open to changing it back to app/components, since that’s somewhat an established pattern in ViewComponent already. What are your thoughts?

@justinfrench
Copy link
Contributor Author

I’ve long felt like the views directory in Rails is overloaded with views, plus layouts and partials.

So… I’m personally a bit enamored with the idea that there could be three separate ‘app’ directories for layouts, views and components.

But that’s straying pretty far from Rails, not sure if it would make Phlex more or less approachable.

@justinfrench
Copy link
Contributor Author

I’ve long felt like the views directory in Rails is overloaded with views, plus layouts and partials.

Okay, this is a way off-topic for the PR (and maybe even for Phlex), but I had a bit more of a think about this. Yes, the views directory is overloaded, but there's more to it: As I use Phlex more in my app, I'm also finding myself bumping up against Rails-ish naming conventions of appending Component and View onto everything, especially with quite fine-grained composable components (Button vs ButtonComponent, Label vs LabelComponent, etc).

There's nothing in Phlex dictating this convention, but naming collisions are a thing to consider. That leads me to think about namespacing with Components::Button instead of ButtonComponent. In most cases, we would be rendering Components::Button from something else in the Components:: namespace, so we'd simply be using Button.

# app/components/card.rb
class Components::Card < ApplicationComponent
  def template
    div class: "whatever" do
      render Heading.new(…)
      render Summary.new(…)
      render Button.new(…)
    end
  end
end

Extrapolating out from that as a naming convention, you'd end up with classes like these, I guess:

  • Layouts::Application, Layouts::Public, Layouts::Admin, etc
  • Views::Posts::Index, Views::Posts::Show, Views::Admin::Posts::Index, etc
  • Components::Button, Components::Form::Label, etc

I haven't tested all this yet, but one of the nice upsides of this might be fewer files & folders and/or keeping related classes together. Examples:

# app/views/posts.rb
module Views::Posts
  class Index < ApplicationView
    def template()
      # ...
    end
  end
  class Show < ApplicationView
    def template()
      # ...
    end
  end
end

My current Phlex app is pretty tiny and early stage though, so I'm not really extracting enough from a real world app.

  • Are there any larger scale open source Rails apps using Phlex (especially heavy use of components) I can reference for some more use cases?
  • Has anyone else been down some of these rabbit holes?

@joeldrapper
Copy link
Collaborator

I completely agree with you. I think this proposal in phlex is relevant to what we do here in phlex-rails. phlex-ruby/phlex#674

@joeldrapper
Copy link
Collaborator

In terms of this PR, let's fix it for app/views/components and continue using suffixes rather than namespaces. But let’s also open an issue to discuss making these changes in 2.0. We’re already planning a Phlex 2.0 release, so it makes sense to do one for PhlexRails too.

@joeldrapper joeldrapper merged commit d2dbd2a into phlex-ruby:main Mar 16, 2024
5 of 11 checks passed
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.

2 participants