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-1705] Do not overwrite Var attributes during format #2421

Merged
merged 2 commits into from Jan 19, 2024

Conversation

masenf
Copy link
Collaborator

@masenf masenf commented Jan 19, 2024

If the values given to format_cond are already Var, do NOT mutate the attributes since subsequent usages of the same Var may be adversely affected.

Add test cases for is_prop=True path of format_cond, where this bug was hiding.

Here is a repro case

import reflex as rx
import reflex.components.radix.themes as rdxt


class State(rx.State):
    val: str = ""


def index() -> rx.Component:
    print(repr(State.val))
    comp = rdxt.flex(
        # The following line exposes the bug: the radio group value gets formatted incorrectly.
        rdxt.text(rx.cond(State.val, State.val, "No Selection")),
        rdxt.text(State.val),
        rdxt.radio_group_root(
            rdxt.radio_group_item(value="1"),
            rdxt.radio_group_item(value="2"),
            rdxt.radio_group_item(value="3"),
            rdxt.radio_group_item(value="4"),
            value=State.val,
            on_value_change=State.set_val,
        ),
        rdxt.button("Clear", on_click=State.set_val("")),
        direction="column",
        justify="center",
        align="center",
        gap="2",
    )
    print(repr(State.val))
    return comp


app = rx.App()
app.add_page(index)

Check the output, the State.val repr should NOT change just because the value was used in an rx.cond.

If the values given to `format_cond` are already Var, do NOT mutate the
attributes since subsequent usages of the same Var may be adversely affected.

Add test cases for `is_prop=True` path of `format_cond`, where this bug was
hiding.
Needed for 3.8 and 3.9 compatibility
@picklelo picklelo merged commit 9446a1e into main Jan 19, 2024
43 checks passed
@picklelo picklelo deleted the masenf/format-cond-no-mutate branch January 30, 2024 03:08
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.

None yet

2 participants