Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Imperative Hook APIs #23

Open
sugarmanz opened this issue Apr 17, 2022 · 2 comments
Open

Imperative Hook APIs #23

sugarmanz opened this issue Apr 17, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@sugarmanz
Copy link
Collaborator

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 sugarmanz added the enhancement New feature or request label Apr 17, 2022
@adierkens
Copy link

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.

@sugarmanz
Copy link
Collaborator Author

sugarmanz commented Apr 18, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants