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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming to ViewComponent, Syntax Changes #206

Closed
joelhawksley opened this issue Feb 12, 2020 · 19 comments
Closed

Renaming to ViewComponent, Syntax Changes #206

joelhawksley opened this issue Feb 12, 2020 · 19 comments
Labels
help wanted

Comments

@joelhawksley
Copy link
Contributor

@joelhawksley joelhawksley commented Feb 12, 2020

馃憢 Hi there folks!

After some conversations with the Rails team, we鈥檝e decided to make the following changes to ActionView::Component:

  • Rename the gem to ViewComponent
  • Revert to the original render(MyComponent.new) syntax

We hope that these changes will better communicate the relationship between this project and Rails. To that end, this library will not be upstreamed into Rails 6.1, but will instead have integrated support via the changes introduced in rails/rails#36388. We鈥檒l be eager to provide this library as a first-class option for Rails developers.

GitHub continues to see value in the project, and I鈥檓 happy to share that I will now be working it full-time 馃帄. We鈥檙e excited for the future of this library and can鈥檛 wait to see where it takes us.

Renaming to ViewComponent

We鈥檒l be renaming the library to ViewComponent.

As it stands, the name ActionView::Component implies that this library is already a part of Rails, when it is not. While we have strived to integrate with Rails, both technically and conceptually, as a non-Rails codebase this name is ultimately improper.

Syntax changes

