Breaking change with TypeScript 1.3?

[johnnyreilly]

Hi all,

When upgrading a project from TypeScript 1.0 to TypeScript 1.3 I noticed the following Knockout code no longer compiled:

module WorkedPreTypeScript13 {

    enum ScreenMode { Edit, Details }

    export class ViewModel {

        private screenModeObservable: KnockoutObservable<ScreenMode> = ko.observable(null);

        private screenMode: KnockoutComputed<ScreenMode> = ko.computed({
            read: this.screenModeObservable, // Dead in the water
            write: (mode: ScreenMode) => {

                // Do other stuff

                // Update observable
                this.screenModeObservable(mode);
            }
        });
    }
}

Just to clarify, my computed needs to contain a read method which looks like this: read(): T; where T is the enum ScreenMode. Previously I just dropped in the KnockoutObservable<ScreenMode> which when () produces T where T is again the enum ScreenMode. i.e. It matches.

With the 1.3 compiler I get this error:

Type 'KnockoutComputed<{}> is not assignable to type 'KnockoutComputed': Type '{}' is not assignable to type 'ScreenMode'

I don't think this is a problem with the Knockout typings as best I understand them. I have extracted the relevant parts of the typings below:

interface KnockoutComputedStatic {
    fn: KnockoutComputedFunctions<any>;

    <T>(): KnockoutComputed<T>;
    <T>(func: () => T, context?: any, options?: any): KnockoutComputed<T>;
    <T>(def: KnockoutComputedDefine<T>, context?: any): KnockoutComputed<T>;
}

interface KnockoutObservable<T> extends KnockoutSubscribable<T>, KnockoutObservableFunctions<T> {
    (): T;
    (value: T): void;

    peek(): T;
    valueHasMutated?:{(): void;};
    valueWillMutate?:{(): void;};
    extend(requestedExtenders: { [key: string]: any; }): KnockoutObservable<T>;
}

interface KnockoutComputedDefine<T> {
    read(): T;
    write? (value: T): void;
    disposeWhenNodeIsRemoved?: Node;
    disposeWhen? (): boolean;
    owner?: any;
    deferEvaluation?: boolean;
    pure?: boolean;
}

interface KnockoutStatic {
    observable: KnockoutObservableStatic;

    computed: KnockoutComputedStatic;
}

declare module "knockout" {
    export = ko;
}

declare var ko: KnockoutStatic;

I'm able to work round the issue quite simply by wrapping screenModeObservable in it's own function as follows:

module WorksWithTypeScript13 {

    enum ScreenMode { Edit, Details }

    export class ViewModel {

        private screenModeObservable: KnockoutObservable<ScreenMode> = ko.observable(null);

        private screenMode: KnockoutComputed<ScreenMode> = ko.computed({
            read: () => this.screenModeObservable(),
            write: (mode: ScreenMode) => {

                // Do other stuff

                // Update observable
                this.screenModeObservable(mode);
            }
        });
    }
}

So is this a bug that arrived with 1.3? Or is this intentional?

If intentional is there anything we could better do from a typings perspective to cater for the original, and now problematic, pattern?

[IanYates]

