add class based component implementation

[rmorshea]

This pull request is provisional, and it may never be merged. Hopefully it can serve as a useful reference if this is asked about in the future.

What This Does

This PR allows you to define components using classes similarly to how you would in React. Python's __setattr__ magic method makes it a bit easier to implement though since you no longer need an update method. Instead the instance attributes can be assigned directly.

@idom.component
class Counter:
    def __init__(self, count: int = 0):
        self.initial_count = self.count = count

    def render(self):
        return idom.html.div(
            f"Count: {self.count}",
            idom.html.button({"onClick": self.reset}, "Reset"),
            idom.html.button({"onClick": self.increment}, "+"),
            idom.html.button({"onClick": self.decrement}, "-"),
        )

    def increment(self, event):
        self.count += 1

    def decrement(self, event):
        self.count -= 1

    def reset(self, event):
        self.count = self.initial_count

Limitations

This PR does not allow class-based components to incur side effects. We may leave that as an intentional limitation though in order to try and push people towards functional components because they encourage better patterns of development.

Disadvantages

1. Having two ways to do the same thing might be confusing. Do we introduce class-based or function-based components first?
2. Class-based components are less composable and can encourage anti-patterns - most new React libraries use hooks for this reason.
3. It will likely be strange for users to see that mutating attributes of their components will not trigger updates. For example, appending to a list won't trigger a re-render.
4. It probably isn't that big of a deal, but I suspect these class-based components might be less performant - users would have to understand that every attribute assignment has side effects they may not be aware of.

Advantages

• If used sparingly, class-based components could enable some useful imperative interfaces. This would probably be a bad use case, but in the Counter example above a parent component could increment the count by mutating its attributes.
• Class based components could serve as a useful introduction to the execution model. The idea being that we avoid scaring off new users, but intentionally limit class-based capabilities in order to push them towards learning/using function-based ones.

[Archmonger]

I'd suggest removing the decorators (@idom.component) from the development flow and instead automate calling this through inheritance.

Inheritance would better align with common pythonic styling.

# Calling this `BasicComponent` in case other component models exist in the future,
# `Component` would be the base class to `BasicComponent`
from idom import BasicComponent

class Counter(BasicComponent):
    ...

[Archmonger]

On a different note, I definitely think this is vastly more beginner friendly compared to the functional components.

If we wanted to flesh this out later, perhaps some of the hook functionality could be prototyped functions to allow for type hinting.

In order to figure out what IDOM has to offer, type hinting is much quicker than scrolling through docs.

[rmorshea]

I am somewhat worried about confusion over mutable attributes here. For example, you cannot simple append to a list and get a re-render, you need to re-assign the attribute.

I might actually make an update() method to resolve this. Then, instead of self.count += 1 you'd need to do self.update(count=self.count + 1). Not as clean, but hopefully clearer.

[Archmonger]

How about instead of update accepting args, it will fully re-render the component with all current class attributes.

We can just require this to be called rather than implicitly doing it via __setattr__

self.count += 1
self.update()

In the case that the user has a ton of attributes to update, this would feel a lot cleaner.

Plus, this follows the pattern of most Python ORMs.

[Archmonger]

Regarding composable parts, wouldn't it be possible to simply create any reusable functions outside the class?

For example, increment and decrement don't have to be created within the class object.

There isn't any overhead for using an external function, as it's accessed via reference by the interpreter.

[rmorshea]

So there are some differences in composability that become more evident as things get more complex.

Imagine that I want a simple component that counts down from some number and has a button to reset the count:

Using hooks that might be done as follows:

@idom.component
def CountDown():
    count, reset_count = use_count_down(5)
    return idom.html.div(
        idom.html.button({"onClick": lambda event: reset_count()}, "reset"),
        f"count: {count}",
    )

def use_count_down(initial_count):
    count, set_count = idom.hooks.use_state(initial_count)

    @idom.hooks.use_effect
    async def decrement():
        await asyncio.sleep(1)
        if count > 0:
            set_count(count - 1)

    return count, lambda: set_count(initial_count)

Assuming there were an effect method for class-based components, the same thing as a class might look like:

class CountDown(ClassComponent):

    def __init__(self, initial_count):
        self.initial_count = self.count = initial_count

    def render(self):
        return idom.html.div(
            idom.html.button({"onClick": self.reset_count}, "reset"),
            f"count: {self.count}",
        )

    async def effect(self):
        await asyncio.sleep(1)
        if self.count > 0:
            self.count -= 1
            self.update()

    def reset_count(self):
        self.count = self.initial_count
        self.update()

It's not clear to me how you could abstract something similar to the use_count_down hook from a class component without doing so in a contrived way. Additionally, something I just noticed in writing this up is that the logic of the effect and related state get spread out across different methods of the class.

[rmorshea]

