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

Introduce support for 3rd-party component frameworks #36388

Merged
merged 2 commits into from Jun 13, 2019

Conversation

@joelhawksley
Copy link
Contributor

@joelhawksley joelhawksley commented Jun 3, 2019

Note: This PR initially was titled: Introduce support for ActionView::Component. I've updated the content to better reflect the changes we ended up shipping, to use the new name of GitHub's library, ViewComponent, and to remove mentions of validations, which we no longer use. - @joelhawksley, March 2020

Introduce support for 3rd-party component frameworks

This PR introduces structural support for 3rd-party component frameworks, including ViewComponent, GitHub's framework for building view components.

Specifically, it modifies ActionView::RenderingHelper#render to support passing in an object to render that responds_to render_in, enabling us to build view components as objects in Rails.

We’ve been running a variant of this patch in production at GitHub since March, and now have about a dozen components used in over a hundred call sites.

The PR includes an example component (TestComponent) that closely resembles the base component we're using at GitHub.

I spoke about our project at RailsConf, where we got lots of great feedback from the community. Several folks asked us to upstream it into Rails.

Why

In working on views in our Rails monolith at GitHub (which has over 4,000 templates), we have run into several key pain points:

Testing

Currently, Rails encourages testing views via integration or system tests. This discourages us from testing our views thoroughly, due to the costly overhead of exercising the routing/controller layer, instead of just the view. It also often leads to partials being tested for each view they are included in, cheapening the benefit of DRYing up our views.

Code Coverage

Many common Ruby code coverage tools cannot properly handle coverage of views, making it difficult to audit how thorough our tests are and leading to gaps in our test suite.

Data Flow

Unlike a method declaration on an object, views do not declare the values they are expected to receive, making it hard to figure out what context is necessary to render them. This often leads to subtle bugs when we reuse a view across different contexts.

Standards

Our views often fail even the most basic standards of code quality we expect out of our Ruby classes: long methods, deep conditional nesting, and mystery guests abound.

ViewComponent

ViewComponent is an effort to address these pain points in a way that improves the Rails view layer.

Building Components

Components are subclasses of ViewComponent and live in app/components.

They include a sidecar template file with the same base name as the component.

Example

Given the component app/components/test_component.rb:

class TestComponent < ViewComponent
  def initialize(title:)
    @title = title
  end
end

And the template app/components/test_component.html.erb:

<span title="<%= @title %>"><%= content %></span>

We can render it in a view as:

<%= render(TestComponent.new(title: "my title")) do %>
  Hello, World!
<% end %>

Which returns:

<span title="my title">Hello, World!</span>

Testing

Components are unit tested directly, based on their HTML output. The render_inline test helper enables the use of Capybara assertions:

def test_render_component
  render_inline(TestComponent.new(title: "my title")) { "Hello, World!" }

  assert_text "Hello, World"
  assert_selector "span[title='my title']"
end

Benefits

Testing

ViewComponent allows views to be unit-tested. Our unit tests run in around 25ms/test, vs. ~6s/test for integration tests.

Code Coverage

ViewComponent is at least partially compatible with code coverage tools. We’ve seen some success with SimpleCov.

Data flow

By clearly defining the context necessary to render a component, we’ve found them to be easier to reuse than partials.

Existing implementations

ViewComponent is far from a novel idea. Popular implementations of view components in Ruby include, but are not limited to:

In action

I’ve created a demo repository pointing to this branch.

Co-authored-by

A cross-functional working group of engineers and members of our Design Systems team collaborated on this work, including by not limited to: @natashaU, @tenderlove, @shawnbot, @emplums, @broccolini, @jhawthorn, @myobie, and @zawaideh.

Additionally, numerous members of the community have shared their ideas for ViewComponent, including but not limited to: @cgriego, @xdmx, @skyksandr, @jcoyne, @dylanahsmith, @kennyadsl, @cquinones100, @erikaxel, @zachahn, and @trcarden.

