Improve WebStorage API

[Dudeplayz]

Describe your motivation

I'm programming in kotlin and heavily using coroutines. The WebStorage API only defines static functions with callbacks, which can only be extended as suspending in Kotlin with:

suspend fun getWebstorageItem(key: String): String? {
    return suspendCoroutine { continuation ->
        WebStorage.getItem(key) { continuation.resume(it) }
    }
}

Because these are static methods I couldn't do:

suspend fun WebStorage.getItem(key: String): String? {
    return suspendCoroutine { continuation ->
        getItem(key) { continuation.resume(it) }
    }
}

Describe the solution you'd like

That's why I suggest to add overload methods that return a CompletableFuture instead of taking a callback. This way I can easily await on the CompletableFuture. Like:

    public static CompletableFuture getItem(String key) {
        // Impl Detail
    }

     WebStorage.getItem("key").await()

This way also other reactive frameworks would benefit from it.
The API could be expanded like this without breaking the existing one.

Describe alternatives you've considered

Also I would suggest to refactor the API to use objects. So that the calls could be:

   WebStorage.getLocalStorage().setItem("key")
   WebStorage.getSessionStorage().setItem("key")
   
   WebStorage.getLocalStorage().getItem("key")
   WebStorage.getSessionStorage().getItem("key")

For spsecific UI:

   UI ui = UI.getCurrent()
   WebStorage.getLocalStorage(ui).setItem("key")
   WebStorage.getSessionStorage(ui).setItem("key")
   
   WebStorage.getLocalStorage(ui).getItem("key")
   WebStorage.getSessionStorage(ui).getItem("key")

Or as alternative construct it directly as object:

   new LocalStorage(UI.getCurrent()).getItem("key")
   new SessionStorage(UI.getCurrent()).getItem("key")   

All my suggestions would allow to keep the current static approach without breaking.

[Legioth]

Agreed that CompletableFuture would be a great match for this API. This could easily be added to the existing API in a backwards compatible way.

The one gotcha is that directly blocking on that instance (i.e. using get or join) would not work due to the way updates cannot be received from the server while the session is locked. But this is no different from the executeJs return value and for that one we're reducing the impact with the help of our own DeadlockDetectingCompletableFuture which throws IllegalStateException for some trivial patterns.

[Dudeplayz]

Agreed that CompletableFuture would be a great match for this API. This could easily be added to the existing API in a backwards compatible way.

@Legioth ok, so favouring this instead of the other suggested approaches?

Should I create a PR, or are you guys doing it?

[Legioth]

Nobody on our side is looking into this right now so feel free to create a PR if you're interested!

[Legioth]

And yes, preferring to just introduce overloads without a callback and instead returning a CompletableFuture. Any option based on using instance methods instead of static methods would lead to spreading out the API surface into multiple locations. CompletableFuture also has the benefit that it integrates nicely with lots of other solutions in addition to Kotlin coroutines.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.5.0.