Regardless though, I'm less worried about trying encourage good patterns of development, and more concerned with how to explain why we have two ways of doing the same thing. Even if the learning curve is steeper, in the long run, it may be beneficial to only have one thing you need to master.

[Archmonger]

use_effect Abstraction

In terms of abstracting to the use_count_down example, couldn't that be automated through wrapping the ClassComponent with a WrappedComponent as it passes through the IDOM component registration logic and configuring use_effect discreetly?

This is with the assumption that the limitation is that use_effect isn't getting called in the CBC example. If not then I'd need some more context.

---

Separating Methods within the Class

I personally think this is a benefit. Increases readability and allows for re-use of the reset_count logic within other parts of the component.

I'd actually argue this might be more composable than the functional example. In the CBC example, now reset_count(self) can be pushed out of the class if needed and re-used for several different components.

In fact, hook functions could now be made generic if needed. For example, effect(self) could be pushed out and re-used, if that situation comes across for more complex examples.

---

Encouraged Design Patterns

Although I did mention beginner friendliness, I would also like to mention that the functional example you posted is very difficult to read, also making it difficult to debug.

It only took one glance on the class based example to fully understand what is going on, while I had to re-read and mentally stack trace the functional example roughly three times to follow the same logical flow.

If a simple decrementing counter required me to constantly bounce my eyes up and down the code, it's telling that the functional syntax is not easily digestible.

---

One Design Pattern vs Two

I understand the concern, but I still think that even if two patterns exist, this issue gets resolved through proper documentation.

I hop between class based views and functional views within Django & DRF based on whichever seem to better fit the current situation.

One style is prioritized within the documentation, but they're both well documented.

[rmorshea]

So this last round of changes modifies the usage a fair bit with the main goal of helping the user avoid mutating their state.

This is done by:

1. isolating all state to a single state attribute
2. adding an idom.update function that operates on component instances

Both these changes bring this implementation more in-line with React's own class-based components.

Ultimately, these result in the following usage:

from typing import NamedTuple
import idom

class GateState(NamedTuple):
    left: bool
    right: bool

@idom.component
class AndGate:
    def __init__(self):
        self.state = GateState(False, False)

    def render(self):
        left, right = self.state
        return idom.html.div(
            idom.html.input({"type": "checkbox", "onClick": self.toggle_left}),
            idom.html.input({"type": "checkbox", "onClick": self.toggle_right}),
            idom.html.pre(f"{left} AND {right} = {left and right}"),
        )

    def toggle_left(self, event):
        left, right = self.state
        idom.update(self, GateState(not left, right))

    def toggle_right(self, event):
        left, right = self.state
        idom.update(self, GateState(left, not right))

The first change collects all state changes between renders into one location. This will hopefully make it easier to both understand the code and perhaps allow for code refactors that can focus on operating on state with pure functions. The example below is a bit contrived, but it gets the idea across:

def toggle_gate_state(state: GateState, which: str) -> GateState:
    attrs = state._asdict()
    attrs[which] = not attrs[which]
    return GateState(**attrs)

The second change, while different from React, helps to avoid the implication that calling update will actually change the state attribute of the component immediately after being called. In React its common for new users to expect that the following:

this.state.x == 0
this.setState({x: 1})
this.state.x == 1  // not true

This behavior may seem unintuitive, but its purpose is to isolate each evolution of state to its associated render.

[Archmonger]

I agree with those changes. I'd only recommend stylistic tweaks.

1. Rename update to set_state. Would fall in line with react, and honestly that name makes more sense now that we're using a state variable.
2. Remove self.state = ... from __init__ and have it declared at the base of the class.
3. Use inheritance for component registration via calling idom.component(cls) within __new__
4. Explicitly say new_state = ... in docs examples for clarity, even if it's less memory efficient. Improves readability.
5. Within examples, initialize values within the State class itself to visually indicate that state = State() is just the initial state. Makes assigning values to GateState later on feel more significant.

My proposed styling would look like this

from typing import NamedTuple
from idom import Component
import idom

class GateState(NamedTuple):
    left: bool = False
    right: bool = False

class AndGate(Component):

    state = GateState()

    def render(self):
        left, right = self.state
        return idom.html.div(
            idom.html.input({"type": "checkbox", "onClick": self.toggle_left}),
            idom.html.input({"type": "checkbox", "onClick": self.toggle_right}),
            idom.html.pre(f"{left} AND {right} = {left and right}"),
        )

    def toggle_left(self, event):
        new_state = GateState(not self.state.left, self.state.right)
        idom.set_state(self, new_state)

    def toggle_right(self, event):
        new_state = GateState(self.state.left, not self.state.right)
        idom.set_state(self, new_state)

---

Preventing Bad Practices

We could actually wrap the current state to print a warning (or throw an exception) if the user tries to directly modify the values of self.state. Due to __setattr__ limitations the warning would only appear for immutable values, but it's better than nothing.

