Add "value( v : object )" in AttrEngine

[davedawkins]

I can work around it with a specialization if I have to, but frameworks can do things like cache a reference to the JS value that was set (as Sutil does, and possibly Svelte). Having to stringify the value loses information that's useful for binding functionality.

[alfonsogarciacaro]

Yes, I was thinking about this. At the end I used string to avoid upcasting by mistake, but I guess we'll need the flexibility of obj. Are you thinking in some specific members or the general MakeAttr helper?

[davedawkins]

For now I was going to specialise value only. It's only a requirement for binding Frameworks and actually shouldn't be allowed through to the browser DOM. We have to rely on the implementation for that though

…

On Mon, 1 Mar 2021 at 13:51, Alfonso Garcia-Caro ***@***.***> wrote: Yes, I was thinking about this. At the end I used string to avoid upcasting by mistake, but I guess we'll need the flexibility of obj. Are you thinking in some specific members or the general MakeAttr helper? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACFV3NIJ6YNV3MFHK2ZVUDTBOLXTANCNFSM4YLNW76A> .

[davedawkins]

My workaround is to overload my derived AttrEngine as follows, and my immediate requirement is satisified

type SutilAttrEngine(helper) =
    inherit AttrEngine<NodeFactory>(helper)

    member _.disabled<'T> (value: IObservable<'T>) = bindAttrIn "disabled" value

    member _.value(value:obj)   = attr("value",value)
    member _.value(value:int)   = attr("value",value)
    member _.value(value:float) = attr("value",value)
    member _.value(value:bool)  = attr("value",value)

[davedawkins]

I think the engine is justified in converting everything to string (since HTML attributes are string based), but I would like to see it performed in such a way that we can capture the original (typed) value in the DSL implementation before this happens, and control how it is performed. For example, we currently have

        member _.foo( value : int ) = h.MakeAttr( "foo", Util.AsString(value) )

if it was more like this

        member _.foo( value : int ) = h.MakeAttr( "foo", value )

with overloads in the Helper class like this:

type AttrHelper<'Node> =
    abstract MakeAttr: key: string * value: string -> 'Node
    abstract MakeAttr: key: string * value: int -> 'Node
    abstract MakeAttr: key: string * value: float -> 'Node
    abstract MakeAttr: key: string * value: bool -> 'Node

then we have given the implementer complete control over how attrName:string * 'T is converted to 'Node

[alfonsogarciacaro]

Hi David! Again, thanks a lot for the discussion and the comments, I've thinking (and coding) through this. Probably not exactly what you wanted 😅 but I've made the following changes:

• I've removed the non-string overloads for value. I agree this is a special case and the engine shouldn't try to swallow the non-string arguments and leave this for the specific implementation.
• I've removed the float overloads when there was an alternative with ICssUnit for consistency with the cleanup made in the CssEngine.
• I've changed the AttrHelper interface to function arguments, as this is the recommended way to pass behaviour in F#. This way, if in the future we need to provide further customization we can do it with an optional argument to avoid a breaking change. Now I need to do the same for the other engines 😅

So the question is, why have I not added function arguments to convert int and float attributes? (Boolean attributes are already a special case because they have different syntax in HTML.) I actually did it, but I realized there were several conflicting cases so I was worried this could cause more confusion.

For example, baseFrequency has multiple overloads: int/float/int * int/float * float. With the extra configuration maybe you could intercept the cases with one argument, but for two arguments (horizontal and vertical frequencies), AttrEngine will already convert them to string and concat them with ", ", would that be expected? Also, the ICssUnit overloads usually have an int alternative, would it be ok to intercept int values while css units get automatically converted to string?

So that's basically the reason I refrained from adding extra arguments for making attributes from ints or floats. Besides the value attribute, would you still need to be able to intercept calls to, say, baseFrequency(float) or cx(int), even when missing baseFrequency(float, float) and cx(ICssUnit)?

[davedawkins]

Again, wow. Thank you. I will be checking this out.

I had actually convinced myself during discussion that value shouldn't be treated differently in the engine, but allow implementations to add any speciality they require (to any attribute).

I think though (I need to actually look at the code) that you've given all options here, so again, thank you.

Incredible response.