RFC: Attribute methods

[joelmoss]

First of all, really starting to enjoy Phlex - I think you may be onto the something here 👍

This is an idea that was borne out of my desire (need?) to automatically modify certain attributes of any HTML tag. Primarily, I want to be able to "rewrite" class names to support CSS modules. But this could be used for any attribute with simple and quite advanced uses.

Define an #[name]_attribute method in your view, and then any attribute in that view will be processed through this method.

So as a first trivial example, I want to be able to pass an array of class names instead of a string (yes I know I can use the classes or token helpers, but I want to save keystrokes - I don't have many left 😜).

class HeaderView < Phlex::View
  # Returns <div class="some class names"></div>
  def template
    div class: ['some', 'class', 'names']
  end

  def class_attribute(value)
    value.join ' '
  end
end

The class_attribute method receives the value given in template (['some', 'class', 'names']), and returns the result of joining the array. But of course, it could do absolutely anything.

Any method that begins with an attribute name and ends with _attribute, can be used to "massage" that attributes value. not convinced on the naming here, but it's a start.

There is also some scope here to specify which tags an attribute method should be applied to, perhaps like this:

  def class_attribute(value, tag)
    value.join ' ' if tag == :div
  end

Anyway, this is just an idea, but also a simple one that would need a very small change to Phlex::View#_build_atttibutes:

# ...
  if respond_to?(:"#{k}_attribute")
    v = send :"#{k}_attribute", v
  end
# ...

Thoughts?

[joelmoss]

After a little testing, I just noticed that my change to _build_attributes won't work as is, because attributes are cached :( So testing other implementations...

[joeldrapper]

Hey Joel, thanks for writing this up. I can see how this could be useful, but also I have thoughts:

First, we're only saving 2 keystrokes in your example. But you can save 8 keystrokes by using a String instead:

div class: ['some', 'class', 'names'] # 37
div **classes('some', 'class', 'names') # 39
div class: 'some class names' # 29

You also save memory allocations and operations:

div class: ['some', 'class', 'names'] # 1 Array, 3 + 1 Strings and a join
div **classes('some', 'class', 'names') # 1 Hash, 1 Array, 3 + 1 Strings and a join
div class: 'some class names' # 1 String and no join

I do quite like the core concept and I can see how it could be used for other things like the CSS modules example you mentioned. But I think it's going to be very difficult to make it fast. For example, calling respond_to? for every keyword argument costs about 3% on our current benchmark which has very few attributes.

We can already override the behaviour of specific attributes on specific tags. For example after #230, we'll be able to override the button tag on a specific view to have it mix in some default classes.

def button(**attributes, &)
  super(**mix(attributes, class: "bg-blue text-white"), &)
end

I wonder if it wouldn't be better to solve the CSS modules problem with a helper. Perhaps a classes override. 🤔

[joelmoss]

The mix method would also only be useful for specific methods. In this case, I'd want it to apply to all tags.

I like the mix DSL, although I would have expected to simply be able to override any of the tag methods in a view. Why us mix needed?

A classes override would solve this, but would also only work for classes.

Another use case for this could be as away to provide additional functionality for a tag, similar to rails helpers like link_to, button_to, etc.

[joeldrapper]

I like the mix DSL, although I would have expected to simply be able to override any of the tag methods in a view. Why us mix needed?

Oh, you can. But mix allows you to mix your tags with other tags. mix({class: "a"}, class: "b") produces { class: "a b" }.

[joelmoss]

Could we perhaps have a "parent" method that all tag methods override? Then I could override that "parent" method and the new mix DSL in it. That way I wouldn't have to override every tag method.

[joelmoss]

OK, so I've come up with an alternative which is little more performant, and also generalises it somewhat...

def resolve_attributes(attributes)
  attributes.tap do |attrs|
    attrs[:class] = "#{attrs[:class]}123abc" if attributes.key?(:class)
  end
end

So instead of defining a method for each attribute name, just define one method that will catch all attributes. It receives all tag attributes, and is expected to return the attributes, allowing you to modify them in any way you want.

There doesn't seem to be any performance hit. However, I did notice that once you start working with strings, that is when performance drops. But that occurs everywhere, and is really a userland issue.

The PR is updated accordingly.

[joeldrapper]

What I like about this is it could be used by the compiler too. 🤔

[joelmoss]

Then I like it even more! 😁

[joelmoss]

The more I think about this, the more I think it would be better to simply make the #_attributes method part of the public API (rename to #attributes), then for my use case I would simply override it. Not really sure we need anything extra.

Stupid simple! ✨

[joeldrapper]

I’m inclined to agree with you. Not sure if attributes is the right name though. People may want to use that as an accessor. We should make it public, but maybe rename it to process_attributes or something like that.

[joelmoss]

process_attributes or resolve_attributes both sound good to me. Your call