reflex-dev / reflex

🕸️ Web apps in pure Python 🐍
https://reflex.dev
Apache License 2.0
19.58k stars 1.12k forks source link

Sluggish performance when style prop changes frequently #2917

Open straygar opened 6 months ago

straygar commented 6 months ago

Describe the bug

Use-case: trying to create a rotary knob component (a hidden vertical slider and a circular knob, the rotation of which is controlled by the slider value).

To Reproduce

class AppState(rx.State):
    val: float = 0

    def set_val(self, val: float):
        self.val = val

def index() -> rx.Component:
    return rx.hstack(
        rx.text(
            'Hello world',
            transform=f'rotate({AppState.val}deg)',
        ),
        rx.slider(
            on_change=AppState.set_val,
            min=0,
            max=360,
        ),
        spacing="2",
        width='50%',
        margin='10rem'
    )

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

Expected behavior The text smoothly rotates as you set the slider value.

What actually happens - a dynamic CSS class is created every time the value changes (styled-components?), leading to sluggish performance.

Screenshots If applicable, add screenshots to help explain your problem.

Specifics (please complete the following information):

Additional context

In React & styled components, I would just pass a useState() value as a prop, and that is snappy enough. Even just an inline HTML style would perform better in this case.

masenf commented 6 months ago

It's definitely possible to hack an inline style, but a bit ugly

        rx.text(
            'Hello world',
            #transform=f'rotate({AppState.val}deg)',
            custom_attrs={'style': rx.Var.create({"transform": "rotate(" + AppState.val.to(str) + "deg)"})},
        ),

(f-string does not currently work in a dict, but I believe we have an open issue for that #2155)

It's also possible to deal with local state that's entirely on the browser side, but that's even uglier

(taken from https://github.com/masenf/reflex-pomodoro/blob/main/reflex_pomodoro/components/use_state.py)

import reflex as rx
from reflex.components.component import MemoizationLeaf

class IntegralFragment(rx.Fragment, MemoizationLeaf):
    """A fragment that does not individually memoize its children."""

integral_fragment = IntegralFragment.create

def useState(name, initial_value) -> tuple[rx.Var, str]:
    """Create a react browser useState var.

    Args:
        name: The name of the state variable.
        initial_value: The initial value of the state variable.

    Returns:
        A tuple of the state variable and the name of the setter function.
    """
    set_name = f"set_{name}"
    var_type = type(initial_value)
    get_var = rx.Var.create(name)._replace(
        _var_type=var_type,
        _var_is_local=False,
        _var_is_string=False,
        merge_var_data=rx.vars.VarData(
            imports={"react": {rx.utils.imports.ImportVar(tag="useState")}},
            hooks={f"const [{name}, {set_name}] = useState({initial_value});"},
        ),
    )
    return get_var, set_name

rotVal, setRotVal_func_name = useState("rotVal", 0.0)
setRotVal = rx.Var.create(setRotVal_func_name)._replace(_var_type=rx.EventChain)

def index() -> rx.Component:
    return rx.hstack(
        IntegralFragment.create(
            rx.text(
                'Hello world',
                custom_attrs={'style': rx.Var.create({"transform": "rotate(" + rotVal.to(str) + "deg)"})},
            ),
            rx.slider(
                on_change=setRotVal,
                min=0,
                max=360,
            ),
        ),
        spacing="2",
        width='50%',
        margin='10rem'
    )

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

Now, i don't expect either of these solutions to really be practical for downstream use, because they break abstractions and are admittedly gross... But I also don't know if this is a use case that reflex can spend the time to optimize for at this point.

straygar commented 6 months ago

@masenf Thanks a lot! 🎉 I agree the workaround isn't pretty, but this will at least help in the short term!

I think it'll also help with another issue I had, which is to allow for purely client-side state (e.g. the selected option of a dropdown, that I want to reset if a user refreshes the page), where storing it on the server-side can cause weird inconsistencies, since other variables depend on that state...

I can imagine quite a few uses for purely client-side state, especially if you wanna build your own Reflex components (the docs on that end are a little lacking currently).

masenf commented 6 months ago

The team has been discussing the exposure of client-side-only state (i.e. formalizing the useState function I have defined above), but we haven't decided if it will actually help, or make things more confusing... the requirement to wrap components in an IntegralFragment to allow multiple components to share the client side state is also a bit weird.

@straygar If you take this route an have further feedback on how to make the API nicer, we'd love to hear it.