Repository: reflex-dev/reflex
Pull request: Add and use `pc.list` to have list mutation detection
To add and use `pc.list`/`PcList` to have list mutation detection.

It's not as straightforward as I thought. Or is my approach too complicated?

For #334. `pc.dict` would be in another PR.

Problem statement:
Create pc.dict and pc.list
Currently Pynecone doesn't detect mutations in list/dict state vars which is annoying and can lead to bugs if one is not careful.

Maybe we could create classes `pc.dict` and `pc.list` that wrap any dict/list State vars, and modify them such that they detect mutations.

Hints from issue discussion:
I can work on this. Can you provide an example how `pc.dict` & `pc.list` would look like? My guess would be sth like this:
```py
def index():
  colors = pc.list(['red', 'oragen'])

  return pc.vstack(
    pc.text('Number of colors:' + len(colors)),
    pc.button('+', on_click=lambda : colors.append('another color'))
  )
```
There was some discussion on Discord about that, but the idea would be to use `pc.list` and `pc.dict` behind the hood.

If you declare:
```
class State(pc.State):
    somevar: list[str] = []
```
then using State.somevar should behave like a list, even if we actually replace it by a `pc.list`

(same thing with dict)

But maybe it can be done in two step: 
1. Making the actual classes `pc.list` and `pc.dict` (so they can be used explicitly)
2. Use them implicitly in `pc.State`
Yeh, we can use a real list / dict to store data, and rewrite \_\_setitem__ to detect any change to it.

Commit message: simply PcList

Patch to review:
diff --git a/pynecone/state.py b/pynecone/state.py
index 0957001285d..378bc1696df 100644
--- a/pynecone/state.py
+++ b/pynecone/state.py
@@ -13,7 +13,7 @@
 from pynecone import constants, utils
 from pynecone.base import Base
 from pynecone.event import Event, EventHandler, window_alert
-from pynecone.var import BaseVar, ComputedVar, Var
+from pynecone.var import BaseVar, ComputedVar, PcList, Var
 
 Delta = Dict[str, Any]
 
@@ -61,6 +61,39 @@ def __init__(self, *args, **kwargs):
         for substate in self.get_substates():
             self.substates[substate.get_name()] = substate().set(parent_state=self)
 
