hooks icon indicating copy to clipboard operation
hooks copied to clipboard

Imperative Hook APIs

Open sugarmanz opened this issue 2 years ago • 2 comments

Been brainstorming a bit about imperative hook APIs:

var someController: SataController? = null
container.hooks.someController.tap("some controller") { sc -> 
    someController = sc
}
// ...
someController?.doSomething()

There is a little bit of overhead here for simply tracking the current value in a variable to use at a later point. It’s not terrible, but it get’s pretty bad pretty quick if you need something more nested:

var update: Update? = null
container.hooks.someController.tap("some controller") { sc ->
    sc?.hooks?.nestedController?.tap("nested controller") { nc ->
        nc?.hooks?.onUpdate?.tap("update") { _update ->
            update = _update
        }
    }
}

With Kotlin, we can do something called property delegation, essentially some syntactic sugar for consolidating logic around custom getters and setters. The idea, is that we can do something like:

val someController by container.hooks.someController()

Powered by something like this under the hood:

operator fun <T1> Hook1<T1, Unit>.invoke() = Capture(this)

class Capture<T1>(hook: Hook1<T1, Unit>) {

    private var current: T1? = null

    init {
        hook.tap("capture") { _, value: T1 ->
            current = value
        }
    }

    operator fun getValue(thisRef: Any?, property: KProperty<*>): T1 = current 
        ?: throw IllegalStateException("hook not currently active")
}

Relatively slim extension for capturing a hooks latest value. However, it falls short when we look at the nested hooks case. Because accessing the value of the captured hook is stateful, we cannot just add another capture:

val someController by container.hooks.someController()
val nestedController by someController.hooks.nestedController()

I thought a bit about how much effort it would take to delegate instance methods to the Capture instance itself, but I think that requires too much overhead in how hook instances are defined (would require an interface to delegate to, or some more code gen for duplicating and forwarding instance APIs) and wouldn’t really be user friendly, even if the end API would be somewhat nice:

val nested by container.hooks.someController().hooks.nestedController()

Instead, I’m currently looking into a callback model, much like how taps exist now, but would still enable the delegation API:

val nested by container.hooks.someController { hooks.nestedController }

And then for additional levels:

val update by container.hooks.someController { hooks.nestedController }.capture { hooks.onUpdate }

Thoughts? Really open to anything, as I’ve got some concerns about how this could be a dangerous pitfall for those who don’t understand hooks.. at least with tapping, it’s very clear that you’re responding when you need to. I’m not even sure how many valid use cases there are for imperatively introspecting the latest value of the hook outside the tap. Especially when you take into account different types of hooks, and how many of them are really pipeline/event based, rather than stateful sync hooks.

sugarmanz avatar Apr 17 '22 02:04 sugarmanz

I really like the concept, and found myself really wanting something similar when using tapable. Depending on the API that you settle on, I may end up including a JS variant to match.

One thing to be careful of though (and one that I frequently ran into) is how easy it is to introduce memory leaks when persisting data outside of the lifecycle of hook's parent.

Given the basic example:

var someController: SomeController? = null
container.hooks.someController.tap("some controller") { sc -> 
    someController = sc
}
// ...
someController?.doSomething()

if container were to be garbage collected, someController would still have a strong ref, which probably isn't desirable unless there's also cleanup code to reset it to null when container is torn down.

adierkens avatar Apr 17 '22 04:04 adierkens

Yeah, that's a great point. I think we'll just have to ensure that any stored state is contained within weak references:

class SimpleCapture<T1>(hook: Hook1<T1, Unit>?) {

    private var current: WeakReference<T1>? = null

    init {
        hook?.tap("capture") { _, value: T1 ->
            current = WeakReference(value)
        }
    }

    operator fun getValue(thisRef: Any?, property: KProperty<*>): T1 = current?.get()
        ?: throw IllegalStateException("hook not currently active")
}

Maybe with some better error handling to throw different errors for when it hasn't gotten a value yet or when the WeakReference value has gotten cleaned up. Or just have it return null when it isn't actually a valid value. I'm not actually sure what I like better. Encapsulating null as a state value is fine, but could be misrepresented if the hook value is nullable? Unless it's understood that null just means there isn't a current value here, which could be fine.

sugarmanz avatar Apr 18 '22 14:04 sugarmanz