Skip to content

Preserve arity for preserving js optional parameters #37173

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

Merged

Conversation

weswigham
Copy link
Member

Fixes #36507

For relationship checking, we consider untyped signatures from js as only having 0 required arguments; however we also use that minimum argument count for calculating weather an initialized optional parameter is optional or not. In the later case, we would like that "js signatures have 0 required arguments" rule to not apply, so we can actually recover what parameters are optional.

@sandersn
Copy link
Member

sandersn commented Mar 3, 2020

I believe this property also causes differences from TS when choosing overloads too. I'll need to read the change to see whether it might fix those too.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Let me track down the overload bug so you can check whether it fixes that too. I think it probably will.

@sandersn
Copy link
Member

sandersn commented Mar 3, 2020

#31888

With small repro:

/** @param {{
    (fn: (a: number) => void): number;
    (fn: (a: number, b: number) => void): string
}} id */
export function test(id) {
    /** @type {string} */
    var val = id((a, b) => a + b);
}

@sandersn
Copy link
Member

sandersn commented Mar 3, 2020

The problems that my simple fix at #35153 had shows that we should look at webpack (and maybe some others?) before merging to see if they break with this.

@weswigham
Copy link
Member Author

Let me track down the overload bug so you can check whether it fixes that too. I think it probably will.

It does not~
Behavior for actual call resolution is unchanged.

@weswigham weswigham merged commit dfc0b58 into microsoft:master Mar 4, 2020
@weswigham weswigham deleted the js-declarations-leading-optional branch March 4, 2020 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants