Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional parameter with jsdoc does not be respected by salsa intellisense #6992

Closed
Thaina opened this issue Feb 10, 2016 · 7 comments · Fixed by #10671
Closed

Optional parameter with jsdoc does not be respected by salsa intellisense #6992

Thaina opened this issue Feb 10, 2016 · 7 comments · Fixed by #10671
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@Thaina
Copy link

Thaina commented Feb 10, 2016

When I use jsdoc to create constructor the intellisense work well. Except a bug that it must obey parameter. It will not know type if the parameter is not all passed in. even it marked as optional by jsdoc

screen shot 2559-02-10 at 12 02 13 pm

As the screenshot. air is optional by jsdoc

screen shot 2559-02-10 at 11 56 43 am

this work but

screen shot 2559-02-10 at 11 54 19 am

not

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Feb 12, 2016
@RyanCavanaugh RyanCavanaugh self-assigned this Feb 12, 2016
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 2.0 milestone Feb 12, 2016
@billti
Copy link
Member

billti commented Feb 12, 2016

Testing with the latest release-1.8 bits and we seem to have fixed this. Note the various different syntaxes supported for optional params below (p2 - p4 are all optional below):

screen shot 2016-02-12 at 11 28 34 am

Please try again with the latest bits in the release-1.8 branch, and if you are still not seeing optionality on params appear problem, comment with more details (ideally some code I can cut/paste), and I'll look into it.

Thanks!

@billti billti closed this as completed Feb 12, 2016
@Thaina
Copy link
Author

Thaina commented Feb 13, 2016

@billti I think you may misunderstand the issue a little

The problem is registering intellisense to constructor

Here is the code I use for test

/**
 * @param {string} p0
 * @param {string} [p1]
 */
function Test(p0, p1) {
    this.P0 = p0;
    this.P1 = p1;
}


var test = new Test("");

And this is result. When parameter is missing it then make an object created from that constructor to not know what it is

untitled-1

@Thaina
Copy link
Author

Thaina commented Feb 15, 2016

@RyanCavanaugh @billti If you close issue then does you get notification?

@billti
Copy link
Member

billti commented Feb 15, 2016

Yes, but it is the weekend (and Monday is a holiday).

@Thaina
Copy link
Author

Thaina commented Feb 15, 2016

@billti OK then I'm apologize. Just not sure that it is or not. Because it still closed

@RyanCavanaugh
Copy link
Member

Currently in TypeScript if you try to invoke a function (or constructor) and zero signatures match, we issue an error and return unknownType (which is effectively identical to any). This is the general pattern we use everywhere because it stops downstream errors.

However, if there are any appropriate signatures, we could give a better Salsa (and arguably a better TypeScript) experience by choosing the last signature in the set instead of unknownType. That would give you back what we generally believe to be the "most general" return type of the function, and you could start seeing downstream errors.

This is probably the right thing to do everywhere. e.g. today, even in TypeScript, if you say

function makeString(n: number) {
    return 'the number is ' + n;
}

let j = makeString(); // j: any
let k = j.subtr(4);

you get an error on the initializer for j, but nothing on the incorrect spelling of substr. That's bad for Salsa (as shown above) and bad for TypeScript because you really do have two errors (not one), and now need to go through two compilation cycles instead of one.

@ahejlsberg any thoughts on this change and/or this pattern in general?

@RyanCavanaugh RyanCavanaugh reopened this Feb 16, 2016
@RyanCavanaugh
Copy link
Member

Open question with this approach: what to do about generic signatures?

function fn<T>(x: T, y: T): T { ... }
let x = fn(3, ""); // was x: any, now x: ??

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants