fix: render hooks in declaration order instead of by HookPosition

docs/wrapping-react/custom-code-and-hooks.md

@@ -34,20 +34,12 @@ console.log(customVariable);
 Custom hooks are any hooks that need to be included in your component. This can include things like `useEffect`, `useState`, or any other hooks from the library you are wrapping.
 
 - Simple hooks can be added as strings.
-- More complex hooks that need to have special import or be written in a specific order can be added as `rx.Var` with a `VarData` object to specify the position of the hook.
-  - The `imports` attribute of the `VarData` object can be used to specify any imports that need to be included in the component.
-  - The `position` attribute of the `VarData` object can be set to `Hooks.HookPosition.PRE_TRIGGER` or `Hooks.HookPosition.POST_TRIGGER` to specify the position of the hook in the component.
+- Hooks that need an import can be added as `rx.Var` with a `VarData` object. The `imports` attribute of the `VarData` object specifies any imports that need to be included in the component.
 
-```md alert info
-# The `position` attribute is only used for hooks that need to be written in a specific order.
-
-- If an event handler need to refer to a variable defined in a hook, the hook should be written before the event handler.
-- If a hook need to refer to the memoized event handler by name, the hook should be written after the event handler.
-```
+Hooks render in the order they are declared. The hooks the framework manages (refs, `on_mount`, and the state hooks that props read) come first, then the hooks returned by `add_hooks` in the order of the list, with a base class's hooks before a subclass's.
 
 ```python
 from reflex.vars.base import Var, VarData
-from reflex_base.constants import Hooks
 from reflex.components.el.elements import Div
 
 
@@ -60,34 +52,19 @@ class ComponentWithHooks(Div, MyBaseComponent):
         hooks1 = """const customHookVariable = "some value";"""
         hooks.append(hooks1)
 
-        # A hook that need to be written before the memoized event handlers.
+        # A hook that reads the variable declared by the hook before it.
         hooks2 = Var(
             """useEffect(() => {
-                console.log("PreTrigger: " + customHookVariable);
+                console.log(customHookVariable);
             }, []);
             """,
             _var_data=VarData(
                 imports={
                     "react": ["useEffect"],
                 },
-                position=Hooks.HookPosition.PRE_TRIGGER,
             ),
         )
         hooks.append(hooks2)
-
-        hooks3 = Var(
-            """useEffect(() => {
-                console.log("PostTrigger: " + customHookVariable);
-            }, []);
-            """,
-            _var_data=VarData(
-                imports={
-                    "react": ["useEffect"],
-                },
-                position=Hooks.HookPosition.POST_TRIGGER,
-            ),
-        )
-        hooks.append(hooks3)
         return hooks
 ```
 
