Skip to content

In JSDoc, parse postfix-? below conditional types/tuple types #39123

New issue

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

By clicking “No 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? No Sign in to your account

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

sandersn
Copy link
Member

Outside of JSDoc comments, postfix-? is parsed at lower precedence than the ? of conditional types, and a postfix-? inside a tuple type results in the type being marked optional.

This PR changes JSDoc parsing to behave the same way, which means that

  1. Conditional types are allowed in JSDoc. Fixes JSDoc: error TS1005 when using conditional type in @typedef #37166.
  2. Tuple types' postfix-? syntax is interpreted correctly in JSDoc. Fixes JSDoc tuple typedef with optional elements became null #38747.

The breaking change is that a postfix-? type followed by another postfix type, like [] or !, is parsed as a conditional type. Postfix-? is not common, so this is an acceptable breaking change.

A postfix-? type T? is still parsed everywhere else and treated as T | null.

Outside of JSDoc comments, postfix-? is parsed at lower precedence than
the `?` of conditional types, and a postfix-? inside a tuple type
results in the type being marked optional.

This PR changes JSDoc parsing to behave the same way, which means that

1. Conditional types are allowed in JSDoc. Fixes #37166.
2. Tuple types' postfix-? syntax is interpreted correctly in JSDoc.
Fixes #38747.

The breaking change is that a postfix-? type followed by another postfix type,
like `[]` or `!`, is parsed as a conditional type. [Postfix-? is not
common](#37166 (comment)),
so this is an acceptable breaking change.

A postfix-? type `T?` is still parsed everywhere else and treated as `T | null`.
@sandersn
Copy link
Member Author

@typescript-bot user test this -- let's see if any of Google's open source uses postfix-?

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 17, 2020

Heya @sandersn, I've started to run the parallelized community code test suite on this PR at d7c8e52. You can monitor the build here.

@sandersn
Copy link
Member Author

@DanielRosenwasser pretty sure this is not going to break anyone -- think it should go into 4.0?
@andrewbranch Mind reviewing this along with Daniel? The reviewers list is broken -- AGAIN.

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@sandersn
Copy link
Member Author

No changes to user tests! I think this is safe for 4.0.

@sandersn sandersn merged commit c3c6be6 into master Jun 17, 2020
@sandersn sandersn deleted the jsdoc-parse-conditional-types branch June 17, 2020 21:37
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -3356,7 +3356,7 @@ namespace ts {
break;
case SyntaxKind.QuestionToken:
// If not in JSDoc and next token is start of a type we have a conditional type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment appears to be out of date?

No Sign up for free to join this conversation on GitHub. Already have an account? No Sign in to comment
Projects
None yet
4 participants