Resolve attribute resolution!

[davepeck]

We're much closer now. This issue asks us to sit back and consider the final spec we'd like to support for attribute resolution and merging, and then to document and implement it.

[ianjosephwilson]

This is a restart of #71 . So far we resolved the bare attribute issue, allowed for duplicate attributes to exist in the "middle" and made the attributes themselves much more well defined.

Considerations

• a. Two targets
  1. TComponent -- These attributes go to python code and can be full blown objects but some will go back into an element.
  2. TElement -- Essentially a name and value or just a value, all strings.
• b. The actual DOM has a well walked API
  • Many well tested ideas exist
  • Probably don't want to be too alien
  • It is the end all be all
• c. Deterministic output order matters
• d. Don't be too wasteful here; We're rendering.
• e. Be pythonic when we can and keep things simple.

Another Proposal: Special attributes indicate "special handling"

Attributes would still accumulate from left to right but the special attribute handling would occur in parallel and then be merged back in at the end of "resolution". Special attributes would indicate "special handling".

Collisions of the non-special attributes would trigger special handling and require merging (?? I'm not 100% sure that is the most intended consequence so maybe we'd have to try it out for a while) For example: <div class="bg-red" class={"width-100"}></div> is it "bg-red width-100" or clobbed to "width-100"? I think we'd trigger special handling here and merge these and it would be easier to handle and be consistent with what would happen if you had <div class="bg-red" class={"width-100"} classlist={['theme-resolve']}></div> resolve to "bg-red width-100 theme-resolve".

# NON-LITERAL HANDLING
classmask: dict[str,bool] = {'width-100': full_width} # the core class structure
classlist: Sequence[str|None] = ['theme-resolve'] # merged into `classmask` under the hood
dataset: dict[str, object] = dict(active=True) # the core data-* structure
ariaset: dict[str, bool|str|None] = dict(role="heading") # the core aria-* structure
styleset: dict[str, str|int|None] = dict(fontWeight="bold", border_width="1px") # the core style structure

# LITERAL HANDLING
#style: str|None (unpacked and merged if "special" handling required)
#class: str|None (unpacked and merged if "special" handling required)
#data-*: str|None (unpacked and merged if "special" handling required)
#aria-*: str|None (unpacked and merged if "special" handling required)
t"""
<div class="bg-red" classmask={classmask} classlist={classlist}></div>
<div data-title="Attribute Discussion" dataset={dataset}></div>
<div ariaset={ariaset}>Attribute Discussion</div>
<div styleset={styleset}>Attribute Discussion</div>
"""

Reasoning

This design provides some "sugar" and is structured enough for tools to know what to expect. Furthermore small mistakes will most likely be detected faster either at runtime or through tooling rather than being ignored silently. Finally these structures can be preserved and passed into a component for further adjustments before the final serialization as an element's properties.

Similar structures have strong parallels in the existing browser DOM implementations via dataset, classList, and style(as a property). This should allow for developers with mixed backgrounds to more quickly understand their intended usage. We also lay the framework for future innovations to be more easily driven by these parallels by staying aligned with the actual consumer of the "HTML" tdom will make: browsers.

By making these opt-in people can just ignore them and do whatever they want with little interference and tdom should still keep spitting out HTML.

First steps

I think at this point its easier to just whip up a proof of concept and then try to arrange actual code and see if this api "works" in the real world.

Precedence

• dataset - access data attributes from js on an object (ie. "structured"), major browser support
• classList - access class values via a list from js (ie. "structured"), major browser support
• className - instead of class to avoid reserved word, a problem we were going to have
• the style property of elements is also proxied into a more structured CSSStyleProperties , allowing getting/setting styles
• there are a plethora of tag builders that treat literal classes and attributes as a "pile" that is then merged with their non-literal counterpart, usually meant to override a default.

Can the value be cleared?

classmask can set a class name to False to clear it (omit class from final style rendering)
dataset can set a property to False or None to clear it (omit attribute)
ariaset can set a property to None to clear it (omit attribute)
styleset can set a property to None to clear it (omit style from final style rendering)

Styles?

There is a lot going on here and I'm not privy to a lot of the specifics here. I know units is a big issue when working with bare numbers, ie. 'px', '%', etc. I'm assuming there are probably a lot of weird oddities like that. I'm not even sure if unpacking the rules is consistent so we'll need to research that but I "think" its possible.

Performance?

Just for now and in the future when I say performance I usually mean "how to not make this so slow no one would ever use it ever".

Only trigger "special" handling if one of the dynamic keys are given or there is a merger of style or class. Ie. a static class and a dynamic class. Otherwise style and class go straight through untouched.

Users can improve performance by using spread attributes sparingly and we can... write that performance optimization eventually to make that actually true.

We might need to scan the attributes first and come back in certain situations to determine what to do. That seems like it might be the fastest but we'd have to see what shook out to work the best maybe a "for now" and a "for later" situation?

[pauleveritt]

Cool writeup, thanks. Regarding the point that the browser is actual consumer, I agree that is correct, with the qualifier of "ultimately."

In my writeup about paths, I had kind of hoped we'd have a place to stash system data on a node. Stuff important during the pipeline, but stripped out during rendering. (Such as path info.) I realize this is premature solution-icing: we might find other ways to do it vs. storing data on nodes.

[davepeck]

Thanks, Ian, this is a great writeup and lots to consider.

I need to mull it over, but I suspect I'm going to feel "perfect is the enemy of the good" about splitting special handling for a single attribute into custom names, aka there's only class, not classmask and classlist.

