Support validations for component attributes and elements

[msalzburg]

We used the components gem in one of our recent web projects and really enjoyed it :)

To ensure the correct usage of our components we decided to "integrate" rails validations. Now invalid components will raise validations errors which can be detected by feature specs.

This behaviour is currently enabled for all environments, probably it should be configurable for each environment.

Obviously this is still work in progress - I am just wondering if you would consider merging it?

[jensljungblad]

Good to hear you've found it enjoyable :)

Yes I think validations are an excellent addition. I had a required option in the past that could be passed to attributes, but I think leveraging Rails validations is a better idea, since you can validate so much more than just presence.

As for how to do it.. Leaving it up to the user to implement in their ApplicationComponent initially sounded pretty good to me. It could have been documented similar to what you have in this PR and looked something like:

class ApplicationComponent < Components::Component
  include ActiveModel::Validations

  def initialize
    super
    validate!
  end

However, this only allows for validation of component attributes, and not of nested element attributes.

So that leaves either a) making it possible for the user to easily extend not only the base component, but the base element or b) include validations by default on Components::Element.

Have you thought of any issues with always including validations? There is already a dependency on Rails, so I wouldn't mind leveraging parts of ActiveModel, if it's for a good reason.

[imathis]

I've been working on implementing Components as well. I really like what you're doing here, but I too wish elements could extend other element classes. I was thinking about making a change to the elements class definition as follows:

def self.element(name, multiple: false, klass: Element, &config)
  plural_name = name.to_s.pluralize.to_sym if multiple

  elements[name] = {
    multiple: plural_name || false, class: Class.new(klass, &config)
  }
…

Which would allow me to create an element from a base class:

class MyComponent < Components::Component
  element :header, klass: MyHeaderElement
end

[jensljungblad]

@imathis Seems like a nice solution! This is mostly to avoid having to define methods etc in the block? Or to share methods among many elements?

[imathis]

@jensljungblad Having to treat each component and its elements as new creations doesn't work with my use case as well, so I'm working on making it possible to extend Elements and Components, allowing them to inherit elements, attributes, and methods, as well as allowing elements to render from templates. I'm not sure if that's something you're interested in or not, but I'd be happy to show you what I'm working on once I have it up and running.

[msalzburg]

Have you thought of any issues with always including validations? There is already a dependency on Rails, so I wouldn't mind leveraging parts of ActiveModel, if it's for a good reason.

@jensljungblad I like your thinking! Including it by default is no big pain and it makes the experience way better.

[msalzburg]

@jensljungblad I updated the branch accordingly. It should be mergeable now?

[msalzburg]

@imathis since this PR focuses on general component validation and is potentially done, maybe we should open a separate branch/pr for your element enhancement?

[msalzburg]

Except of the "nested_block" naming issue all comments should be resolved. How should we proceed?

[msalzburg]

Everything fine?

[jensljungblad]

Do we want to support validating the presence of elements? Is that a use case you've come across? I suppose then that validation would have to be moved to the render stage, since the elements are added after initialization.

class CardComponent
  element :header
  element :body
  element :footer

  validates :body, presence: true
end

[msalzburg]

Element presence validation already works ;) But I also added some tests.

Guess this branch is finally mergeable and we will continue in #32

[jensljungblad]

@msalzburg Haha right, because we yield the block before we run validate!.. Excellent :)

[jensljungblad]

@msalzburg Thanks for the contribution, great work!