js with window object fails to compile

[ChogyDan]

TypeScript Version: Version 2.0.0

Code

// from https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API/Using_the_Web_Storage_API
function storageAvailable(type) {
    try {
        var storage = window[type],
            x = '__storage_test__';
        storage.setItem(x, x);
        storage.removeItem(x);
        return true;
    }
    catch(e) {
        return false;
    }
}

Expected behavior:
This is a snippet of code from: https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API/Using_the_Web_Storage_API

Since this is sample javascript code, I was expecting it to work.

Actual behavior:
var storage gets typed as Window, and thus can't resolve setItem nor removeItem.

[ChogyDan]

FWIW, my environment, I'm editing with Visual Studio Code, and running Ionic 2(beta) from the cli. Both the ionic build and Visual Studio threw compile errors.

I'm not really sure what the error is exactly, so please edit the title as appropriate!

[yortus]

@ChogyDan the simplest fix is to annotate what the parameter type is allowed to be, like in the first line below (the rest is unchanged):

function storageAvailable(type: 'localStorage' | 'sessionStorage') {
    try {
        var storage = window[type];
        var x = '__storage_test__';
        storage.setItem(x, x);
        storage.removeItem(x);
        return true;
    }
    catch(e) {
        return false;
    }
}

[DanielRosenwasser]

Unfortunately we don't map string literals types to their property names (#6080), so @yortus' suggestion won't work fully.

You'll have to use a type assertion for storage:

let storage = window[type] as Storage;

[DanielRosenwasser]

TypeScript Version: I dunno, sorry

tsc --version

[yortus]

@DanielRosenwasser the version I posted does work fine and compiles too. The only difference is that storage gets typed as any instead of Window (if type is any then the compiler picks up the [index: number]: Window; declaration in the Window interface).

Typing storage as any seems OK here since the function is doing a runtime test for the existence of the methods. It storage were more strongly typed, it would give a false compile-time assurance that the methods existed, when they may not for some runtimes (which is the whole point of the function).

[ChogyDan]

Version updated, thanks. I also appreciate the recommended fixes for the code. @DanielRosenwasser is interesting because both sessionStorageand localStoragetype as Storage.

But for the sake of argument, explicitly typing storageas anyalso works, since that is what is supposed to be happening anyway. I feel like that typing the return type of window[type] as Window is not what typescript should be doing. I thought typescript was doing the "wrong" thing here, and that's why I filed a bug.

[yortus]

The surprising part as @ChogyDan mentions is this:

let a;
let b = window[a]; // type of b is 'Window' !?

This arises from the Window interface declaration in lib.d.ts which has the following indexer:

interface Window {
    ...
    [index: number]: Window;
}

I was going to suggest adding [index: string]: any to fix the OP case. However I tried adding it both before and after the numeric indexer but it didn't work. No matter which way I arranged the two indexers, indexing with any always used the numeric indexer and infered the result as Window.

[mhegazy]

i would say remove the indexer from Window all together.

[ChogyDan]

@mhegazy I looked up the indexer in the spec, and it looks like it is supporting window === window.frames and window.frames is an array (which... makes window an array as well). Indexing on window.frames gives a window object.

I thought removing the indexer might be a good idea, but I guess it won't follow the spec.

References:
Window object: https://developer.mozilla.org/en-US/docs/Web/API/Window
frames object: https://developer.mozilla.org/en-US/docs/Web/API/Window/frames
Discussion on how window===window.frames is confusing but the accepted standard for js: https://groups.google.com/forum/?hl=en#!topic/mozilla.dev.platform/VijG80aFnU8

[Download]

I wrote that specific code snippet on Mozilla.com. I have to admit I have never used TypeScript before.

My idea for the snippet was that people should be able to easily copy-paste and use it... So I'm sorry to hear that it's giving issues. If you have any suggestions for how to change that snippet so it would work out-of-the-box in both TypeScript and Javascript then please share them here.

[mhegazy]

i do not think there is something wrong with the snippet per se. the issue here is the DOM spec says that indexing into the window object with a number returns you a frame, in the snippet, the TS compiler does not know what is the type of type would be at run time, so it is safe to assume it can be a number, @yortus' suggession in #10457 (comment) is the ideal workaround here.

My proposal is to remove the index signature all together. I would expect ppl indexing into window with any more than indexing into it with number to get frames.

[ChogyDan]

Are we sure this isn't an issue with typescript itself? It seems that indexing with type any will default to using the number indexer, instead of what would make sense to me, the string indexer. I've written some sample code (in visual studio):

class NumberWasIndexed {
    makeNotAssignable: string;
}
class StringWasIndexed {}

class test {
    num1: number;
    string1: string;
    [number: number]: NumberWasIndexed;
    [string: string]: StringWasIndexed;
}
var aTest: test;

function someFunction ( all:           string | number | symbol | any,
                        withoutAny:    string | number | symbol,
                        _number:                number,
                        _string:       string,
                        _symbol:                         symbol,
                        _any:                                     any) {
    var stringIsIndexed = aTest[_string];
    var numIsIndexed = aTest[_number];
    numIsIndexed = aTest[all];  //Why is this using the number indexer?
    stringIsIndexed = aTest[withoutAny]; //and this one uses the string indexer.
    stringIsIndexed = aTest[_symbol];
    numIsIndexed = aTest[_any]; //same question
}

[mhegazy]

string | number | symbol | any === any. there is no difference, and there should not be, between all and _any.
The symbol one is a bug. i would say this should be an error. indexing with symbols is tracked by #1863
indexing with any, is the question here. numeric indexer is considered a subdomain of the string indexer. so the logic for look up goes like if there is a numeric indexer, and the argument is number like, then use it if not use the string indexer, otherwise it is any. so if you are indexing with any, it is as likely to be a number as it is to be a string. given that most objects do not have both indexers, using the most specific seems the correct thing to do. this way indexing in an array with any will return you the element type and not just any.

[yortus]

using the most specific seems the correct thing to do. this way indexing in an array with any will return you the element type and not just any.

@mhegazy I don't think there is one correct thing to do here. The current logic favours arrays over objects, leading to surprises like this issue.

I was expecting similar behaviour to how TypeScript handles overloads - i.e. declaration order matters. The following example is very similar to this issue except the indexers are overloaded functions. In this case, calling indexer with an argument of type any will pick whichever overload is declared first. That way the programmer can tweak the declaration to get the most appropriate behaviour for that case.

interface Window {
    indexer(string: string): any;
    indexer(number: number): Window;
}
let w: Window;
let prop: any;

// If we swap the overloads above, a would be Window
let a = window.indexer(prop);  // a is any (uses first overload)
let b = window.indexer('foo'); // b is any (regardless of overload order)
let c = window.indexer(42);    // c is Window (regardless of overload order)

[mhegazy]

The current logic favours arrays over objects, leading to surprises like this issue.

that is correct. except that non-array objects rarely have numeric indexers.

[Download]

So in this specific case, would changing window[type] to window['' + type] help?

[mhegazy]

yes. But ideally you would not need to do this.

[ChogyDan]

I was waiting for the "TypeScript gods" to make a declaration of how to resolve this dilemma: does TS need to change to accommodate more precise indexer definitions? or should the the .d.ts file be adjusted?

I realized that unless there are other abstractions that TS is trying to model that need complicated indexers, it shouldn't be changed. The window abstraction doesn't need to be the driver for this change. So removing the indexer seems like the right call.

I went ahead and researched how to do a pull request for this issue. You can see my work here: microsoft/TypeScript-DOM-lib-generator#146