Unify vdom interface

[rmorshea]

closes: #755

Checklist

Please update this checklist as you complete each item:

• Tests have been included for all bug fixes or added functionality.
• The changelog.rst has been updated with any significant changes.
• GitHub Issues which may be closed by this Pull Request have been linked.

[Archmonger]

This should probably be the final PR in the 1.0 release, since it's a fairly significant overhaul.

[Archmonger]

I did some more thinking, id would be the the single property that makes this implementation messy. We don't want to override the built-in python id function, so do we sufix it with a _?

[Archmonger]

Discussed this via call with Ryan.

All suffixed underscores should be stripped out.

[Archmonger]

We might want to have another discussion as to whether we should move forward with this.

Given both options have positives/negatives, I'm still undecided.

---

Using Dict

Pros

• Feels more like ReactJS props (since in React/JSX props are "at the top" within each HTML tag)
• Does not have an issue with Python keywords`

Cons

• Might feel weird to some
• Currently requires using camelCase within the dict, which is not pythonic.

Using Kwargs

Pros

• Feels more pythonic
• Can do long-form type hints for all available properties

Cons

• Does not have good visual similarity to ReactJS
• Switching to this will require developer migration efforts
• Will feel weird to use for data-* and aria-* attributes
• Makes writing conditional parameters more awkward, since they'll need to be an unpacked dict.

[rmorshea]

To clarify, only the following are keywords that would require you to add a trailing _:

False      await      else       import     pass
None       break      except     in         raise
True       class      finally    is         return
and        continue   for        lambda     try
as         def        from       nonlocal   while
assert     del        global     not        with
async      elif       if         or         yield

Functions like id, map, or type which are available in the global namespace, but are not keywords do not conflict with kwargs passed to VDOM constructors.

Personally I don't think this is a large enough issue to block these changes.

[Archmonger]

Do we want the conversion script to be able to handle props in a dict variable?

MODAL_CONTAINER = {
    "id": "modal-container",
    "className": "modal fade",
    "tabIndex": "-1",
    "aria-hidden": "true",
}

@component
def modal():
    return div(
        MODAL_CONTAINER,
        ....
    )

[rmorshea]

Unfortunately, handling that is complicated. Looking at the div call in isolation, there's no way to tell if it's a child element or an attribute dictionary. You'd have to work backwards and determine the type of the variable. It's probably possible, but a lot more challenging than just converting dict literals to kwargs.

[rmorshea]

@Archmonger can you try running the script against some of your code? It fairs pretty well against IDOM's source but that's a fairly small sample size. You should be able to copy and paste in an environment where IDOM is installed and do the following:

python fix_vdom_constructor_usage.py [patterns]

Where [patterns] is any number of glob pattern for the files you want to modify. For example:

python fix_vdom_constructor_usage.py tests/**/*.py tests/*.py

[Archmonger]

Looks like there was a incomplete merge commit at this LOC

EDIT: Perhaps not, maybe that's an actual docstring? The indentation makes it look fairly comical though.

Probably better represented as this:

dedent(
   f"{actual}\n"
   "▲   actual   ▲\n"
   "▼  expected  ▼\n"
   f"{expected}\n"
)

[Archmonger]

Running the output on conreq@app_store then did a visual inspection of all the deltas.

It only messed in in one situation, where it forgot to leave a preceding space on an inline ... if .... else ... statement on this LOC.

It essentially resulted in x if y elsehtml.button(...)

---

I think this interface looks way more pythonic, so I dig it.

[rmorshea]

Outside of the updating narrative docs this change is pretty much complete. Usages of vdom() where a child is a dictionary without a tagName key produce a warning explaining the API change. IDOM will also distribute a CLI utility for upgrading to the new API (e.x. idom update-html-usages [PATTERNS]).

[Archmonger]

We probably want that CLI to be a bit more user friendly.

idom update-html-usages [DIR_OR_FILE]

If we inspect the path to be a dir, then we should automatically create the glob **/*.py

Otherwise, only format one file.

[rmorshea]

Yup. Just made that change.

[Archmonger]

Will review when tests pass

[rmorshea]

There's some minor kinks in the tests to work out, but this is ready for review.

[Archmonger]

Not many nitpicks on this one.

Can we potentially sneak in a revert to the use_state type hints on this PR? It's fairly painful to to use_state right now.

[Archmonger]

When we cut a 1.0.0 release of IDOM, I will sneak it into the 3.0.0 release of Django IDOM

[Archmonger]

On a different note, in the future releases of IDOM, I recommend we switch to semantic versioning rather than feature-set versioning.

We will need to rename our GitHub Projects milestones as such.

[rmorshea]

Completely forgot to update docs/changelog will create a followup PR