feat: Synchronizing localStorage between tabs using browser events

[abulvenz]

This PR creates a synchronization mechanism between different tabs. When state-relevant localStorage variables are changed in different tabs it will cause a state change event in the current tab.

The default is off, so the behavior will not change.
When you want to use the sync effect, do it like this:

class MyState(rx.State):
  synced_local_storage: str = rx.LocalStorage(
       "DefaultValue", 
       name="my_key", 
       sync=True  # this is new :)
  )

I have tested it manually with the client storage example from the homepage.

I hope that

• I added the javascript changes at the right spot (I haven't done too much react before).
• The setter-prefix usage is the right way to accomplish the effect. Another way might have been to rely on something in the context.js.jinja template, but I didn't see how I could add it there.
• I maybe have found a solution for doing an integration test (or adding this to the existing integration test, using driver.switch_to.new_window("tab")

Note this PR was inspired by the ease and lightning fast reaction when changing the dark/light mode using the rx.color_mode_button(). Maybe in the future, such features can also rely on the state.

All Submissions:

• Have you followed the guidelines stated in CONTRIBUTING.md file?
• Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update
  Tiny one.

New Feature Submission:

• Does your submission pass the tests?
• Have you linted your code locally prior to submission?

Changes To Core Features:

• Have you added an explanation of what your changes do and why you'd like us to include them?

• Have you written new tests for your core changes, as applicable?
  Not yet

• Did you successfully run tests with your changes locally?

[masenf]

Wow this is an awesome idea and good execution!

I did find a minor issue though

import reflex as rx


class State(rx.State):
    sync1: str = rx.LocalStorage(sync=True)
    sync2: str = rx.LocalStorage(sync=True)

    def set_sync2(self, foo: str):
        self.sync2 = foo


def index() -> rx.Component:
    return rx.fragment(
        rx.color_mode_button(rx.color_mode_icon(), float="right"),
        rx.vstack(
            rx.input(value=State.sync1, on_change=State.set_sync1),
            rx.input(value=State.sync2, on_change=State.set_sync2),
        ),
    )


# Create app instance and add index page.
app = rx.App()
app.add_page(index)

In this case, the user has overwritted set_sync2 and it doesn't accept the value arg anymore and thus, does not work (throws an error in the console TypeError: State.set_sync2() got an unexpected keyword argument 'value').

I think the way to handle this is to implement an internal event handler that allows the setting of arbitrary values.

Have a look at state.py on_load_internal function. And also have a look at how client side storage is handled in hydrate_middleware.py.

Consider replacing the client storage functionality of hydrate (which we're trying to deprecate anyway) with an explicit hydrate_client_storage_internal event handler defined on rx.State. This event handler could take an arbitrary dict and apply the values to the state.

Let me know if you have any further questions about this suggestions, and thanks again for the awesome contribution.

[abulvenz]

I did find a minor issue though

How did you come up that fast with this edge case? 🥇

You are right, I have tried your example and I understand the issue with it. Of course I asked myself why the caller of a setter needs to know the names of arguments of a setter, but maybe that is some python/reflex-detail that I'm not aware of.

I could not help notice though that the compiler doesn't seem to have any problems with the foo.

const on_change_[...] = useCallback((_e0) => addEvents([Event("state.state.set_sync2", {foo:_e0.target.value})], (_e0), {}), [addEvents, Event])

My idea would be to use some of that generation logic to have nice events with proper payloads in Javascript.
But that change is out of my scope of knowledge of the compilation part of reflex.
Your other suggestions are too deep inside the god-class BaseState for my current overall understanding of reflex 😄

So my suggestion would be: Let's throw an error with a better message: "Please name your setter argument 'value'."

[abulvenz]

Thanks again for your review @masenf and many thanks for your work on this awesome framework.

When re-reading my last post I realized that I was not posing any concrete questions about your suggestions. I'm sorry about that.
When reading through the code there arose some questions about how your idea was to implement this.

• Usually frontend events know which payload to send (that's what the code-example in my last post was about), i.e. the name of the setter argument.
• The client storage handling in hydrate_middleware.py uses setattr directly. I placed some debug print in my setter override set_sync2 and saw that it wasn't called during hydration. Is that why you meant that this should be deprecated soon?
• From what I understood so far, hydration means synchronizing the initial state with the frontend. My change is not for the initial state (which works now), but for the common operation of the page.

So I want to lay out a set of implementation options. Maybe you have a better opinion that me as a reflex-noob.

1. Throw an error explaining the problem and either let the user fix her setter arg name or use that in combination with 2. This transfers the problem to the user.
2. Misuse the sync parameter, but with proper documentation. It could be sync: bool | str where it being str means not only true but has to specify the name for the payload key. That is the fastest way to handle the problem with setter override. I already implemented it for testing purposes and could immediately push it.
3. Calculate the setter arg name and add a calculated field to the frontend options of LocalStorage. E.g. options._setter_arg_name to construct the correct payload in the frontend. This would be no struggle for the user. But I wouldn't know at the moment in which state of the app this has to be done.
4. Add something to the compiler that adds another level of indirection for the events. For each EventSpec we could create in the context.js.jinja something like const event_set_sync2 = (arg0)=Event('state...set_sync2',{foo:arg0}); (Maybe that could even help with some other future features, e.g. passing multiple or fixed arguments to Events.)
5. Inspect the payloads in _process_event and fix the payload fields according to the setter arg names. Then the question would be: why do we need the fields inside the payloads? Maybe it could cause weird side-effects later.

(Regarding my background: Last August I started using python again after a fifteen year break. So my favorite solutions are the combination of 1. and 2. or even only 2. :)

[masenf]

@abulvenz Apologies for the delay; we're preparing for a huge release for the last week and I didn't have time to get back to you earlier.

I have pushed to your branch with my suggested change and added a test using that driver.switch_to.new_window method and it's working quite well.

I've added a new update_vars_internal event handler on the base rx.State class; then using this event handler to set the required values. It doesn't require any munging of the state_key, so the code around the JS storage handler is also now simplified. Further, I relocated the hydrateClientState logic from the state.hydrate event to this new state.update_vars_internal event and am calling it on every page load. This should fix another recently reported bug (#2565)

Have a look at the changes and let me know if they make sense.

[abulvenz]

Hi @masenf .
I like your changes and especially the test!
Feel free to merge it like that, I tested it locally and it seems to work as expected.
One minor point that might be an architectural decision:
The overridden setters are not called due to the use of setattr instead of going via the EventHandlers. If that is the intended behavior, maybe we should mention that in the documentation.
Thanks for your effort!

[masenf]

The overridden setters are not called due to the use of setattr instead of going via the EventHandlers. If that is the intended behavior, maybe we should mention that in the documentation.

This is indeed the intended behavior. Any overridden setter will be triggered by the tab that initiates the change to the var. That setter will update the state and the final value will be pushed into local storage from that tab. Once the storage event gets called in the other tabs, we should preserve that value exactly as is in the other tabs associated with that browser, rather than passing it through the setter again, which might not be idempotent.

[abulvenz]

Thanks for the explanation @masenf. I just reassured myself that setattr invalidates the value for the computed and cached vars. So that should really be the way to go. Let's merge it!

[abulvenz]

Thank you, @masenf !