@rails-bot rails-bot bot added the actionview label Jun 3, 2019
merge master
@joelhawksley joelhawksley force-pushed the joelhawksley:actionview-component branch from 8e06178 to 1f261e0 Jun 3, 2019
@hayesr
Copy link
Contributor

@hayesr hayesr commented Jun 3, 2019

2¢ on Template architecture …
I agree that inline templates don't feel right. I'd love to see some kind of convention for matching template files.

Without having used this, my estimation is that it would be nice to have an app/components directory that would follow controller naming & namespacing conventions. I suppose there has to be an app/views/components directory. eg

# components
app/components/foo.rb
app/components/billing/total.rb

# corresponding views
app/views/components/_foo.html.erb
app/views/components/billing/_total.html.erb

Then, a method we could overwrite to make it dynamic would be super cool

class MyComponent < ActionView::Component::Base
  def partial_name
    super + "-#{object.status}"
  end
end
app/views/components/_my_component-pending.html.erb
app/views/components/_my_component-active.html.erb
@compiled = true

instance = new(*args)
instance.content = view_context.capture(&block) if block_given?

This comment has been minimized.

@tenderlove

tenderlove Jun 3, 2019
Member

I'd like it if we could pass this in to new rather than using a setter. I know it's to do with the superclass not matching signatures, but I wanted to comment with that anyway. Using a setter like this will complicate downstream uses because they'll need to know to call the setter. On the flip side, making it a required parameter to initialize will teach people it's required because they won't be able to construct the object without it. Plus you don't even need to read the docs to figure that out! 😊

This comment has been minimized.

@joelhawksley

joelhawksley Jun 5, 2019
Author Contributor

Interesting. I'm not opposed to that approach, but I wonder how it might look for components that don't render content.

Right now they just ignore the nil content accessor, but if we were always passing a content argument wouldn't they need to explicitly no-op that argument? Like so:

def initialize(_content:); end

module ActionView
class Component < ActionView::Base
include ActiveModel::Validations

This comment has been minimized.

@tenderlove

tenderlove Jun 3, 2019
Member

Do we want this by default? Validating all components will punish performance of components that have no validations. If most components require validation, it probably makes sense. I just don't have a good feeling of the requirement.

This comment has been minimized.

@skyksandr

skyksandr Jun 4, 2019

Maybe one more layer called class ApplicationComponent < ActionView::Component?
That will be the place to specify template handler as well as including ActiveModel::Validations

This comment has been minimized.

@joelhawksley

joelhawksley Jun 5, 2019
Author Contributor

My gut says it would be best to measure the impact of having validations enabled by default.

If it's not much overhead, I think this would be a nice default.

This comment has been minimized.

@joelhawksley

joelhawksley Jun 5, 2019
Author Contributor

@skyksandr I 💯agree about having an ApplicationComponent. We already follow this pattern internally and have found it useful.

@skyksandr
Copy link

@skyksandr skyksandr commented Jun 4, 2019

Speaking about variants:

Inline templates and/or Sinatra style

Pros: markup and methods close to each other
Cons: should we use inheritance to override it?

If you're not familiar with Sinatra's inline templates:

class OurComponent
  def hello_message
    "Hello, world!"
  end

  ...
end

__END__

@@ template
<div class="title"><%= hello_message %></div>

We can even drop @@ template

class OurComponent;  ...; end

__END__

<div class="title"><%= hello_message %></div>

Sidecar

I like the sidecar approach, would only ask to consider placing files as close in the directory structure as possible, like:

app/views/components/
  - my_component.rb
  - my_component.html.erb
  - my_component.html+mobile.erb

or

app/views/components/
  - my_component/
    - component.rb
    - template.html.erb
    - template.html+mobile.erb
@tenderlove
Copy link
Member

@tenderlove tenderlove commented Jun 4, 2019

I really like @skyksandr's proposal of:

app/views/components/
  - my_component/
    - component.rb
    - template.html.erb
    - template.html+mobile.erb