Starting with the next release, we鈥檒l be reverting to supporting the original render(MyComponent.new) syntax, per discussion on this PR (rails/rails#38362) and in private with the Rails team. We鈥檒l mark the other syntax options as deprecated, and remove them in v2.0.0.

Once these changes are complete, we鈥檒l be able to disable the monkey patches we have in place when using Rails 6.1+. We have no plans to deprecate support for previous versions of Rails.

Thanks

I want to reiterate how thankful we are for the support this project has received from the community. We鈥檝e been blown away by the contributions from all of you, both in quantity and quality 鉂わ笍

@jonspalmer
Copy link
Collaborator

@jonspalmer jonspalmer commented Feb 13, 2020

Great progress!

@rainerborene
Copy link
Collaborator

@rainerborene rainerborene commented Feb 14, 2020

Today I had a great chat with @joelhawksley about the future of actionview-component, well, now renamed to view-component. Before we get into that, let me share why I decided to use this gem in the first place.

About a year ago I was using a combination of Rails built-in helpers and active_decorators at the startup I'm working with, and it quickly became a huge mess over time. I couldn't recognize which method belongs to which view anymore (not to mention the lack of tests).

After migrating to actionview-component, something (bad) happened: all the design flaws of my application were exposed. Now I had the necessary information to refactor and write good code this time by isolating dependencies, and make view classes follow the single responsibility principle. I could test all those expensive pieces of crap code without sacrificing clarity, speed and most important: developer happiness.

I think the same thing will happen (or already did) to you. That is what actionview-component is all about: to help you design better applications using the tools already available on the Rails framework. No need to pull dozens of extra dependencies into your Gemfile like cells. Just to be clear, I have nothing against Trailblazer developers. They are doing a great work.

But if you look on this pull request rails/rails#36388, you'll realize that this library is entirely based on a single protocol called render_in, which means that any class that responds to this message can become a component. It can't be easier than that.

Said all that, what is the purpose of this gem? Correct me If I'm wrong @joelhawksley.

  • Facilitate working with templates by creating a minimal layer around this new actionview protocol. So, you don't have to write boring boilerplate code every time. Easy delegation to the current view_context is one of the things that we've talked about.
  • Provide a great developer experience out of the box, meaning generators and test helpers.
  • Be completely agnostic about validations, caching and directory structure. If you want to use validations, fine. If you don't, just change one line of code. It's up to you.
  • Allow component classes to render inline markup using the good old-fashioned Rails helpers like link_to, form_for and so on.
  • Depend solely on actionview alone (not even activemodel).
  • Make extensible and pluggable like most things in Rails internals.

I'm looking forward to the brighter future of view-component gem. 馃檹

@andrewmcodes
Copy link
Collaborator

@andrewmcodes andrewmcodes commented Feb 14, 2020

Sad this will no longer be upstreamed into Rails but excited for the future of this project 馃帀 Thanks for all of your work on this @joelhawksley.

@xdmx
Copy link

@xdmx xdmx commented Feb 15, 2020

To that end, this library will not be upstreamed into Rails 6.1,

I'm a bit sad about this. What's the reason behind it? Not ready yet for prime time or is there no interest from the Rails Core team to have it provided by Rails itself? Will it be in the future (6.2?) or is it a final decision?

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Feb 18, 2020

@xmdx I'm a bit sad too, but I'm also conscious of how much this library is still in flux. I don't think it was going to be mature enough to be upstreamed this year.

Given more time, a stable API, and support from / usage by the community, I could see our chances being better for the next release cycle.

@jaredcwhite
Copy link
Contributor

@jaredcwhite jaredcwhite commented Feb 19, 2020

It's a bummer this won't be builtin to the Rails framework, but I'm glad the underlying Rails view layer now supports rendering objects of any kind, whether ViewComponents or otherwise. Seems like a major step forward for the ecosystem.

duncanjbrown added a commit to DFE-Digital/apply-for-teacher-training that referenced this issue Mar 10, 2020
Following a change to the docs at
https://github.com/github/view_component#installation, part of a change
to rename this gem to view_component (because it's not going to be in
Rails 6.1).

github/view_component#206
duncanjbrown added a commit to DFE-Digital/apply-for-teacher-training that referenced this issue Mar 10, 2020
Following a change to the docs at
https://github.com/github/view_component#installation and
https://github.com/github/view_component#setting-up-rspec, part of a
change to rename this gem to view_component (because it's not going to
be in Rails 6.1).

github/view_component#206
davidgisbey pushed a commit to DFE-Digital/apply-for-teacher-training that referenced this issue Mar 12, 2020
Following a change to the docs at
https://github.com/github/view_component#installation and
https://github.com/github/view_component#setting-up-rspec, part of a
change to rename this gem to view_component (because it's not going to
be in Rails 6.1).

github/view_component#206
@danturu
Copy link

@danturu danturu commented Mar 14, 2020

@joelhawksley do you plan to add something like:

class Button < ViewComponent::Base
  field :text # ideas for naming 'prop', 'property', 'attribute'
  field :size, default: 'lg'
end

It's much simpler than:

class Button < ViewComponent::Base
  def initialize(text:, size: 'lg')
   @text = text
   @size = size
  end

  private
   
  attr_reader :text, :size
end

@rainerborene
Copy link
Collaborator

@rainerborene rainerborene commented Mar 14, 2020

@danturu
Copy link

@danturu danturu commented Mar 14, 2020

@rainerborene I could but if it fits the plans it should be available out of the box without an additional gem. Also this or different DSL will give the developers the explicit way of how the component should be defined.

@joelhawksley joelhawksley added the help wanted label Mar 16, 2020
@joelhawksley joelhawksley mentioned this issue Mar 20, 2020
@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Mar 23, 2020

@rosendi I'm hesitant to add a DSL here, as it doesn't seem consistent with Rails.

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Mar 23, 2020

Closed by #268

@AlecRust
Copy link

@AlecRust AlecRust commented Mar 23, 2020

@joelhawksley are validations removed all together? README/this issue implies it can still be enabled somehow?

Edit: Ah, judging by some internal code I found include ActiveModel::Validations needs to be included in the component.

@jaredcwhite
Copy link
Contributor

@jaredcwhite jaredcwhite commented Mar 23, 2020

@AlecRust In my application_component.rb base class, I added:

  include ActiveModel::Validations

  def before_render_check
    validate!
  end

and that restored the validation functionality.

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Mar 23, 2020

@AlecRust - @jaredcwhite is correct.

The test suite contains many example components, but this is the one that will be of interest to you: https://github.com/github/view_component/blob/master/test/app/components/validations_component.rb

@ultrawebmarketing
Copy link
Contributor

@ultrawebmarketing ultrawebmarketing commented Mar 25, 2020

very sad that it won't be upstream, though i get it why. what i don't understand was the decision to remove the validation by default. personally i thought that was one of the strongest features of using this project. if i may ask, what was the reason for that and is there any change to reconsider?

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Mar 25, 2020

@ultrawebmarketing the main reason we removed validations was that we found the pattern to be dangerous. Introducing a path to raise exceptions to the view layer ended up causing runtime errors in production.

Instead, we've moved to coercing our component's parameters to one of several known values, using a helper called fetch_or_fallback:

# GitHub::FetchOrFallbackHelper
# A little helper to enable graceful fallbacks
#
# Use this helper to quietly ensure a value is
# one that you expect:
#
# allowed_values  - allowed options for *value*
# given_value     - input being coerced
# fallback        - returned if *given_value* is not included in *allowed_values*
#
# fetch_or_fallback([1,2,3], 5, 2) => 2
# fetch_or_fallback([1,2,3], 1, 2) => 1
# fetch_or_fallback([1,2,3], nil, 2) => 2
module GitHub
  module FetchOrFallbackHelper
    InvalidValueError = Class.new(StandardError)

    def fetch_or_fallback(allowed_values, given_value, fallback)
      if allowed_values.include?(given_value)
        given_value
      else
        if Rails.development?
          raise InvalidValueError, <<~MSG
            fetch_or_fallback was called with an invalid value.

            Expected one of: #{allowed_values.inspect}
            Got: #{given_value.inspect}

            This will not raise in production, but will instead fallback to: #{fallback.inspect}
          MSG
        end

        fallback
      end
    end
  end
end

Which we use in a component like:

class IconComponent < ApplicationComponent
  ICON_COLOR_DEFAULT = :default
  ICON_COLOR_MAPPINGS = {
    gray: "icon-gray",
    white: "icon-white",
    default: "icon-answered"
  }
  ICON_COLOR_OPTIONS = [ICON_COLOR_DEFAULT, *ICON_COLOR_MAPPINGS.keys]

  def initialize(icon_color: ICON_COLOR_DEFAULT)
    @icon_color = fetch_or_fallback(ICON_COLOR_OPTIONS, icon_color, ICON_COLOR_DEFAULT)
  end
end

This allows us to have the safety guarantees of validations without exposing ourselves to the risk of exceptions.

@ultrawebmarketing
Copy link
Contributor

@ultrawebmarketing ultrawebmarketing commented Mar 31, 2020

That is really cool, perhaps consider putting that in core?

@danturu
Copy link

@danturu danturu commented Apr 22, 2020

@rosendi I'm hesitant to add a DSL here, as it doesn't seem consistent with Rails.

Why? ActiveModel has the same DSL, Mongo ruby-wrapper has field, other component-based frameworks such as Cells

I'm asking again because after a half year working with rails components there is too many boilerplate code and the definition could look much cleaner.

With the Attribute-DSL we can do coercions, fallback validation, etc.

class Button < ViewComponent::Base
  property :text
  property :size, type: String, default: 'lg', "enum": ["sm", "gt", "md"] <-- will falback to lg unless match
end

Also the comment property could be added in addition for generating storybooks (like React-Intl does):

class Button < ViewComponent::Base
  property :text 
  property :size, type: String, comment: 'The button size' 
end

P.S. I wanted to stay align with the official best-practices so I still don't use Attribute-DSL.

@joelhawksley
Copy link
Contributor Author

@joelhawksley joelhawksley commented Apr 22, 2020

@rosendi interesting. Might you open an issue so we can discuss this idea by itself?

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

No branches or pull requests

9 participants