fix: render hooks in declaration order instead of by HookPosition

Merged
FA
12d ago
+183-135

Closes #7584.

Problem

VarData keeps hooks in an ordered tuple, and it merges each hook's dependencies before the hook. _sort_hooks then split the hooks into INTERNAL, PRE_TRIGGER and POST_TRIGGER buckets, which broke that order. Two parts of the code lose a hook's position:

  • VarData.merge cannot keep two different positions.
  • _get_added_hooks writes code[sub_hook] = None, which overwrites the INTERNAL position that the component's own prop hooks gave the same hook.

So a hook can render before a declaration it reads. This happens on main with one component: a prop with a derived hook, plus an add_hooks Var that reads the same state:

const derived = s.a_rx_state_ + 1        // INTERNAL
const s = useStateContext("...")         // overwritten to PRE_TRIGGER
const added = s.a_rx_state_

Fix

  • _render_hooks joins the hooks in the order they were declared and merged. _sort_hooks and the unused memo slot are removed.
  • VarData no longer has a position field. VarData(position=...) still works, but it shows a deprecation warning and has no effect. Hooks.HookPosition stays, and its docstring marks it as deprecated.
  • The framework's own position uses are removed: the ref, mount, var and event-loop hooks, window_event_listener and clipboard.

Tests

  • test_hooks_render_after_the_state_hook_they_read: the case above. It fails on main.
  • test_hooks_render_after_their_dependencies: random hook graphs (25 seeds), read through props and through add_hooks across a random component tree. Each hook must render after the hooks it reads. 11 of the 25 seeds fail on main.
  • test_var_data_position_is_deprecated_and_ignored: the argument warns, and hooks with different positions now merge in order instead of raising.

Validation outside the unit tests

I compiled each app on main and on this branch, ran ESLint no-use-before-define on every generated module, and compared each component's hook sequence:

AppModulesuse-before-define (main / branch)Components with a changed hook order
examples/playground580 / 01 of 335
a fixture with every hook-adding component220 / 02 of 18
a small shop app240 / 00 of 52
reflex-enterprise dnd + mantine demos260 / 00 of 21

The three reorders do not change behaviour: the state declaration still comes before every reader, and a useEffect moves before some useRef declarations. The same ESLint check finds the error in #7583's head that was reported there, so the check can detect this bug.

Prod builds of playground (20 routes) and the fixture load with no page or console errors. On the fixture, window_event_listener (resize), clipboard (paste) and form submit work.

The enterprise dnd and combobox code passes position= and now gets the deprecation warning. Its compiled output does not change.

Follow-ups

  • #7583 rebases on this and needs no position handling. Stacked on this branch, the reproducer from #7583 gives the merged useStateContext first.
  • reflex-enterprise can remove its position= arguments.