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

JsDoc annotations don't play well with arguments destructuring #19645

Closed
KtorZ opened this issue Nov 1, 2017 · 17 comments
Closed

JsDoc annotations don't play well with arguments destructuring #19645

KtorZ opened this issue Nov 1, 2017 · 17 comments
Assignees
Labels
Bug A bug in TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation Fixed A PR has been merged for this issue Help Wanted You can do this

Comments

@KtorZ
Copy link

KtorZ commented Nov 1, 2017

Code

As stated in this comment (I am afraid this won't be seen since the issue is closed, hence the issue):

/**
 * @param {Object} args 
 * @param {string} args.prop1
 * @param {string} args.prop2
 */
function patate({ prop1, prop2 }) {

}

Expected behavior:

Type-checking this using --allowJs and --checkJs shouldn't result in an error but rather apply the right type-annotations to the both properties.

Actual behavior:
The compiler raises the following error:

error TS8024: JSDoc '@param' tag has name 'args', but there is no parameter with that name.
@ghost
Copy link

ghost commented Nov 1, 2017

Might be fixed by #18832 that would allow us to do the matching positionally.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 1, 2017

Duplicate of #7429

@mhegazy mhegazy added the Duplicate An existing issue was already created label Nov 1, 2017
@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@KtorZ
Copy link
Author

KtorZ commented Jan 12, 2018

@mhegazy You mentioned earlier that this was a duplicate of #7429 and #7429 is now labeled "fixed". Nevertheless, the bug I described earlier is still present in 2.6.2 and the latest nightly (2.7.0-dev.20180112).

:/ Any idea ?

@mhegazy mhegazy added Bug A bug in TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation and removed Duplicate An existing issue was already created labels Jan 12, 2018
@mhegazy mhegazy reopened this Jan 12, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jan 12, 2018

you are right, the fix in #7429 did not address this case as well.

@mhegazy mhegazy added the Help Wanted You can do this label Jan 12, 2018
@que-etc
Copy link

que-etc commented Feb 20, 2018

@KtorZ

There is a workaround which we are using in our project. I don't really know if it works for you, as the following approach can't be used with function declarations, but anyway:

/**
 * @type {function({prop1: string, prop2: string}): void}
 */
const patate = ({prop1, prop2}) => {
    // ...
}

/**
 * This definition works as well.
 * @type {(args: {prop1: string, prop2: string}) => void}
 */
const patate = ({prop1, prop2}) => {
    // ...
}

It's certainly not as legible as describing every property in it's own @param annotation, though you can move object's definition to its own @typedef.

And there is one major pitfall of which you should know. TypeScript currently has a bug (#22080) which allows to invoke former function without any arguments:

/**
 * @type {function({prop: string}): void}
 */
const func = ({prop}) => {
    // ...
};

// TypeScript should raise an error for the following
// line ('Expect 1 argument, but got 0'), but unfortunately it doesn't. 
func();

I hope it helps.

@KtorZ
Copy link
Author

KtorZ commented Feb 20, 2018

Thanks. For now I've been simply doing the destructuring in the function body and it works fine, though I'd rather cut off this extra line... Perhaps I can apply your solution in some cases :/

/**
 * @param {Object} args 
 * @param {string} args.prop1
 * @param {string} args.prop2
 */
function patate(args) {
  const { prop1, prop2 } = args;
} 

@ghost
Copy link

ghost commented Mar 16, 2018

We definitely need this. If I can help somehow let me know guys.

@Madd0g
Copy link

Madd0g commented Mar 30, 2018

I've seen multiple discussions about this and people seem to be pleased it's working, but with version 2.7.2 even the simple version doesn't work for me:

/**
 * @param {Object} args 
 * @param {string} args.prop1
 * @param {string} args.prop2
 */
function patate(args) {
  const { prop1, prop2 } = args;
} 

With allowJs and checkJs, I get JSDoc '@param' tag has name 'prop1', but is no parameter with that name.

Which TS version should have the dotted parameters notation supported (with no desctructuring in the the parameters)?

@KtorZ
Copy link
Author

KtorZ commented Mar 30, 2018

This "hack" used to work for me in (2.7.0-dev.20180112) So I'd say somewhere between 2.7.0 & 2.7.1

@sandersn
Copy link
Member

@Madd0g I can't repro your bug with master. Can you try typescript@next? I've fixed a lot of bugs in jsdoc recently, so it's likely that it was recently fixed.

@Madd0g
Copy link

Madd0g commented Mar 31, 2018

Huh, interesting, some of my code has this non-standard syntax for optional/required, it's really weird because every google search I do now specifies a different syntax for optional/required, so I don't know how I settled on this back when I wrote it:

/**
 * @param {!object} args 
 * @param {!string} args.prop1
 * @param {?string} args.prop2
 */
function patate(args) {
  const { prop1, prop2 } = args;
} 

With this syntax typescript@next still gives that error, but since I can't find any docs that mention this syntax, I guess it's just wrong?

Yes, the code from my previous comment works perfectly for me in typescript@next.

Thanks.

@sandersn
Copy link
Member

sandersn commented Apr 2, 2018

@Madd0g yep, jsdoc is a giant mess. Typescript actually understands that optional and required syntax, it just doesn’t expect it in conjunction with the multi-line literal type syntax. In other words, the parser is looking for exactly object on the first line, not !object.

Note that required doesn’t actually mean anything to the compiler, since structNullChecks: true means that everything is required by default and strictNullChecks: false means the compiler allows null/undefined everywhere.

You might want to open a separate bug for your case, actually.

@steph643
Copy link

steph643 commented Jun 6, 2018

As far as I can tell, the issue is still partially there (VSCode 1.23.1).

See the number type becoming any and the missing param comment:

image

image

Strangely enough, if you omit the Object, you get the param comment back (but not the type):

image

@steph643
Copy link

steph643 commented Jun 6, 2018

VSCode 1.24, released a few hours ago, is much better at this:

  • param types are now parsed correctly,
  • it seems param comments are still missing.

@sandersn
Copy link
Member

sandersn commented Jun 6, 2018

@steph643 can you file a separate bug for the param comments?

@steph643
Copy link

steph643 commented Jun 7, 2018

@sandersn : done, see above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation Fixed A PR has been merged for this issue Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

7 participants