Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF-3225] implement __format__ for immutable vars #3617

Conversation

adhami3310
Copy link
Member

For new-style immutable vars, the format string mechanism will use a tagged marker that can be used to reconstruct the original Var from the formatted string.

@adhami3310 adhami3310 requested a review from masenf July 3, 2024 23:00
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good, going to test it now

tests/test_var.py Outdated Show resolved Hide resolved
reflex/vars.pyi Outdated Show resolved Hide resolved
masenf
masenf previously requested changes Jul 9, 2024
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the ImmutableVar cannot be created from an f-string with vars inside of it

import reflex as rx


class S(rx.State):
    foo: str = "foo"


# Create an new style Var with data.
iv = rx._x.vars.ImmutableVar.create("foo", _var_data=S.foo._var_data)

# Create an f-string using the new style Var.
f_s = f"{iv}bar"

# Create an old style Var from the f-string.
n_v = rx.Var.create(f_s)
print(n_v, repr(n_v))

# Create a new style Var from the f-string.
n_iv = rx._x.vars.ImmutableVar.create(f_s)
print(n_iv, repr(n_iv))

It seems there is a bit of code in rx.Var.__post_init__ which is trying to assign to self._var_name and self._var_data, but this does not work for immutable instances.

Instead we can change the code to call __init__ again with the updated values, and this should be fine in nearly every case. It sort of breaks immutability, but if we consider __post_init__ to be just an extension of __init__, then it makes sense.

diff --git a/reflex/vars.py b/reflex/vars.py
index ad7c223a..f55aa025 100644
--- a/reflex/vars.py
+++ b/reflex/vars.py
@@ -502,8 +502,10 @@ class Var:
         # Decode any inline Var markup and apply it to the instance
         _var_data, _var_name = _decode_var(self._var_name)
         if _var_data:
-            self._var_name = _var_name
-            self._var_data = VarData.merge(self._var_data, _var_data)
+            self.__init__(
+                _var_name=_var_name,
+                _var_data=VarData.merge(self._var_data, _var_data),
+            )
 
     def _replace(self, merge_var_data=None, **kwargs: Any) -> BaseVar:
         """Make a copy of this Var with updated fields.

@adhami3310 adhami3310 dismissed masenf’s stale review July 10, 2024 20:43

changed post_init behavior in a different file

@masenf masenf merged commit d4d0778 into main Jul 11, 2024
47 checks passed
Alek99 pushed a commit that referenced this pull request Jul 15, 2024
* implement format for immutable vars

* add some basic test

* make reference only after formatting

* win over pyright

* hopefully now pyright doesn't hate me

* forgot some _var_data

* i don't know how imports work

* use f_string var and remove assignments from pyi file

* override post_init to not break immutability

* add create_safe and test for it
@masenf masenf deleted the khaleel/ref-3225-immutablevar__format__-should-save-a-reference-to-the branch December 12, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants