reactive-python / reactpy

It's React, but in Python
https://reactpy.dev
MIT License
7.89k stars 318 forks source link

Logging when `set_state` is called and no changes occur #565

Closed Archmonger closed 1 year ago

Archmonger commented 2 years ago

Current Situation

Currently, there exists circumstances where set_state can be called and nothing will occur. For example, modifying a mutable class (such as a dataclass)

@dataclass
class MyState:
   value: str = ""

@idom.component
def my_component():
   state, set_state = idom.hooks.use_state(MyState())

   async def on_click(event):
      state.value = "dog"
      set_state(state)
      # This does not trigger a re-render. Need to use copy.copy(state).

   return button( {"onClick"=on_click}, ... )

Proposed Changes

Notify the user if set_state was called and no re-render was queued.

Implementation Details

When in IDOM_DEBUG_MODE... Show either logging.warning or logging.info when set_state is called but a re-render was not trigged.

rmorshea commented 2 years ago

React also has a useDebugValue hook. It would be fairly simple to implement something that logs when the value changes:

@component
def SomeComponent():
    is_online, set_is_online = use_state(False)

    use_debug_value("is online" if is_online else "is offline")

    ...

Which might log:

DEBUG: SomeComponent(10c0049a0) is online
DEBUG: SomeComponent(10c0049a0) is offline
Archmonger commented 2 years ago

On a related note, does react have the same issues of mutability?

rmorshea commented 2 years ago

Yes it does. There's a section in React's new documentation about it which I'm also writing for IDOM presently.

rmorshea commented 2 years ago

React's doc suggest using immer to avoid some of these problems. The best solution to this problem in Python that I'm aware of is pyresistent (which I plan to call out in the docs).

Archmonger commented 2 years ago

While I could see this being a challenge in JavaScript, this could be resolved within the IDOM framework.

From my perspective, I'm effectively calling set_state(copy.copy(my_object)) every time, unless the value is immutable. Immutable collections in pyresisant are effectively doing the same on every value mutation. This adds on unnecessary overhead.

Perhaps if issubclass(my_object, (collections.MutableSequence, collections.MutableSet, collections.MutableMapping)), then we can automatically force a re-render on every set_state. Or to mimic what is currently occurring, the state hook can call copy.copy(my_object), if old_state==new_state).

I don't think there's a need to inherit the same limitations as react within the Python space.

Thoughts?

rmorshea commented 2 years ago

While it might feel like a limitation, this choice by React makes you think about mutations in the hope that it will encourage you to avoid them. While you can get away with mutating state and then forcing a render by setting a copy, it can introduce unexpected bugs. Consider that this:

new_list = old_list + [value]
# or this even
new_list = old_list.copy()
new_list.append(value)

Is subtlety different, from this:

old_list.append(value)
new_list = old_list.copy()

Because we've modified the old_list, anything which is holding onto that old_list might now be affected.

Here's a somewhat more concrete example... Imagine that you have some message form with a dropdown for selecting a recipient and a button for sending messages. The catch is that sending the message takes some time, and we need to await some things before it actually gets sent. Consider the code below and try to spot the bug:

@component
def MessageForm():
    state, set_state = use_state({
        "recipient": "Alice",
        "message": ""
    })

    async def handle_send(event):
        await some_long_task()
        await send_message(state["recipient"], state["message"])

    def handle_recipient_change(event):
        state["recipient"] = event["target"]["value"]
        new_state = state.copy()
        set_state(new_state)

    def handle_message_change(event):
        state["message"] = event["target"]["value"]
        new_state = state.copy()
        set_state(new_state)

    ....

There's a very subtle bug here where, if the user attempts to send the message (which takes some time) and then immediately changes the recipient, the message will get sent to the recipient they just changed to rather than the recipient which was selected at the time they tried to send. A similar bug would happen with the state["message"] as well. This is because we're modifying the same state instance which we use in the event handler to send_message(state["recipient"], state["message"]).

rmorshea commented 1 year ago

closing. this is addressed by #568 and the use_debug_value hook.