microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.08k stars 12.49k forks source link

Check property declarations in JS #27023

Open shrinktofit opened 6 years ago

shrinktofit commented 6 years ago

Recently I found it's helpful to add JSDOC style comment to hint my IDE the variable's type.

For example, If I add comment to a variable:

function fx(jsonAsset) {
   /** @type {import("a-dts-file-describes-the-type-of-the-json-object").Type} */
    const typedObject = jsonAsset.json;

    // IDE will give the member list of the type
    typedObject.
}

My IDE(Visual Studio Code Lastest) would correctly identify the type of typedObject. So that it give me a very nice intelligent prompt. Even when I enable the type check through // @ts-check, it will give the type errors.

This feature is helpful to me and my project. It's written in Javascript(although we will try to rewrite it using Typescript in future but not for now).

We are using Babel with a plugin called transform-class-properties means that we can declare class member in ES6 class just like Typescript does:

class C {
    /* @types {HTMLCanvasElement} */
    @decorator(/* ... */)
    canvas = null;
}

But when I add similar comment to canvas declaration, it can not be recognized by IDE. The IDE seems like only decides the type through its initializer. Could you support this feature please?

shrinktofit commented 6 years ago

Update: It seems more like a BUG?

class Test {
    /**
     * @type {HTMLCanvasElement}
     */
    _canvas; // IDE knows its type
}

and

class Test {
    /**
     * @type {HTMLCanvasElement}
     */
    _canvas = document.createElement('canvas'); // IDE knows its type
}

all take effect, but

class Test {
    constructor() {
        this._canvas = document.createElement('canvas'); // Because this assignment
    }
    /**
     * @type {HTMLCanvasElement}
     */
    _canvas; // IDE no longer knows its type, as "any"
}

doesn't take effects.

AlCalzone commented 6 years ago

Try this (might require typescript@next due to a recently fixed bug):

class Test {
    constructor() {
        /**
         * @type {HTMLCanvasElement}
         */
        this._canvas = document.createElement('canvas');
    }
}
shrinktofit commented 6 years ago

Try this (might require typescript@next due to a recently fixed bug):

class Test {
    constructor() {
        /**
         * @type {HTMLCanvasElement}
         */
        this._canvas = document.createElement('canvas');
    }
}

This works fine. However, how to turn

class Test {
    constructor() {
        this._canvas = document.createElement('canvas'); // Because this assignment
    }
    /**
     * @type {HTMLCanvasElement}
     */
    _canvas; // IDE no longer knows its type, as "any"
}

work?

shrinktofit commented 6 years ago

@andy-ms I think typescript itself can recognize the type but IDE can't. Given:

class Test {
    constructor() {
        this._canvas = document.createElement('canvas');
    }
    /**
     * @type {HTMLCanvasElement}
     */
    _canvas;
}

The following statement's error can be recognized, underlined by a wave line:

this._canvas = 0; // Error is occured once @ts-check is enabled:
                           // Cannot assign number 2 to type HTMLCanvasElement.

But:

shrinktofit commented 6 years ago

Update: It's fun:

Working:

image

Not Working:

image

The difference is the order of member declaration.

Source code:

class Test1 {
    constructor() {
        this._canvas = document.createElement('canvas');
    }

    /**
     * @type {HTMLCanvasElement}
     */
    _canvas;
}

class Test2 {
    /**
     * @type {HTMLCanvasElement}
     */
    _canvas;

    constructor() {
        this._canvas = document.createElement('canvas');
    }
}
ghost commented 6 years ago

@sandersn Above seems to be working now, but not:

class Test1 {
    x;
    constructor() {
        this.x = 0;
    }
}

which I think should succeed even in --noImplicitAny mode since we infer types from assignments in JS.

It looks like the class fields proposal allows fields without initializers. I think we should be able to infer a type in this case. It looks like currently in getTypeOfVariableOrParameterOrPropertyWorker we have separate paths for binary expressions and property declarations -- would be nice to merge those so we could handle cases like this.