Skip to content

Commit

Permalink
fix: Avoid double tracking of ref and parent
Browse files Browse the repository at this point in the history
Before, we would listen to both the ref
and the parent reactive variable.
  • Loading branch information
maartenbreddels committed Feb 15, 2024
1 parent 8809aeb commit ad2027f
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
13 changes: 8 additions & 5 deletions solara/toestand.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,12 @@ def get(self, add_watch=None) -> S:
warnings.warn("add_watch is deprecated, use .peek()", DeprecationWarning)
if thread_local.reactive_used is not None:
thread_local.reactive_used.add(self)
return self._storage.get()
# peek to avoid parents also adding themselves to the reactive_used set
return self._storage.peek()

def peek(self) -> S:
"""Return the value without automatically subscribing to listeners."""
return self._storage.get()
return self._storage.peek()

def subscribe(self, listener: Callable[[S], None], scope: Optional[ContextManager] = None):
return self._storage.subscribe(listener, scope=scope)
Expand Down Expand Up @@ -495,7 +496,7 @@ def wrapper(f: Callable[[], T]):
return wrapper(f)


class ValueSubField(ValueBase[T]):
class ReactiveField(ValueBase[T]):
def __init__(self, field: "FieldBase"):
super().__init__() # type: ignore
self._field = field
Expand All @@ -509,7 +510,7 @@ def __str__(self):
return str(self._field)

def __repr__(self):
return f"<Reactive subfield {self._field}>"
return f"<Reactive field {self._field}>"

@property
def lock(self):
Expand Down Expand Up @@ -548,6 +549,7 @@ def get(self, add_watch=None) -> T:
warnings.warn("add_watch is deprecated, use .peek()", DeprecationWarning)
if thread_local.reactive_used is not None:
thread_local.reactive_used.add(self)
# peek to avoid parents also adding themselves to the reactive_used set
return self._field.peek()

def peek(self) -> T:
Expand All @@ -559,7 +561,7 @@ def set(self, value: T):

def Ref(field: T) -> Reactive[T]:
_field = cast(FieldBase, field)
return Reactive[T](ValueSubField[T](_field))
return cast(Reactive[T], ReactiveField[T](_field))


class FieldBase:
Expand Down Expand Up @@ -758,6 +760,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):

# alias for compatibility
State = Reactive
ValueSubField = ReactiveField

auto_subscribe_context_manager = AutoSubscribeContextManagerReacton
reacton.core._component_context_manager_classes.append(auto_subscribe_context_manager)
18 changes: 16 additions & 2 deletions tests/unit/toestand_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import solara
import solara as sol
import solara.lab
import solara.toestand as toestand
from solara.server import kernel, kernel_context
from solara.toestand import Reactive, Ref, State, use_sync_external_store

Expand Down Expand Up @@ -842,17 +843,24 @@ def test_reactive_auto_subscribe_sub():
bears = Reactive(Bears(type="brown", count=1))
renders = 0

ref = Ref(bears.fields.count)
reactive_used = None

@solara.component
def Test():
nonlocal reactive_used
nonlocal renders
reactive_used = toestand.thread_local.reactive_used
renders += 1
count = Ref(bears.fields.count).value
count = ref.value
return solara.Info(f"{count} bears around here")

box, rc = solara.render(Test(), handle_error=False)
assert rc.find(v.Alert).widget.children[0] == "1 bears around here"
Ref(bears.fields.count).value += 1
assert reactive_used == {ref}
ref.value += 1
assert rc.find(v.Alert).widget.children[0] == "2 bears around here"
assert reactive_used == {ref}
# now check that we didn't listen to the while object, just count changes
renders_before = renders
Ref(bears.fields.type).value = "pink"
Expand Down Expand Up @@ -900,19 +908,25 @@ def Test():
def test_reactive_auto_subscribe_subfield_limit(kernel_context):
bears = Reactive(Bears(type="brown", count=1))
renders = 0
reactive_used = None

@solara.component
def Test():
nonlocal renders
nonlocal reactive_used
reactive_used = toestand.thread_local.reactive_used
renders += 1
_ = bears.value # access it to trigger the subscription
return solara.IntSlider("test", value=Ref(bears.fields.count).value)

box, rc = solara.render(Test(), handle_error=False)
assert rc.find(v.Slider).widget.v_model == 1
assert renders == 1
assert reactive_used is not None
assert len(reactive_used) == 2 # bears and bears.fields.count
Ref(bears.fields.count).value = 2
assert renders == 2
assert len(reactive_used) == 2 # bears and bears.fields.count
rc.close()
assert not bears._storage.listeners[kernel_context.id]
assert not bears._storage.listeners2[kernel_context.id]
Expand Down

0 comments on commit ad2027f

Please sign in to comment.