snapview / sunrise Goto Github PK

We should probably add tsd to tests to ensure correct types.

The following discussion popped during code review. I copy it here to continue.

    renderFunction(val as T)

Michał:

pbraune Unsafe type casting as T. Maybe it is better to check if val === null? What if renderFunction actually expects null or other false value?

me:

Hi mklobukowski , null is not the issue. Please open it in you IDE to investigate.

map ensures, the argument passed to the function is T or in this this case T | null but null is already "eliminated" 2 lines above with val ? typeof renderFunction === 'function'....

The issue is that renderIf accepts cell as Value<T | null>. And Value is type Value<T> = T | Cell<T>. And since map became generic to accept Value<T> by itself.

If it's passed as Value TypeScript cannot decide whether it's Cell or just T. Because I don't like type casting and beleave that TypeScript should be able to handle this case correctly, I created a separate branch to investigate this "issue".
There is a test case and some "mess" to solve it. Any help is welcome.

Michał:

pbraune I see. I took a look at sunrise and it is harder than I expected. Let's use as T for now.

Btw, regarding the sunrise issue. I wonder if it isn't becoming too complicated just because we want to produce a Value instead of a Cell in the following code. This leads to UnwrapCell type which has troubles handling UnwrapValue<Value<T>> because it distributes result over the union. I have found no solution to that. But it led me to the question: Is there a significant gain by not returning a cell when all inputs aren't cells? If we always returned FormulaCell, there wouldn't be a need for UnwrapValue type at all.

export function formula<T>(
    fn: Function,
    ...sources: Value<unknown>[]
): FormulaCell<T> | T {
    if (sources.some(isCell)) {
        return new FormulaCell(fn, ...sources)
    } else {
        return fn(...sources)
    }
}

When returning FormulaCell always:

declare function myMap<V, R>(fn: (v: V) => R, input: Value<V>): FormulaCell<R>;

{
    function callIf<T, U>(
        cellOrValue: Value<T>,
        cb: (arg: T) => U
    ): U | null {
        const res = myMap((value) => {
            if (value) {
                return cb(value)
            }
            return null
        }, cellOrValue)

        return deref(res)
    }
}

If you add an DOM event handler to update a cell value whenever that event fires, it would be nice to remove that event handler (unsubscribe from that event) if the cell will be destroyed.