Allow SGML attributes to accept objects which respond to to_h

[stevegeek]

I think it would be nice to be able to use things like instances of Data for your tag attributes, eg

div(data: my_data_object)

Currently this raises:

  unexpected Phlex::ArgumentError: Invalid attribute value for data: #<data attr="test">.

Here is one possible solution, simply check if value responds to #to_h

[stevegeek]

Should add Im not sure what the exact implications are of allowing anytihing that responds to to_h , the solution could also be more specific and include a case for Data, Struct etc ?

[joeldrapper]

This sounds reasonable to me. I think you could get same effect by putting ** before the value, but I don’t see why we couldn’t support to_h-ables out of the box.

[joeldrapper]

Oh, I think the reason we’ve avoided this so far is due to difficulties caching. It would force us to do an un-cacheable step to prepare the cache key.

[stevegeek]

Ah right. Yes currently I do data: {**my_data} , just liked idea of not having the overhead

[joeldrapper]

For us to allow regular to_h-able objects, we would need to either iterate over each key of the attributes looking for to_h-ables and converting them in advance of the cache check (slow), or trust that your object has implemented hashing correctly and is safe to store in the cache for a long time (potentially quite dangerous).

[joeldrapper]

Perhaps we trust your object’s .hash method, but then when we verify with .eql?, we verify against the Hash from .to_h again, not against the original object. This would reduce the safety concerns and would only be a small performance hit.

[ismasan]

Just chipping in to say that I'd also find this useful. I have builder objects that can then pass their properties as DOM element attributes to Phlex, such as div(class: 'foo', data: builder).

Re. the small performance hit. With the new Phlex compiler:

1). Would the perf gains from the compiler offset this perf. hit sufficiently to make it less important?
2). Could the perf. hit of allowing #to_h interfaces be optimised somehow in the compile step? (I'm guessing probably not since element attributes are passed at runtime)

[joeldrapper]

I will take another look at this soon as I would like to support it. I don’t think there’s any way for the compiler to improve performance here as the compiler will fall back to dynamic attribute generation anyway if the attributes contain non-literals. It will only be able to compile attributes that are entirely literals. It’s possible we could split the literal and non literal parts, but that’s tricky because I believe the order of HTML attributes is significant.

Performance is fine if we can cache the input. The thing I’m worried about is if we’re given an object that does not correctly implement eql?, we may end up rendering one user’s data for another user since the cache is shared.

We could theoretically implement it in a way that it calls to_h each time, and then uses the hash of the Hash instead of the custom object. But the problem with this is we would need to walk the attributes Hash in all cases to see if it contained any custom objects — even in cases where we found that we’d already cached those attributes.

[stevegeek]

Could there be an optional module that one can include in their custom data classes that implements a "Phlex cache safe" set of methods?

Eg

class ViewData < Literal::Data
  include Phlex::Cacheable
end

Not sure if this makes sense at all, as don't know if the hash generation can be generalised like this