Also perhaps a warning/exception if the user replaces the state with something that isn't the same type() as the previous one through isinstance(old_state, new_state).

[Archmonger]

An alternative is to do the set_state through a decorator.

This would enforce a function call to utilize set_state once at most.

I'd only implement it this way if updating the state twice within a single function call wouldn't operate as expected due to technical limitations. And if effect hooks wouldn't be impacted.

...

class AndGate(Component):

    ...

    @idom.set_state
    def toggle_left(self, event):
        return GateState(not self.state.left, self.state.right)

    @idom.set_state
    def toggle_right(self, event):
        return GateState(self.state.left, not self.state.right)

[rmorshea]

Base Class vs Decorator

I think I prefer the @idom.component decorator for a couple reasons:

1. It will allow me to statically identify component classes in flake8-idom-hooks for linting purposes
2. I like that there's just one way to define a component - through a decorator.
3. Doing it through a base class doesn't really simplify any of the magic that has to happen under the hood.

Put State Declaration at Class Level

Unfortunately this doesn't quite work in cases where the initial state depends on parameters from __init__. One could certainly imagine some external package for doing something like that though:

import idom

@idom.component
class Whatever:

    state = State()

    @state.init
    def init_state():
        ...

    @state.change
    def change_it_somehow(self, state):
        ...

set_state Decorator

I think I'm going to leave that out of the core implementation in IDOM, but that reminds me a lot of mobx or immer. So maybe we could make a separate library for doing this in the future similar to the hypothetical API above.

[rmorshea]

After looking into it for a bit, it turns out that making state depend on constructor parameters is an anti pattern. So maybe we can just have people declare a default at the class level and disable direct assignment to the state attribute entirely.

[Archmonger]

If I understand correctly, it sounds like the only styling change thats debated right now is option 3.

Honestly using a decorator on a class isn't bad, but they're so uncommon that it might feel strange to new devs.

If there's going to be significant issues with the flake8 hook when using inheritance then I'm not against the decorator.

[rmorshea]

I'm hoping that you'll be able to use hooks with class-based components. To do that, I'll need to enforce the rules of hooks in the render() method and thus I need to be able to statically identify which classes are component classes.

[rmorshea]

A few more slight changes:

• split effect() into render_effect() (runs after every render) and mount_effect() (runs once after first render)
• allow state attribute to be defined with @property

With these changes I was able to rewrite the snake example: https://gist.github.com/rmorshea/84d66b2ef61ae38171c8066bb4368d36

[Archmonger]

Reading through the snake game rewrite. I dig it, definitely improves readability of the render function, and conceptual understanding of the effect hook.

Couple of thoughts.

1. render_effect is too similarly named to render.
   • I'd suggest use_effect effect_hook run_effect execute_effect start_effect begin_effect display_effect show_effect as alternatives.
   • Keep mind mind that this style is should be mimicked to *_reducer *_callback *_memo *_ref as well. render_* is especially awkward in those use cases.
   • Just using use_effect is probably best.
2. Does on_direction_change need to be created inside render? Would be significantly cleaner if it can be pushed out to the class level.
3. In general I think anything using lambda should be abstracted away from the developer. Potentially can be done through light wrappers for the sake of readability.
   • This definitely doesn't belong in this PR, I just thought I'd mention an awkward design pattern while I'm critiquing
   • Although lambda is amazing, it's generally unintuitive. So you'll get a lot of people asking "why doesn't this work" when they just forgot to use add a lambda.

Example solutions to the lambda struggle

# I don't think any of my suggestions here are ideal
# Can't really think of a way other than light wrappers for lambda though.
# But long term there should be thought of reducing/removing reliance on lambda.

# Current Design
{"onClick": lambda event: idom.set_state(self, GameState.play)}

# Suggested Design(s)
{"onClick": idom.attribute(idom.set_state, self, GameState.play) )}
{"onClick": idom.func(idom.set_state, self, GameState.play) )}
{"onClick": idom.execute(idom.set_state, self, GameState.play) )}
{"onClick": idom.html.func(idom.set_state, self, GameState.play) )}
{"onClick": idom.html.python(idom.set_state, self, GameState.play) )}

[Archmonger]

Does IDOM functional components support multiple of the same hooks? For example, having two use_effect hooks within the same component?

[rmorshea]

Yes.

For class-based components to accomplish the same thing you'll have to do that manually from one method:

async def mount_effect(self):
    await asyncio.gather(
        self.some_effect(),
        self.another_effect(),
    )
    return self.cleanup_mount_effect

def cleanup_mount_effect(self):
    self.cleanup_some_effect()
    self.cleanup_another_effect()

[Archmonger]

This PR should probably be shelved and looked at within the v3 release cycle.

[rmorshea]

I'm going to close this for now.