+        self._init_mutable_fields()
+
+    def _init_mutable_fields(self):
+        """Initialize mutable fields.
+
+        So that mutation to them can be detected by the app:
+        * list
+        """
+        for field in self.base_vars.values():
+            if field.type_ is list:
+                setattr(
+                    self,
+                    field.name,
+                    PcList(
+                        getattr(self, field.name),
+                        reassign_field=lambda: self._reassign_field(field.name),
+                    ),
+                )
+
+    def _reassign_field(self, field_name: str):
+        """Reassign the given field.
+
+        Primarily for mutation in fields of mutable data types.
+
+        Args:
+            field_name (str): The name of the field we want to reassign
+        """
+        setattr(
+            self,
+            field_name,
+            getattr(self, field_name),
+        )
+
     def __repr__(self) -> str:
         """Get the string representation of the state.
 
diff --git a/pynecone/var.py b/pynecone/var.py
index 21816bd66de..8a34e5c9585 100644
--- a/pynecone/var.py
+++ b/pynecone/var.py
@@ -748,3 +748,54 @@ def type_(self):
         if "return" in self.fget.__annotations__:
             return self.fget.__annotations__["return"]
         return Any
+
+
+class PcList(list):
+    """A custom list that pynecone can detect its mutation."""
+
+    def __init__(
+        self,
+        original_list: list,
+        reassign_field: Callable = lambda: None,
+    ):
+        """Initialize PcList.
+
+        Args:
+            original_list (list): The original list
+            reassign_field (function | None):
+                The method in the parent state to reassign the field.
+            field_name (str): The field name of the list in the parent state.
+        """
+        self._reassign_field = reassign_field
+
+        super().__init__(original_list)
+
+    def append(self, *args, **kargs):
+        """Append.
+
+        Args:
+            args: The args passed.
+            kargs: The kwargs passed.
+        """
+        super().append(*args, **kargs)
+        self._reassign_field()
+
+    def __setitem__(self, *args, **kargs):
+        """Set item.
+
+        Args:
+            args: The args passed.
+            kargs: The kwargs passed.
+        """
+        super().__setitem__(*args, **kargs)
+        self._reassign_field()
+
+    def __delitem__(self, *args, **kargs):
+        """Delete item.
+
+        Args:
+            args: The args passed.
+            kargs: The kwargs passed.
+        """
+        super().__delitem__(*args, **kargs)
+        self._reassign_field()
diff --git a/tests/test_app.py b/tests/test_app.py
index 4f1dc09af02..7d2eb9373c0 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -4,6 +4,7 @@
 
 from pynecone.app import App, DefaultState
 from pynecone.components import Box
+from pynecone.event import Event
 from pynecone.middleware import HydrateMiddleware
 from pynecone.state import State
 from pynecone.style import Style
@@ -156,3 +157,64 @@ def test_set_and_get_state(TestState: Type[State]):
     state2 = app.state_manager.get_state(token2)
     assert state1.var == 1  # type: ignore
     assert state2.var == 2  # type: ignore
+
+
+@pytest.fixture
+def list_mutation_state():
+    class AppState(State):
+        """The app state."""
+
+        friends = ["Tommy"]
+
+        def make_friend(self):
+            self.friends.append("another-fd")
+
+        def change_first_friend(self):
+            self.friends[0] = "Jenny"
+
+        def unfriend_all_friends(self):
+            self.friends.clear()
+
+        def unfriend_first_friend(self):
+            del self.friends[0]
+
+    return AppState()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "event_tuples",
+    [
+        pytest.param(
+            [
+                ("app_state.make_friend", ["Tommy", "another-fd"]),
+                ("app_state.change_first_friend", ["Jenny", "another-fd"]),
+            ],
+            id="append then __setitem__",
+        ),
+        pytest.param(
+            [
+                ("app_state.unfriend_first_friend", []),
+                ("app_state.make_friend", ["another-fd"]),
+            ],
+            id="delitem then append",
+        ),
+    ],
+)
+async def test_list_mutation_detection(
+    event_tuples: list[tuple[str, list[str]]], list_mutation_state: State
+):
+    """Test list mutation detection
+    when reassignment is not explicitly included in the logic.
+    """
+    for event_name, expected_friends_in_delta in event_tuples:
+        result = await list_mutation_state.process(
+            Event(
+                token="fake-token",
+                name=event_name,
+                router_data={"pathname": "/", "query": {}},
+                payload={},
+            )
+        )
+
+        assert result.delta["app_state"]["friends"] == expected_friends_in_delta


Code review comments:
diff_hunk: @@ -748,3 +748,54 @@ def type_(self):
         if "return" in self.fget.__annotations__:
             return self.fget.__annotations__["return"]
         return Any
+
+
+class PcList(list):
path: pynecone/var.py
text: @author:
`PcList` sounds more reasonable to me than just `List` cuz it's a list object specifically for pcnecone. IMO, `List` is appropriate when it is just a list but with more generalised capabilities.

@user1:
Yup sounds good!

@user2:
Small nit, but `PcList` doesn't follow PEP-8 naming convention, which we should defer to when possible. Prefer `PCList`:

> *Note: When using acronyms in CapWords, capitalize all the letters of the acronym. Thus HTTPServerError is better than HttpServerError.*

Source: [PEP-8](https://peps.python.org/pep-0008/)


@author:
Thanks for catching that! [fd7c722](https://github.com/pynecone-io/pynecone/pull/339/commits/fd7c722759bc0e45631f10030e5446935b2f5df3)