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

Don't emit binding pattern pattern / optional parameter error when an initializer is present #52880

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Feb 20, 2023

Fixes #50286

Before #50094, this code was fine because in TS, a question token and an initializer are mutually exclusive.

But after #50094, we now take into account optionality from JSDoc. But that means that we won't have a question token, and therefore can have an initializer, and that initializer can prevent a crash on call. So, we can just skip this error if there's an initializer; in TS code, this won't happen, but it fixes the issue for JS code.

This check is not type-aware. We could be smarter and use the new parameterInitializerContainsUndefined function I added in #52696 to detect undefined (or null, with modification), but there's no point; we already emit a type error beforehand because we will check that the initializer is assignable to the binding pattern and undefined or null are not.

While here, use isOptionalDeclaration instead of checking questionToken and isJSDocOptionalParameter.

Copy link
Member

@iisaduan iisaduan left a comment

Choose a reason for hiding this comment

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

Looks good!

@jakebailey jakebailey merged commit ef79b42 into microsoft:main Feb 22, 2023
@jakebailey jakebailey deleted the fix-50286 branch February 22, 2023 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix regression for optional JSDoc types which throws TS2463 in typescript@next
3 participants