Implement on_mount and on_unmount for all components.
#1636
Conversation
These end up being a little weird due to react StrictMode when running the dev server... so the events are kind of duplicated, since the useEffect gets called twice essentially.
| return ( | ||
| EVENT_TRIGGERS | ||
| | set(self.get_controlled_triggers()) | ||
| | set((constants.ON_MOUNT, constants.ON_UNMOUNT)) |
There was a problem hiding this comment.
Shouldn't we just add these to the default EVENT_TRIGGERS ?
There was a problem hiding this comment.
i went back and forth on this, and ultimately did it this way because these aren't "real" event triggers that come from browser events, but are synthetic events that reflex handles.
| @@ -182,6 +183,24 @@ def format_string(string: str) -> str: | |||
| return string | |||
|
|
|||
|
|
|||
| def format_var(var: Var) -> str: | |||
There was a problem hiding this comment.
Shouldn't this be the same as the Var.str method? Should we move this code there? This was the intention of that method.
There was a problem hiding this comment.
hmm, it's a little different. i cribbed this from Tag.format_prop, so i assumed it must be doing something special. i'll see if i can combine them without breaking tests, so we can just use str(var) instead.
| [ | ||
| "(() => {", | ||
| format_var(event_chain), | ||
| f"; preventDefault({format_var(event_arg)})" if event_arg else "", |
There was a problem hiding this comment.
The Event function in state.js should also handle the preventDefault logic I thought?
There was a problem hiding this comment.
if we're formatting an arbitrary var as an event chain, there's no guarantee that Event gets called at all; it could be anything.
|
We also need to update |
I meant to get to it today, but it got away from me. Definitely will have to land that before this can come in. |
This reverts commit 5959362.
Now the ref hooks are managed by a separate internal function so that downstream component wrappers can worry less about breaking framework functionality when overriding `_get_hooks`; but this means any internal functionality that was previously overriding `_get_hooks` was still unconditionally getting the base `_get_ref_hooks` functionality, that is was intentionally trying to override. Really glad we wrote integration tests for this behavior!
All components expose
on_mountandon_unmounttriggersThese are tied directly into the react lifecycle hook
useEffect. Theon_mountis called as the main part of the effect, withon_unmountchain being called as the cleanup function. The ordering of when these are called for different components is non-deterministic. In dev mode, these triggers will fire twice, because react calls alluseEffecthooks twice in StrictMode.REF-231
Expose
format_varandformat_event_chainPreviously, these were internal to
Tag.format_props, but exposing them here allows them to be reused in other contexts.Partial fix for REF-209, but leaving open until the
Tagcode can be refactored to use the new methods.Split
_get_hooksinto_get_hooks_internalMake it easier to downstream components to override
_get_hookswithout having to worry about screwing up Reflex's ref and on_mount handling behavior.