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

In JSDoc, `?` of conditional is frequently parsed as postfix-`?` #27424

Open
alexander255 opened this issue Sep 28, 2018 · 5 comments
Open

In JSDoc, `?` of conditional is frequently parsed as postfix-`?` #27424

alexander255 opened this issue Sep 28, 2018 · 5 comments
Labels
Bug
Milestone

Comments

@alexander255
Copy link

@alexander255 alexander255 commented Sep 28, 2018

TypeScript Version: 3.2.0-dev.20180927
Search Terms: JSDoc conditional types

Using Conditional Types in JSDoc comments confuses the TypeScript parser since the T extends Y ? A : B syntax looks similar to the Y? (meaning Y|null) syntax from JSDoc.

Code

/**
 * @template {{}} T
 * @param {T} o
 * @returns {T extends Readonly<infer U> ? (keyof U)[] : string[]}
 */
function Object_keys(o) {
	let keys = Object.keys(o);
	// @ts-ignore: Type assertion for stripping `Readonly<…>`
	return keys;
}

Expected behavior:

No error should be reported.
The type of Object_keys should be: <T extends {}>(o: T): T extends Readonly<infer U> ? (keyof U)[] : string[].

Actual behavior:

TypeScript compiler shows an error ("?" expected) and mistakes the existing ? operator for JSDoc <type>|null syntax.

The actual type therefor ends up being: <T extends {}>(o: T): T extends Readonly<infer U> | null ? (keyof U)[] : string[].

Possible fixes:

  1. When encountering a top-level (not within parenthesis) ? token in an extends clause, scan ahead and check whether it is followed by an | or & token (type intersection or union operators). If yes, treat it as |null and keep processing; otherwise, assume it's the start of a conditional type declaration. (Requires at least an LF(2) parser.)
  2. Prohibit the JSDoc ? operator in the top-level of an extends clause, always causing it to be treated as the start of the conditional type declaration.

Playground Link: None, Playground does not seem to support JavaScript with JSDoc instead of TypeScript as input.

@alexander255 alexander255 changed the title TypeScript compiler gets confused by TypeScript compiler gets confused by `?` operator when parsing conditional types Sep 28, 2018
@RyanCavanaugh RyanCavanaugh added the Bug label Sep 28, 2018
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.2 milestone Sep 28, 2018
@sandersn

This comment has been minimized.

Copy link
Member

@sandersn sandersn commented Sep 28, 2018

  1. Forbid conditional types in JSDoc. Postfix-? support is more important.

(2) is probably the best option if the implementation isn't too messy.

@noinkling

This comment has been minimized.

Copy link

@noinkling noinkling commented Apr 28, 2019

I also just ran into this bug (v3.4.3), was very confused about how the null got in there until I found this issue.

@sandersn

This comment has been minimized.

Copy link
Member

@sandersn sandersn commented May 20, 2019

After discussion with the JSDoc people, who talked to Closure people, here's option (4):

  1. Forbid postfix-? support. It is rarely used and was only included in Closure as legacy syntax.
@sandersn sandersn changed the title TypeScript compiler gets confused by `?` operator when parsing conditional types In JSDoc, `?` of conditional is frequently parsed as postfix-`?` May 20, 2019
@sandersn sandersn modified the milestones: TypeScript 3.5.0, Backlog May 20, 2019
@brendankenny

This comment has been minimized.

Copy link
Contributor

@brendankenny brendankenny commented May 20, 2019

👍 for (4) Forbid postfix-? support. It's confusing having both pre and post ? variants anyway, especially because in the postfix position it starts looking a lot like a ts optional property.

@goodmind

This comment has been minimized.

Copy link

@goodmind goodmind commented Jun 15, 2019

Found same problem when reprinting AST from jsdoc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.