[ianjosephwilson]

I agree we should mull it over, there are a LOT of subtle issues here, but I also think it would be helpful to compile a few small built-out examples and sort of run through a few variations to see what actually makes sense when you return and read it. At this point things are so feature small we could probably even compare implementations.

[ianjosephwilson]

Just to be clear if you want the class within a component's call list there isn't really a way right? I mean you can pull it out of **kwargs but you can't specify it the function signature.

def Form(class="", **kwargs):
    # SyntaxError: invalid syntax ^
    return t'<form class={class}></form>'

You'd have to re-route it through a different name. Looks like jsx is using className.

def Form(className="", **kwargs):
    return t'<form class={className}></form>'

Or sometimes I've used a situational name like this:

def Form(extraFormClasses=(), **kwargs):
    return t'<form class="form-default" class={extraFormClasses}>...</form>'

<{Form} extraFormClasses={['theme-holidays']} />

Some builders use class_ to be pythonic but that always seemed kind of gross to me.

I think we are suffering from a serious lack of use-cases right now. Looking through my code I think I would probably use the list usage the most although maybe that is because I wasn't using a helper but a lot of times it isn't a boolean driving a single class but a mix of booleans driving a mix of classes like this (this is more about class building and not about modals because this is just a small cutout piece):

                let classes = '.w-100.mw7.center';
                if (config.overlayBackgroundClass) {
                    classes += '.' + config.overlayBackgroundClass;
                }
                if (config.withFullScreenHeight) {
                    classes += '.h-100';
                }
                if (config.withMargin) {
                    classes += '.mt2.mb1';
                }
                if (config.withPadding) {
                    classes += '.pa2.pb1';
                }
                return m('div' + classes, {
                    style: {
                        // Only show scroll bar if content doesn't fit.
                        overflowY: 'auto',
                        maxHeight: '100%',
                    }
                }, children);

def SimpleModalContent(children: Node|None=None, config: ModalConfig|None = None):
    if not config:
        config = ModalConfig()
    classes = []
    if config.overlay_bg_class:
        classes.append(config.overlay_bg_class)
    if config.with_full_screen_height:
        classes.append('h-100')
    if config.with_margin:
        classes.extend(['mt2', 'mb1'])
    if config.with_padding:
        classes.append(['pa2', 'pb1'])
    style = {'overflowY': 'auto',  'maxHeight': '100%' }
    return t'''<div class="w-100 mw7 center" class={classes} style={style}>{children}</div>'''

So the most common use case I see is something like this:

common_t = t"<div class="theme-default bg-red" class={classlist_to_be_merged_with_literal}></div>"

The biggest problem I have with using ONLY a plain list is that there is no way to turn off a class you set in a literal so if it might need to be set off you HAVE to remove it from the html literal class in the code itself. Maybe that's a good thing? We just don't have enough expected usage to go on right now. So I guess this is where I'm stuck. The classnames helper makes the type out to be this (which I think we both agree is a little overboard):

# option 1: (sort of what we have now) swiss army chain saw ( I think it is actually worse then this in that it is recursive)
className: str | None | False | Sequence[str | None | False | dict[str,bool | None]] = "theme-holiday"

So maybe some improvements...

# option 2: split out the dict but still keep it in the same saw
className: str | None | False | Sequence[str | None | False] | dict[str,bool | None] = "theme-holiday"
# option 3: split out the toggler completely
className: str | None | False | Sequence[str | None | False] = "theme-holiday"
classmask: dict[str,bool | None] = {"theme-holiday": False}

I guess option 2 is the least crazy option right now but this seems like the type of thing we just throw in and then TDOM supports for forever... so maybe we should revisit it earlier than later. This would allow this:

def get_body_container():
    toggle_classes = {'theme-default': False}
    add_classes = ['theme-holiday']
    def DefaultContainer(add_classes: list|None=None, toggle_classes:dict|None=None):
        return t"""<div class="theme-default bg-red" class={add_classes} class={toggle_classes}></div>"""
    
    return t"<{DefaultContainer} add_classes={add_classes} toggle_classes={toggle_classes}>...</{DefaultContainer}>"

[ianjosephwilson]

Alternatively the last block could be combined like this although as I was saying before usually there a big list of stuff and splitting a conditional like this into two parts would be confusing to come back to:

def get_body_container():
    toggle_classes = {'theme-default': not is_holiday, 'theme-holiday': is_holiday}
    def DefaultContainer(add_classes: list|None=None, toggle_classes:dict|None=None):
        return t"""<div class="theme-default bg-red" class={add_classes} class={toggle_classes}></div>"""
    
    return t"<{DefaultContainer} toggle_classes={toggle_classes}>...</{DefaultContainer}>"

And final final,we drop toggling completely and just do lists/strs:

def get_body_container():
    add_classes =['theme-holiday']
    def DefaultContainer(add_classes: list|None=None):
        classes = add_classes[:] if add_classes else []
        if [cn for cn in classes if not cn.startswith('theme-')]:
            classes.append('theme-default')
        # bg-red cannot be removed
        return t"""<div class="bg-red" class={classes}></div>"""
    
    return t"<{DefaultContainer} toggle_classes={toggle_classes}>...</{DefaultContainer}>"

Just some more things to consider.

[ianjosephwilson]

I made a much more digestible (less experimental) set of changes that I think turned out pretty well in #97 . There are maybe a few things we would want more strict or less strict but it is pretty close to what is there now ... but with merging. It washes most of the weird edge cases too which is nice.