@@ -98,19 +75,63 @@ export function Div_7178f430b7b371af8a12d8265d65ab9b() {
   const customHookVariable = "some value";
 
   useEffect(() => {
-    console.log("PreTrigger: " + customHookVariable);
+    console.log(customHookVariable);
   }, []);
 
-  /* memoized triggers such as on_click, on_change, etc will render here */
+  return jsx("div", {});
+}
+```
+
+### Hooks that read other hooks
+
+When a hook reads a value that another hook declares, list that declaration in the `hooks` attribute of its `VarData`. Each entry maps a hook's code to its own `VarData` (or `None`). A hook's dependencies always render before it, in whichever component the hook ends up in, so a hook `Var` can be reused across components or passed in through a prop without caring about where its dependency is declared.
+
+```python
+from reflex.vars.base import Var, VarData
+from reflex.components.el.elements import Div
+
+
+class CountLogger(Div, MyBaseComponent):
+    """MyComponent."""
+
+    def add_hooks(self) -> list[str | Var]:
+        """Add hooks to the component."""
+        use_count = VarData(
+            imports={"react": ["useState"]},
+        )
+        log_count = Var(
+            """useEffect(() => {
+                console.log(count);
+            }, [count]);
+            """,
+            _var_data=VarData(
+                imports={"react": ["useEffect"]},
+                hooks={"const [count, setCount] = useState(0);": use_count},
+            ),
+        )
+        return [log_count]
+```
+
+The `useState` declaration renders before the `useEffect` that reads it, and both imports are included:
+
+```javascript
+export function Div_2b1c5f3e9a7d4c6e8f0a1b2c3d4e5f60() {
+  const [count, setCount] = useState(0);
 
   useEffect(() => {
-    console.log("PostTrigger: "+ customHookVariable);
-  }, []);
+    console.log(count);
+  }, [count]);
 
   return jsx("div", {});
 }
 ```
 
+```md alert warning
+# The `position` attribute of `VarData` and `Hooks.HookPosition` are deprecated.
+
+Earlier versions sorted hooks into `PRE_TRIGGER` and `POST_TRIGGER` groups. Passing `position` now warns and has no effect on the order. To place a hook after another one, list the hook it reads in its `VarData(hooks=...)` as shown above.
+```
+
 ```md alert info
 # You can mix custom code and hooks in the same component. Hooks can access a variable defined in the custom code, but custom code cannot access a variable defined in a hook.
 ```

packages/reflex-base/news/+hook-declaration-order.bugfix.md

@@ -0,0 +1 @@
+Component hooks render in the order they are declared and merged, so a hook always comes after the hooks it reads. Before, a component whose prop and `add_hooks` Var read the same state could render a hook before the state declaration it reads and fail with a `ReferenceError`. A hook that reads state must carry it: return `rx.Var(f"const x = {State.value}")` from `add_hooks`, not a plain string built with `str(State.value)`.

packages/reflex-base/news/+hook-position.deprecation.md

@@ -0,0 +1 @@
+`VarData(position=...)` and `Hooks.HookPosition` are deprecated and ignored: hooks render in declaration order. Remove the `position` argument; to place a hook after another one, put the one it reads in its `VarData(hooks=...)`.

packages/reflex-base/src/reflex_base/compiler/templates.py

@@ -8,7 +8,6 @@
 from typing import TYPE_CHECKING, Any, Literal
 
 from reflex_base import constants
-from reflex_base.constants import Hooks
 from reflex_base.utils import memo_paths
 from reflex_base.utils.format import json_dumps
 from reflex_base.vars.base import VarData
@@ -18,39 +17,6 @@
     from reflex_base.components.component import Component
 
 
-def _sort_hooks(
-    hooks: dict[str, VarData | None],
-) -> tuple[list[str], list[str], list[str]]:
-    """Sort the hooks by their position.
-
-    Args:
-        hooks: The hooks to sort.
-
-    Returns:
-        The sorted hooks.
-    """
-    internal_hooks = []
-    pre_trigger_hooks = []
-    post_trigger_hooks = []
-
-    for hook, data in hooks.items():
-        if data and data.position and data.position == Hooks.HookPosition.INTERNAL:
-            internal_hooks.append(hook)
-        elif not data or (
-            not data.position
-            or data.position == constants.Hooks.HookPosition.PRE_TRIGGER
-        ):
-            pre_trigger_hooks.append(hook)
-        elif (
-            data
-            and data.position
-            and data.position == constants.Hooks.HookPosition.POST_TRIGGER
-        ):
-            post_trigger_hooks.append(hook)
-
-    return internal_hooks, pre_trigger_hooks, post_trigger_hooks
-
-
 class _RenderUtils:
     @staticmethod
     def render(component: Mapping[str, Any] | str) -> str:
@@ -1191,19 +1157,16 @@ def styles_template(stylesheets: list[str]) -> str:
     ])
 
 
-def _render_hooks(hooks: dict[str, VarData | None], memo: list | None = None) -> str:
-    """Render hooks for macros.
+def _render_hooks(hooks: dict[str, VarData | None]) -> str:
+    """Render hooks in the order they were declared and merged.
+
+    Every hook's dependencies are declared before it, so declaration order is
+    the dependency order.
 
     Args:
         hooks: Dictionary of hooks to render.
-        memo: Optional list of memo hooks.
 
     Returns:
         Rendered hooks code as string.
     """
-    internal, pre_trigger, post_trigger = _sort_hooks(hooks)
-    internal_str = "\n".join(internal)
-    pre_trigger_str = "\n".join(pre_trigger)
-    post_trigger_str = "\n".join(post_trigger)
-    memo_str = "\n".join(memo) if memo is not None else ""
-    return f"{internal_str}\n{pre_trigger_str}\n{memo_str}\n{post_trigger_str}"
+    return "\n".join(hooks)

packages/reflex-base/src/reflex_base/components/component.py

@@ -24,7 +24,7 @@
 from reflex_base.components.dynamic import load_dynamic_serializer
 from reflex_base.components.field import BaseField, FieldBasedMeta
 from reflex_base.components.tags import CommonTag, Tag
-from reflex_base.constants import Dirs, EventTriggers, Hooks, Imports, MemoizationMode
+from reflex_base.constants import Dirs, EventTriggers, Imports, MemoizationMode
 from reflex_base.constants.compiler import SpecialAttributes
 from reflex_base.event import (
     EventCallback,
@@ -1959,8 +1959,7 @@ def _get_ref_hook(self) -> Var | None:
         ref = self.get_ref()
         if ref is not None:
             return Var(
-                f"const {ref} = useRef(null); {Var(_js_expr=ref)._as_ref()!s} = {ref};",
-                _var_data=VarData(position=Hooks.HookPosition.INTERNAL),
+                f"const {ref} = useRef(null); {Var(_js_expr=ref)._as_ref()!s} = {ref};"
             )
         return None
 
@@ -1974,14 +1973,7 @@ def _get_vars_hooks(self) -> dict[str, VarData | None]:
         for var in self._get_vars():
             var_data = var._get_all_var_data()
             if var_data is not None:
-                vars_hooks.update(
-                    var_data.hooks
-                    if isinstance(var_data.hooks, dict)
-                    else {
-                        k: VarData(position=Hooks.HookPosition.INTERNAL)
-                        for k in var_data.hooks
-                    }
-                )
+                vars_hooks.update(dict.fromkeys(var_data.hooks))
                 for component in var_data.components:
                     vars_hooks.update(component._get_all_hooks())
         return vars_hooks
@@ -2016,7 +2008,7 @@ def _get_hooks_internal(self) -> dict[str, VarData | None]:
         result = {
             **self._get_events_hooks(),
             **{
-                str(hook): VarData(position=Hooks.HookPosition.INTERNAL)
+                str(hook): None
                 for hook in [self._get_ref_hook(), self._get_mount_lifecycle_hook()]
                 if hook is not None
             },

packages/reflex-base/src/reflex_base/components/memoize_helpers.py

@@ -175,7 +175,6 @@ def _var_data_key(data: VarData | None) -> tuple | None:
         data.imports,
         data.hooks,
         tuple(id(dep) for dep in data.deps),
-        data.position,
         tuple(id(component) for component in data.components),
         tuple((priority, id(component)) for priority, component in data.app_wraps),
     )

packages/reflex-base/src/reflex_base/components/state_context.py

@@ -11,7 +11,6 @@
 
 from reflex_base.components.component import Component
 from reflex_base.constants import Dirs
-from reflex_base.constants.compiler import Hooks
 from reflex_base.vars.base import VarData
 
 
@@ -62,7 +61,4 @@ def get_events_hooks_var_data() -> VarData:
     Returns:
         A new VarData carrying both providers as app_wraps.
     """
-    return VarData(
-        position=Hooks.HookPosition.INTERNAL,
-        app_wraps=get_event_app_wraps(),
-    )
+    return VarData(app_wraps=get_event_app_wraps())

packages/reflex-base/src/reflex_base/constants/compiler.py

@@ -189,7 +189,7 @@ class Hooks(SimpleNamespace):
     )
 
     class HookPosition(enum.Enum):
-        """The position of the hook in the component."""
+        """Deprecated: hooks render in the order they are declared and merged."""
 
         INTERNAL = "internal"
         PRE_TRIGGER = "pre_trigger"

packages/reflex-base/src/reflex_base/vars/base.py

@@ -46,7 +46,7 @@
 from reflex_base.constants.compiler import Hooks
 from reflex_base.constants.state import FIELD_MARKER
 from reflex_base.environment import _on_env_var_set, environment
-from reflex_base.utils import exceptions, imports, serializers, types
+from reflex_base.utils import console, exceptions, imports, serializers, types
 from reflex_base.utils.compat import MISSING_TYPE, annotations_from_namespace
 from reflex_base.utils.decorator import once
 from reflex_base.utils.exceptions import (
@@ -344,9 +344,6 @@ class VarData:
     # Dependencies of the var
     deps: tuple[Var, ...] = dataclasses.field(default_factory=tuple)
 
-    # Position of the hook in the component
-    position: Hooks.HookPosition | None = None
-
     # Components that are part of this var
     components: tuple[BaseComponent, ...] = dataclasses.field(default_factory=tuple)
 
@@ -378,7 +375,8 @@ def __init__(
             imports: Imports needed to render this var.
             hooks: Hooks that need to be present in the component to render this var.
             deps: Dependencies of the var for useCallback.
-            position: Position of the hook in the component.
+            position: Deprecated and ignored: hooks render in the order they
+                are declared and merged.
             components: Components that are part of this var.
             app_wraps: App-level wrapper components this var requires when used.
             field_dependencies: Every state field this var is built from,
@@ -387,6 +385,14 @@ def __init__(
                 single field. Keyword-only in practice: it trails the older
                 parameters so positional callers of those are unaffected.
         """
+        if position is not None:
+            console.deprecate(
+                feature_name="VarData(position=...)",
+                reason="Hooks render in the order they are declared and merged. "
+                "Remove the position argument.",
+                deprecation_version="0.10.1",
+                removal_version="1.0",
+            )
         if isinstance(hooks, str):
             hooks = [hooks]
         if not isinstance(hooks, dict):
@@ -402,7 +408,6 @@ def __init__(
         object.__setattr__(self, "imports", immutable_imports)
         object.__setattr__(self, "hooks", tuple(hooks or {}))
         object.__setattr__(self, "deps", tuple(deps or []))
-        object.__setattr__(self, "position", position or None)
         object.__setattr__(self, "components", tuple(components or []))
         object.__setattr__(self, "app_wraps", tuple(app_wraps or []))
 
@@ -418,7 +423,6 @@ def __init__(
                 object.__setattr__(self, "imports", merged_var_data.imports)
                 object.__setattr__(self, "hooks", merged_var_data.hooks)
                 object.__setattr__(self, "deps", merged_var_data.deps)
-                object.__setattr__(self, "position", merged_var_data.position)
                 object.__setattr__(self, "components", merged_var_data.components)
                 object.__setattr__(self, "app_wraps", merged_var_data.app_wraps)
 
@@ -508,21 +512,6 @@ def merge(*all: VarData | None) -> VarData | None:
             }.values()
         )
 
-        positions = list(
-            dict.fromkeys(
-                var_data.position
-                for var_data in all_var_datas
-                if var_data.position is not None
-            )
-        )
-        if positions:
-            if len(positions) > 1:
-                msg = f"Cannot merge var data with different positions: {positions}"
-                raise exceptions.ReflexError(msg)
-            position = positions[0]
-        else:
-            position = None
-
         components = tuple(
             component for var_data in all_var_datas for component in var_data.components
         )
@@ -536,7 +525,6 @@ def merge(*all: VarData | None) -> VarData | None:
             imports=imports_,
             hooks=hooks,
             deps=deps,
-            position=position,
             components=components,
             app_wraps=tuple(
                 (priority, wrapper) for (priority, _tag), wrapper in app_wraps.items()
@@ -554,7 +542,6 @@ def __bool__(self) -> bool:
             or self.imports
             or self.hooks
             or self.deps
-            or self.position
             or self.components
             or self.app_wraps
         )
@@ -582,7 +569,6 @@ def _identity_key(self) -> tuple:
             self.imports,
             self.hooks,
             tuple(dep._hash_key() for dep in self.deps),
-            self.position,
             tuple(id(component) for component in self.components),
             frozenset(
                 (priority, component.tag or type(component).__name__)

packages/reflex-components-core/news/+hook-declaration-order.bugfix.md

@@ -0,0 +1 @@
+`window_event_listener` and `clipboard` hooks now render in declaration order with the rest of the component's hooks instead of being moved after them.