Removes unsafe export-API

[Lysander]

Problem

Until this PR, the export function was not safe to use:

class Exporter<T : Any>(initialize: Exporter<T>.() -> Unit) {
    lateinit var payload: T // this is bad and dangerous!

    fun export(payload: T): T {
        this.payload = payload
        return payload
    }

    init {
        initialize()
    }
}

As you can see there is the lateinit va payload: T, which gets only set, if export()-function gets calles in the initialize-Lambda.

This cannot be forced! And that leads to a brittle API :-(

Imagine some control structures like if, which prohibits the call if some value fails its condition. Then the outer export call will try to access the payload-field in order to return it. But it is still not initialized:

val something= false

export {
    if(something) {
        export("This wont get called always!")
    }
}.also { /* an exception would be thrown before this block. Hard to find the precise code section on top! */ }

Solution

We have decided to remove this function from fritz2!

It is simply not possible to reach for a good design, that would fit all kind of situations.

One idea could be to introduce some default-parameter to cover the case, where no export is called inside the Exporter-scope. But this would force people to always provide some senseless default value, that will never be used in the end. As this function is often used to return some Tag<*> from a nested UI-code section, one would have to create an "empty"-tag-object, which will be rendered inside the DOM anyways...

You can guess, there is no good default - that's probably why there is nothing like this built into Kotlin's standard lib ;-)

As you can see this is unfortunately API breaking, but there was no other solution.

Migration Guide

Thus said, it should be quite simple to bypass the use, by just

// with `export`
fun RenderContext.component(): HtmlTag<HTMLInputElement> = export {
    div(id = id) {
        export(
            input(id = inputId) {
                type("text")
            }
        )
    }
}

// without `export`
// before
fun RenderContext.component(): HtmlTag<HTMLInputElement> {
    lateinit var result // we are save to use this here, as the `input`-factory will always be called.
    div(id = id) {
        result = input(id = inputId) {
            type("text")
        }
    }
    return result
}

If you have code, where it is not guarantueed, that the var will be initialized, you cant use lateinit , but should rather use a "normal" var oder make the type of the var nullable like this:

// without `export`
fun RenderContext.component(flag: Boolean): HtmlTag<HTMLInputElement>? {
    var result: HtmlTag<HTMLInputElement>? = null // we not safe, that `input` will be called!
    div(id = id) {
        if(flag) {
            result = input(id = inputId) {
                type("text")
            }
        }
    }
    return result
}

fix #878

[haukesomm]

I just had a look at your PR and noticed the following: If a Tag is used as the default value, it will always be rendered, no matter whether it is actually needed.

As a fix, I would like to propose an overloaded export function with a getter function instead of a static value (or at least an option to have one):

fun export(default: () -> T, scope: Exporter<T>.() -> Unit): T

[Lysander]

@haukesomm Good one! exactly this lead to the solution to simpley remove this function! I have updated the PR (just deleting the files) and the PR comment to explain our decision.