Seems like it would make packaging and distribution easy. I imagine you could have the same directory structure inside a gem just inside a lib folder.

@jonathanhefner
Copy link
Member

@jonathanhefner jonathanhefner commented Jun 4, 2019

We’re not sure if the current integration point in Rails (ActionView::RenderingHelper#render) is the best option.

To me, the API that komponent implements feels a bit more Railsy:

<%= component 'button', text: 'My button' %>

Namely, using a method other than render allows passing a String or Symbol (rather than a Class) to designate the component. It could also offer the possibility of positional args, where appropriate:

<%= component :section, "Some Label" do %>
  Some content.
<% end %>
@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Jun 5, 2019

Thanks for the feedback folks!

Since posting this PR, we've found ourselves wanting the template to be in a separate file.

Personally, I prefer @skyksandr's suggestion of:

app/views/components/
  - my_component.rb
  - my_component.html.erb
  - my_component.html+mobile.erb

As it feels a little heavy-handed to have one folder per component, especially if components can still define their templates inline in the same file.

Namely, using a method other than render allows passing a String or Symbol (rather than a Class) to designate the component.

One of the advantages to the current approach of passing a class name (TestComponent) to render is that we are performing an unambiguous class lookup. There is a significant amount of complexity/overhead in the way that Rails looks up templates via a symbol or a string.

More broadly, whether we use the existing render entrypoint is as much a functional question as it is a philosophical one:

Do we view this new architecture as something that should be thought of as a part of the existing Rails rendering functionality, or something entirely new? I feel like integrating with the existing view rendering mental model would make this new architecture easier to reason about for other developers.

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Jun 5, 2019

Then, a method we could overwrite to make it dynamic would be super cool

class MyComponent < ActionView::Component::Base
  def partial_name
    super + "-#{object.status}"
  end
end
app/views/components/_my_component-pending.html.erb
app/views/components/_my_component-active.html.erb

@hayesr one of the optimizations the currently proposed architecture gives us is the ability to easily precompile the ERB ahead of time, something that is currently difficult to do with existing Rails templates due to how formats/locals/etc can vary.

I'm curious what the use case would be for having separate templates for different status values in this example, instead of having a single template for all statuses.

Perhaps we could find a middle ground where we allowed dynamic template lookup, but pre-compiled all the templates that matched the name of the component ahead of time?

@tenderlove discussed this issue, and how we currently work around it at GitHub by always referencing fully qualified template paths, in his RailsConf keynote this year: https://youtu.be/8Dld5kFWGCc?t=1857

@jonathanhefner
Copy link
Member

@jonathanhefner jonathanhefner commented Jun 5, 2019

One of the advantages to the current approach of passing a class name (TestComponent) to render is that we are performing an unambiguous class lookup. There is a significant amount of complexity/overhead in the way that Rails looks up templates via a symbol or a string.

I absolutely agree that the performance benefits are a major selling point. But I think we could retain those benefits with a component registry. It could be a simple Hash (or HWIA) that's populated on app startup. It might also be a good place to trigger template pre-compilation.

@joelhawksley joelhawksley force-pushed the joelhawksley:actionview-component branch 3 times, most recently from 27eb8ef to 8ff1390 Jun 5, 2019
@hayesr
Copy link
Contributor

@hayesr hayesr commented Jun 6, 2019

@joelhawksley I would defer to the optimization. I was thinking about something I'm working on, a "status widget" similar to the GitHub issue badge, but with more potential for branching logic. And I prefer distinct templates to one template with a lot of <% if %> <% else %>. (Like your RailsConf talk example)

—I don't think a middle ground is necessary. Probably my use case would be better served by switching components rather than templates.

Now I've got new ideas 😁What happens when one Component inherits from another? Does the template path work like it does with Controllers? (I would vote yes please) For example one might expect that templates defined by SuperComponent would be used by SubComponent but that _sub_component.html+tablet.erb would override _super_component.html+tablet.erb

To the question of 1-folder-per-component, would it kill the pre-compilation to make it possible to override the directory but not the template name? I agree that it's a little heavy to have a folder for every component, but I think that app/views/components directory is going to get awfully cluttered. My preference would be to have folders, and I would set those explicitly in each Component class given the ability.

@zachahn
Copy link
Contributor

@zachahn zachahn commented Jun 6, 2019

I also kinda like the idea of a flat structure in app/views/components! And I also really like how this implementation uses class names instead of strings/symbols for lookup. Since I'm pretty familiar with the general rules of how Rails autoloads classes, it feels very straightforward to me :)

If components each had their own directories, would the class definition of the component look something like MyComponent::Component in the example we're talking about? (I personally feel more comfortable with a class name like ButtonComponent instead of Button::Component since the latter might have a hard time resolving if the Button class already existed somewhere in the application lol)

@joelhawksley joelhawksley force-pushed the joelhawksley:actionview-component branch from 8ff1390 to ccc1bf8 Jun 6, 2019
@nogtini
Copy link

@nogtini nogtini commented Jun 6, 2019

To go with the analogy of container/presentational components, how would the container-prefixed components handle fetching? Or is this left up to the controller? If so, wouldn't embedding components inside /views/components effectively make all components presentational/presentational components if they're delegating behavior management to the controller?

I mean this only in the sense that having a type of component that delegates behavior to the controller and is used to render presentational components seems to me very different than the React ecosystem where every component is effectively view and controller bundled together, all the way down.

If, on the other hand, the container components are in fact views that fetch and manage their own state, wouldn't that be pushing the boundaries of what it means to be a view and perhaps need at least a directory promotion outside of "all components are views"?

@baweaver
Copy link
Contributor

@baweaver baweaver commented Jun 9, 2019

Very fond of the idea, it opens up a lot of power in Rails.

One thing I'd be curious about, would it be possible to drop the render and imply it? Currently it's:

<%= render(TestComponent, title: "my title" ) do %>
  Hello, World!
<% end %>

Some ideas:

<!-- Class-based -->
<%= TestComponent.new(title: 'something') do %>
  Hello World!
<% end %>

<!-- Class-based, array accessor style -->
<%= TestComponent[title: 'something'] do %>
  Hello World!
<% end %>

The reason I ask is because if they retain that style it may make it more intuitive to have nested components:

<%= GridComponent[title: 'something'] do %>
  <%= Tile[name: 'something', image: 'src'] %>
<% end %>

Then in a file:

def tiles
  @tiles.map { |attrs| Tile[attrs] }
end

def self.template
  <<~'erb'
    <div><%= tiles %></div>
  erb
end

It could also potentially encapsulate some of the testing behavior from inferring the render, making a test potentially look like this:

def test_render_component
  assert_equal(
    %(<span title="my title">Hello, World!</span>),
    TestComponent[title: "my title"] { "Hello, World!" }.to_html
  )
end

Though stylistically this gets a bit towards more of what one would see in a PORO-oriented design, just a few ideas and musings.

@rmacklin
Copy link
Contributor

@rmacklin rmacklin commented Jun 9, 2019

One thing I'd be curious about, would it be possible to drop the render and imply it?

Some ideas:

<!-- Class-based -->
<%= TestComponent.new(title: 'something') do %>
  Hello World!
<% end %>

<!-- Class-based, array accessor style -->
<%= TestComponent[title: 'something'] do %>
  Hello World!
<% end %>

I like that [] syntax ("class-based, array accessor style") for the same reasons I talked about a similar alternative in joelhawksley/view-component-demo#1 (comment):

While I don’t mind the render Issues::Badge syntax, we also adopted a syntax backed by helper methods, which turns:

<%= render PullRequestBadge, state: issue.pull_request.state.to_sym, is_draft: issue.pull_request.draft? %>

into something like:

<%= pull_request_badge state: issue.pull_request.state.to_sym, is_draft: issue.pull_request.draft? %>

One thing we liked about this syntax is that it looks very much like a custom element (if you squint, you might not see a difference 😉). To that end, we even started using a pattern of accepting additional keyword arguments to be passed directly as DOM attributes in the helpers (in addition to the explicit arguments). So, for instance:

<%= issue_badge state: issue.state.to_sym, id: 'some_id', 'data-foo': 'bar' %>

would render the Issues::Badge component and pass along the id and data-foo attributes onto the (root) rendered element.

I pulled two small examples from our app into rmacklin/components_in_rails (the helpers are in app/helpers/components) and rendered them in a very bare bones style guide:
https://github.com/rmacklin/components_in_rails/blob/master/app/views/home/index.html.erb
with these examples:

style_guide/_button_group_example.html.erb

<%= button_group do |button_group| %>
  <%= button_group.button('Cat in the Hat', active: true, class: 'js-custom-class foobar') %>
  <%= button_group.button do %>
    <span class="fa fa-pencil"></span> Bartholomew and the Oobleck
  <% end %>
  <%= button_group.button('Yertle the Turtle', :'data-foo' => 'bar') %>
<% end %>

style_guide/_help_bubble_example.html.erb

<%= help_bubble(
  'You will need to restart this move in',
  accompanying_text: 'Looking for a different property?',
  direction: 'ne',
  id: 'different_property_help_bubble'
) %>

Here are a few more examples...

<%= options_dropdown label: 'Actions' do |dropdown| %>
  <%= dropdown.item 'Dropdown Item 1' %>
  <%= dropdown.item_link 'Dropdown Item 2', url: 'http://www.example.com' %>
<% end %>
<%= datapair key: 'Custom Class', value: 'Inspect Me', id: 'datapair_with_id', class: 'js-datapair' %>
<%= expandable_section title: 'title', initial_state: 'collapsed' do %>
  <div>Content</div>
<% end %>
<%= banner_alert title: 'Something', subtitle: 'I am a subtitle' do %>
  <%# ... %>
<% end %>

A more involved component looks like:

<%= filterable_collection do |fc| %>
  <%= fc.filter_box instructions: 'Enter in your filters below.' do %>
    <%= simple_form_for model.new, url: '/' do |f| %>
      <%# ... %>
      <% end %>
      <div class="btn-toolbar">
        <%= f.submit 'Search', class: 'btn btn-primary', :'data-disable-with' => 'Please Wait...' %>
      </div>
    <% end %>
  <% end %>

  <%= fc.results html_options: { id: 'results' } do %>
    <%# Initial results go here %>
  <% end %>
<% end %>

Given a component class such as MyComponent, the method name could be inferred from the class name by default (my_component), but any class could also choose to explicitly name its corresponding view_context method (e.g. Issues::Badge could define issue_badge if it made sense for the view method to be slightly different than the inferred default, or MyComponent could change its view method to deprecated__my_component when we've deprecated it).

We could also make it configurable whether the methods are mixed into the view context directly (which has a larger potential for naming collisions with other methods/locals) or onto a more granular scope where they could be called on an explicit receiver (which could come with a short alias), e.g.

<%= c.pull_request_badge state: issue.pull_request.state.to_sym, is_draft: issue.pull_request.draft? %>

This pseudo-custom-element syntax was appealing to our team, so we started using it a lot in our rails views.

@rmacklin
Copy link
Contributor

@rmacklin rmacklin commented Jun 9, 2019

I really like @skyksandr's proposal of:

app/views/components/
  - my_component/
    - component.rb
    - template.html.erb
    - template.html+mobile.erb

Seems like it would make packaging and distribution easy. I imagine you could have the same directory structure inside a gem just inside a lib folder.

We were thinking about distribution, too. I could imagine we may also want to distribute CSS (or SCSS) with the component, and potentially even associated javascript (to progressively enhance the component, e.g. with an associated stimulus controller). With sprockets, gems can distribute CSS/JS in lib/assets/, but it'd be nice for the associated stylesheet and javascript module to be co-located with the template and ruby class in lib/my_component/. That said, a lot of apps have replaced sprockets with node-based front end bundlers, so realistically these components might be distributed through a gem and a corresponding node module.

@jaredcwhite
Copy link

@jaredcwhite jaredcwhite commented Jun 9, 2019

I'm really digging this PR so far. I want to echo @rmacklin 's comment about bundling assets—in particular, I could see a serious use case here where the template format is something that could be parsed client-side as well, in which case a bundled Stimulus controller could import the template via Webpack and update a component client-side with the same template the server-rendered component uses. (The "holy grail" as it were…) As it is, I often find myself building a "component" as either a server-rendered partial OR a Stimulus controller + HTML template on the client, and that approach doesn't feel as Rails-y as I would like.

But even a basic first pass at this PR would be awesome.

@rmacklin
Copy link
Contributor

@rmacklin rmacklin commented Jun 9, 2019

update a component client-side with the same template the server-rendered component uses. (The "holy grail" as it were…)

I was having the exact same thoughts/dreams (having previously used mustache for shared client/server templates), but worried it might be getting too far out there. Hearing someone else say it is at least a little reassuring, heh

@dubadub
Copy link

@dubadub dubadub commented Jun 9, 2019

Components in Rails is the most desirable feature I'd like to see. Thank you very much for making that effort and bringing that topic to the surface, @joelhawksley.

Rails still has its power, and server-rendered apps solve many use cases. I think, for 80% apps, using SPA is overkill and Rails has its place under the sun and will have it for a long time. Also, now, when there is Stimulus which is elegant, simple yet powerful, I'm excited to see that the community wants to move in that direction. If Rails has a right componentisation approach, it will make my experience much better.

My Pain Points

  • Low transparency on the dependencies makes my new feature dev experience worse. CSS, JS and images are very detached from the context they're used. I need to switch a lot in file tree; things are easy to collide, peer developers don't know what's already been done if there are no strong naming conventions.
  • Low transparency on the dependencies pollutes asset files with dead code. The dependency of template from used css and js is not explicit. Often dead assets remain in the codebase because I forgot to delete them when I deleted a view.

Well, the point is one - low transparency.

It could be the result of my poor abilities to control, but I think Rails can make this work for us. At the end of the day, Rails puts developers' experience first.

How I'd like to see it solved

In my ideal world, I'd like to see my components defined with related JS, CSS, images, other assets and template in one place. The ruby part of it seems to be unnecessary and should be optional.

In my views, I will have templates related to controller actions, but the sole purpose of these will be to use existing components to build that particular page. Like that:

# app/views/pages/home.slim
= c("layout/main--wide") do
    = c("home/hero")
    = c("home/cta", user: @current_user)
    = c("home/how")
    = c("home/facts")

Ideally, it shouldn't have references any JS or css classes and only declares a page structure.

In component, I will have my template, JS, CSS:

app/views/components/
  - layout/
    - main--wide.html.slim
    - main.html.slim
    - layout.scss
    - layout.js
    - logo.png
  - home/
  	- hero/
  		- hero.html.slim
  		- hero.scss
  	...

We can assume that everything inside the component directory is needed.

And the really cool thing to do would be to add some magic in it.

When we are building assets, we know router endpoints, and for each endpoint, we can find related view and generate components tree.

Based on the tree and config, we can define rules to build our packs declaratively:

pack path: /^admin/, name: :admin
pack path: /^public/, name: :public
pack path: /^internal/, name: :internal

Then, during assets building, it will check all the routes starting with admin and build all the resources for production.

In runtime, it will serve a pack depending on the path.

It's not impossible, is it?

PS.

I spent some time playing with the approach taken by https://evilmartians.com/chronicles/evil-front-part-1, and as I know, it was inspired by komponent gem. I got rid of the Sprocket pipeline altogether, have a template, css, js (stimulusjs controllers), images located together. It is working very well. The downside of it is that I need to manually provide all the dependencies for JS, CSS, and set up webpack properly (it was painful). I can make a public example of the approach if there is interest.

@pixeltrix
Copy link
Member

@pixeltrix pixeltrix commented Jun 10, 2019

I assume that we'd want to support a ApplicationComponent in a similar fashion to how we have ApplicationModel, ApplicationJob, etc. ?

@paxer
Copy link

@paxer paxer commented Jun 11, 2019

this is a great proposal as a starting point, what I would personally like to see in Component feature is a Component has own isolated Javascript and CSS attached to it. JS and CSS could be optional but it is very handy to have it in one place as well as namespace CSS by component name automagically can make things even nicer.

so

app/views/components/
  - my_component/
    - component.js
    - component.css
    - component.rb
    - template.html.erb
    - template.html+mobile.erb
@ravicious
Copy link

@ravicious ravicious commented Jun 11, 2019

Just FYI regarding the presentational/container component approach by Dan Abramov, the linked blogpost starts with a disclaimer:

Update from 2019: I wrote this article a long time ago and my views have since evolved. In particular, I don’t suggest splitting your components like this anymore. If you find it natural in your codebase, this pattern can be handy. But I’ve seen it enforced without any necessity and with almost dogmatic fervor far too many times. The main reason I found it useful was because it let me separate complex stateful logic from other aspects of the component. Hooks let me do the same thing without an arbitrary division. This text is left intact for historical reasons but don’t take it too seriously.

I see how this pattern could've been useful for naming components in your app. OTOH, I think it should be possible to write appropriate naming guidelines without directly mentioning the concept of presentational & container components so that people don't go overboard with them like they did in Redux.

@jaredbeck
Copy link
Contributor

@jaredbeck jaredbeck commented Jun 11, 2019

Templating engine support
We should probably support arbitrary templating engines like normal Rails views.

Yes please. Eg. HAML is quite popular. Napkin math: downloads since late 2017: haml / rails = 7M / 45M = 20% of community uses haml (+/- the usual concerns about download counts)

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Jun 13, 2019

@sebyx07

Shouldn't we get inspired by https://hyperstack.org to write HTML in ruby?
Inspiration from https://github.com/ebryn/ember-component-css

And for style, ruby supports the syntax to generate css. Inspiration from https://github.com/ebryn/ember-component-css

def style
{
 '.label': { 'text-align': 'center' }
}
end

then the output Inspiration from https://github.com/ebryn/ember-component-css

.my-component-32131321 .label{
...
}

We've struggled with this debate internally.

While embedding other syntax in Ruby (I could see arguments for ERB/Slim/HAML/etc, CSS, JS, and others) might be appealing, we've run into friction/awkwardness with the inline ERB HEREDOC templates demonstrated in this PR. In fact, we're currently exploring using sidecar templates instead.

The reality is that many editors struggle to highlight multiple syntaxes in the same file. Heck, we don't reliably highlight HEREDOCs in diffs here on GitHub!

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Jun 13, 2019

@eric-hemasystems

Obviously the consensus on good architecture can evolve and the long success of gems like cells shows that people are finding success with this strategy. But at the same time if feels odd to re-add a concept that was removed previously and declared a bad idea. Are we sure the architectural concerns that the Rails once had with this strategy are no longer relevant?

Are we 100% sure? Not at all.

As you have said, there is plenty of evidence in the community that people are finding success with this strategy.

A primary focus for us as we continue to work on this project is how to reason about, and then communicate, the role view components should fill in the architecture of a Rails app.

For now, we are focusing on building out our component architecture so that we can learn more through experience.

Our latest "ah-ha!" moment has been that perhaps view components have the potential to be a codification of view models/presenters/decorators, common patterns in many Rails apps but not part of the official Rails architecture today. To that end, we're experimenting with creating components that are combinations of existing view & view model pairs.

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Jun 13, 2019

@mockdeep

I've been thinking a lot about the issues mentioned above, especially with regards to code coverage against views. Maybe this is out of left field, but I've often wondered why it's not more common to do the templating in Ruby rather than rendering erb/haml/slim. Erector did this, though it hasn't been very actively maintained for a while. Rails also has the content_tag helper, which would be pretty similar with aliases for tag names. Something like this could allow for even better coverage metrics.

Oh, how I would love to open-source our components so you could see how much this is already true for us!

Our component stack has very few HTML tags at this point. Most of our components are using content_tag under the hood 😄.

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Jun 13, 2019

@Spone

Hi, Komponent co-creator here. I'm super excited by all this. I've been using this View Component approach in all our web projects for the past 2 years, and I'm convinced it would be super useful to have this as part of Rails! It's a very powerful approach in a monolith project, when you don't necessarily have a use case for React / Vue / Angular.

I'm so glad to feel your excitement, too! komponent was a big inspiration for us wanting to bring this pattern to Rails.

@domchristie
Copy link

@domchristie domchristie commented Jun 14, 2019

I'm also excited about this, and love how elegant the solution is!

Just wondered how this might work with Russian doll caching? In particular would the caching system need to be updated in order to invalidate when the internals of TestComponent change (or an associated template file)?

@rip747
Copy link

@rip747 rip747 commented Jun 14, 2019

These feel exactly like ViewModels from ASP.Net 😍😍😍 Look to their design for inspiration.

@rizwanreza
Copy link
Contributor

@rizwanreza rizwanreza commented Jun 14, 2019

Not the same thing, but this reminds me of components back in the Rails 1.x days: https://github.com/rails/rails/blob/1-2-stable/actionpack/lib/action_controller/components.rb :)

@joelhawksley joelhawksley deleted the joelhawksley:actionview-component branch Jun 14, 2019
@sunnyrjuneja
Copy link

@sunnyrjuneja sunnyrjuneja commented Jun 17, 2019

quick question, what's the release target for this? 6.0 or 6.1?

@kaspth
Copy link
Member

@kaspth kaspth commented Jun 17, 2019

Rails 6 has shipped a release candidate already, we won't start that over just to add this. So this is 6.1. Please note: this, as is, adds support for rendering objects that respond to render_in — it doesn't add ActionView::Component, that's another PR.

tobyprivett pushed a commit to tobyprivett/view-component that referenced this pull request Jun 18, 2019
- Note that Rails 6.1 has support for `render_in` but doesn't actually add
ActionView::Component

(confused me for a while and explained by
rails/rails#36388 (comment))
@joshleblanc
Copy link
Contributor

@joshleblanc joshleblanc commented Jul 16, 2019

Question re: sidecar templates.

If you're already using sidecar templates, it would make sense to have a sidecar scss file. If such were the case, would it make sense to make the scss locally scoped to the component?

I'm thinking of svelte here, where a component has it's own script/css/html and it's all locally scoped. Except in this case, they would be separate files.

So styling for one component could have

div {
  padding 4px;
}

and it wouldn't affect another component's div.

joelhawksley added a commit to joelhawksley/rails that referenced this pull request Dec 9, 2019
In rails#36388,
we supported passing objects that `respond_to` `render_in`
to `render`, but _only_ in views.

This change does the same for controllers, as Rails
generally gives the expectation that `render` behaves
the same in both contexts.

Co-authored-by: Aaron Patterson <tenderlove@github.com>
joelhawksley added a commit to joelhawksley/rails that referenced this pull request Dec 9, 2019
In rails#36388,
we supported passing objects that `respond_to` `render_in`
to `render`, but _only_ in views.

This change does the same for controllers, as Rails
generally gives the expectation that `render` behaves
the same in both contexts.

Co-authored-by: Aaron Patterson <tenderlove@github.com>
@joelhawksley joelhawksley changed the title Introduce support for ActionView::Component Introduce support for 3rd-party component frameworks Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet