reflex icon indicating copy to clipboard operation
reflex copied to clipboard

add deps and position field in VarData

Open Lendemor opened this issue 1 year ago β€’ 1 comments

When rendering hooks, some use case are not covered with the current hooks API.

This PR add two fields, deps and position to VarData that allow some controls on how to render hooks, both in relation to the useCallback generated by StatefulComponent.

Problem 1:

  • if you are writing a signature spec for an event, and this one depend on a hook you wrote in add_hooks, when wrapped in useCallback, your event handler will not update its data if the hook_name is not added to the dependency array. Solution: Passing the name of the hook in the new deps props.

Problem 2:

  • When adding a hook, you might either want:
    • to reference your hook inside an event handler (as a deps for example), so the hook need to be defined BEFORE.
    • to reference a memoized event handler inside your hook, so the hook need to be defined AFTER. (see clipboard.py) Solution: Inside a hook, pass either HooksPosition.PRE_TRIGGER or HooksPosition.POST_TRIGGER to the position props.

Lendemor avatar Dec 10 '24 23:12 Lendemor

i'm overall not crazy about packing more stuff in VarData because these fields do seem to have a pretty niche use in the framework... but i don't have an alternate suggestion that's better, so i think we can be pragmatic and take this.

I sort of agree on this, that's why at first I tried to pass the position to a HookVar with special logic, but in the end passing them through VarData made code much cleaner and easier to read.

Also even if they are niche, they won't impact the majority of people who don't need them so it should be fine.

Lendemor avatar Dec 13 '24 00:12 Lendemor