Avalonia.FuncUI
Avalonia.FuncUI copied to clipboard
Ensuring unique hook identity for custom hooks (v2)
More elegant than what I did in https://github.com/fsprojects/Avalonia.FuncUI/pull/207 .
Basically this allows library authors to create composed hooks without explicitly giving each hook a unique identity.
Note that this is a (small) breaking change.
@JordanMarr can you take a look?
Very nice! I should have some time to look at this in more detail on Wed morning.
After looking at your PR, I have some questions.
First, when do I, as an end user, need to be cognizant of line numbers? Is it anytime I am creating a hook, or does this only apply to "composed hooks"?
So then let's say I make a useCounter
hook that encapsulates a useState
hook; is that a composed hook?
Note: this example is still based off of the master branch. Also, I added the ?identity
arg only because I copied and pasted from another hook. Should I have used it somewhere in my hook? Does your PR make it unnecessary?
module Examples.CounterWithCustomHook
open Avalonia.Layout
open Avalonia.Controls
open Avalonia.FuncUI
open Avalonia.FuncUI.DSL
open Avalonia.FuncUI.Types
[<AutoOpen>]
module CustomHook =
open System.Runtime.CompilerServices
type IComponentContext with
member ctx.useCounter(initialCount: int, [<CallerLineNumber>] ?identity: int) =
let counter = ctx.useState(initialCount)
let increment () = counter.Set(counter.Current + 1)
let decrement () = counter.Set(counter.Current - 1)
counter.Current, increment, decrement
let cmp () = Component (fun ctx ->
let count, increment, decrement = ctx.useCounter 0
Grid.create [
Grid.rowDefinitions "20"
Grid.columnDefinitions "*, 80, *"
Grid.children [
StackPanel.create [
Grid.column 1
Grid.row 1
StackPanel.children [
// Increment
Button.create [
Button.content "▲"
Button.horizontalAlignment HorizontalAlignment.Stretch
Button.horizontalContentAlignment HorizontalAlignment.Center
Button.onClick (fun _ -> increment ())
]
// Current count
TextBlock.create [
TextBlock.text (string count)
TextBlock.horizontalAlignment HorizontalAlignment.Center
]
// Decrement
Button.create [
Button.content "▼"
Button.horizontalAlignment HorizontalAlignment.Stretch
Button.horizontalContentAlignment HorizontalAlignment.Center
Button.onClick (fun _ -> decrement ())
]
]
]
]
]
:> IView
)
@JordanMarr excellent questions.
Every state hook is identified in the component context by its caller line number vs. the order in which they are called in react. This makes is possible to render hooks conditionally, because we don't use the order (which would change) to store/restore the hooks state next time we render.
The problem with this approach is that if a hook is not directly called in the component's render function the caller line number of the function in which the hook is called will be used.
In the example below the caller line number for the let counter = ctx.useState(initialCount)
never changes. This results in all ctx.useCounter
hooks pointing at the same state when used in a component.
module Main =
open Avalonia.Controls
open Avalonia.FuncUI
open Avalonia.FuncUI.DSL
open Avalonia.Layout
[<AutoOpen>]
module CustomHook =
open System.Runtime.CompilerServices
type IComponentContext with
member ctx.useCounter(initialCount: int) =
let counter = ctx.useState(initialCount)
let increment () = counter.Set(counter.Current + 1)
let decrement () = counter.Set(counter.Current - 1)
counter.Current, increment, decrement
let view =
Component
(fun ctx ->
// increment / decrement on this state hook modify state of count below.
let _, increment, decrement = ctx.useCounter(0)
let count, _, _ = ctx.useCounter(0)
DockPanel.create [
DockPanel.children [
Button.create [
Button.onClick (fun _ -> decrement ())
Button.content "-"
]
Button.create [
Button.onClick (fun _ -> increment ())
Button.content "+"
]
TextBlock.create [
TextBlock.text (string count)
]
]
]
)
This can be solved by passing the caller line number like this:
[<AutoOpen>]
module CustomHook =
open System.Runtime.CompilerServices
type IComponentContext with
member ctx.useCounter(initialCount: int, [<CallerLineNumber>] ?identity: int) =
let counter = ctx.useState(initialCount, identity.Value)
let increment () = counter.Set(counter.Current + 1)
let decrement () = counter.Set(counter.Current - 1)
counter.Current, increment, decrement
This brings us to the problem this PR is trying to solve: multiple state hooks in a custom hook.
[<AutoOpen>]
module CustomHook =
open System.Runtime.CompilerServices
type IComponentContext with
member ctx.useCounter(initialCount: int, [<CallerLineNumber>] ?identity: int) =
let counterA = ctx.useState(initialCount, identity.Value) // <- can't be the same
let counterA = ctx.useState(initialCount, identity.Value) // <- can't be the same
counterA, counterB
Right now you need to manually add a identity suffix, with this PR you can do this:
[<AutoOpen>]
module CustomHook =
open System.Runtime.CompilerServices
type IComponentContext with
member ctx.useCounter(initialCount: int, [<CallerLineNumber>] ?identity: int) =
ctx.useGrouped (identity.Value, fun ctx ->
let counterA = ctx.useState(initialCount) <- implicitly get's set to `identity.{42} assuming this is line 42`
let counterA = ctx.useState(initialCount) <- implicitly get's set to `identity.{43}`
counterA, counterB
)
@JordanMarr not sure if the additional complexity of using the caller line number is worth it vs. using the calling order (like react). We might want to consider switching to the react way.
@JordanMarr not sure if the additional complexity of using the caller line number is worth it vs. using the calling order (like react). We might want to consider switching to the react way.
I honestly do think conditional hooks are a useful thing, I'm doing React most of my dev day and there's a bunch of situations when you want a hook with side effects to be executed under a certain condition and having to go through all the trouble of setting up a new component just to be able to conditionally call a hook is a hassle. I haven't seen the code of this PR yet, but unless it's an incredibly complex logic to allow this or it affects performance greatly I'd say it's a good thing to have.
Sorry for the delay, it's been a busy couple of weeks.
That's definitely an improvement!
My only concern is that the concept of passing a line number at all is hook internal implementation details leaking. If line numbers are confusing to me, they will be confusing to others too.
Is it fair to say that the tradeoff is: no conditional hooks and line numbers or conditional hooks with no line numbers? (EDIT: I had it backwards)
If so, conditional hooks would require a lot less knowledge -- only one simple rule for end users to remember: "no conditional hooks!"
Approved (with comment that it will be confusing to users)
I agree with @JordanMarr and think we should move to simpler calling order
based identity. Also think that conditional hooks are sometimes pretty useful @sleepyfran but we could still allow explicit hook identity to enable this to more experienced users if we really wanted.
I'm on vacation starting from today, but think the "a hooks identity is determined by the caller order" is pretty simple to implement - and what we should do IMO.
That sounds like a best of both worlds approach. The common scenario “just works” if you follow React rules and avoid conditional hooks, but a line number can be passed for more niche, advanced cases.
Closed in favour of #214