I've been using KO and TypeScript for months and I've just always written my ko.computed read functions the way you ended up with as your workaround. I guess that's because I'm usually doing more than just passing through an observable unchanged in a computed (it's touching several of them, or checking for some condition and varying its result).

TypeScript has been happy to treat the KO computed as a function - since it is one. +1 for marking this as a bug.

If it helps track down the cause, I've just tried an uglier way of fixing it which the compiler seems to be happy with...

module WorksWithTypeScript13 {

    enum ScreenMode { Edit, Details }

    export class ViewModel {

        private screenModeObservable: KnockoutObservable<ScreenMode> = ko.observable(null);

        private screenMode = ko.computed({
            read:  <()=>ScreenMode> (this.screenModeObservable),
            write: (mode: ScreenMode) => {

                // Do other stuff

                // Update observable
                this.screenModeObservable(mode);
           }
        });
    }
}

The :KnockoutComputed is superfluous in all cases I tried (not commenting on readability, just saying the compiler doesn't use it, as far as I'm aware, to help infer what happens on the other side of the variable assignment).

Nice find!

[johnnyreilly]

Oh yeah - makes sense that a type assertion would totally work there. Interestingly this approach would result in less code (as there is no longer the need for wrapping function). It's a shame that you lose a bit on the readability front with this approach though... (too many <> and () for my liking)

[danquirk]

My first thought was this is the same as the other recent Knockout related issue but I think it's actually different. Need to investigate further and reduce the repro a bit.

[IanYates]

I've just updated to the latest language service and compiler (commit: fd229a9d99).
Since doing so, I lose type inference on KO computed functions.

This

var x = ko.computed(()=> {
    if (1 == 2) { return; }
    return 'abc';
});

correct reports its type as KnockoutComputed<string>;

This

var y = ko.computed({
    read:() => {
        if (1==2) { return; }
        return 'abc';
});

reports its type as KnockoutComputed<any>

This was fine with the latest TypeScript release (v1.3) in VS2013.

I'm trying out union types so I followed the instructions in issue #1110 to replace the compiler and language service. This is the first problem I've come across, but it's a big one (for me).

[IanYates]

Hmmm... I've investigated a bit more

Here's a relevant definition

interface KnockoutStatic {
    ...
    computed: KnockoutComputedStatic;
}

Then we have

interface KnockoutComputedStatic {
    fn: KnockoutComputedFunctions<any>;

    <T>(): KnockoutComputed<T>;
    <T>(func: () => T, context?: any, options?: any): KnockoutComputed<T>;
    <T>(def: KnockoutComputedDefine<T>, context?: any): KnockoutComputed<T>;
    (options?: any, context?: any): KnockoutComputed<any>;
}

I've found that the TypeScript compiler, as of the commit I'm using, is matching against the (options?: any, context?: any): KnockoutComputed<any>; rather than the <T>(def: KnockoutComputedDefine<T>, context?: any): KnockoutComputed<T>; line for my y variable.

Commenting out the (options?: any, context?: any): KnockoutComputed<any>; line fixes my problem. I'm not relying on that method signature anywhere in my codebase so excluding it is fine. It is a change in behaviour between TypeScript compiler versions, not necessarily a bug though. So I'm now using this definition

interface KnockoutComputedStatic {
    fn: KnockoutComputedFunctions<any>;

    <T>(): KnockoutComputed<T>;
    <T>(func: () => T, context?: any, options?: any): KnockoutComputed<T>;
    <T>(def: KnockoutComputedDefine<T>, context?: any): KnockoutComputed<T>;
    //(options?: any, context?: any): KnockoutComputed<any>;
}

An issue still remains - one that's bugged me for a while and is semi-related to the initial post of this issue. That is, in the code

var z = ko.computed({
    read: () => {
        if (1 == 2) {
            return;
        }
        return 'abc';
    },
    write: value=> {

}

intellisense and the TS language services see the type of value as {} rather than as string in the write function. That's fixed by doing

var z = ko.computed({
    read: () => {
        if (1 == 2) {
            return;
        }
        return 'abc';
    },
    write: (value:string)=> {

}

or

var z = ko.computed<string>({
    read: () => {
        if (1 == 2) {
            return;
        }
        return 'abc';
    },
    write: value=> {

}

[RyanCavanaugh]

I can't reproduce this in 1.3. Can you post a self-contained code block that shows this behavior?

[Igorbek]

@RyanCavanaugh

/// <reference path="typings/knockout/knockout.d.ts" />

var computed = ko.computed({
    read: () => "test",
    write: (value) => { }
});  // inferred as KnockoutComputed<{}> instead of KnockoutObservable<string>

I created reproducing code deferred from KO:

interface X<T> {
    thisDefineTheType: () => T;  // by this callback TS should infer T
    thisUsesTheType: (v: T) => T;
}

var f: <T>(options: X<T>) => T;

var result = f({ thisDefineTheType: () => "", thisUsesTheType: v => v });  // result/T is inferred as {}

[danquirk]

That repro is the same as the issue in #1181.

[JsonFreeman]

@johnnyreilly In general, when coming up with workarounds for these issues, I recommend turning to explicit type arguments, rather than a type assertion. Type assertions allow you to do something unsafe, which is often not necessary for these issues.