Skip to content

Infer optional types for IIFE parameters with missing arguments#13358

Merged
ahejlsberg merged 3 commits into
masterfrom
iifeMissingArguments
Jan 9, 2017
Merged

Infer optional types for IIFE parameters with missing arguments#13358
ahejlsberg merged 3 commits into
masterfrom
iifeMissingArguments

Conversation

@ahejlsberg

@ahejlsberg ahejlsberg commented Jan 9, 2017 •
edited by DanielRosenwasser
Loading

Copy link
Copy Markdown
Member

We currently contextually type un-annotated IIFE parameters by their corresponding arguments. This PR expands upon this concept by inferring un-annotated IIFE parameter with missing arguments as optional. That allows us to handle the following common idiom with no errors:

let foo = (function (window, undefined) {
    // Body
})(window);

Without this PR the example is an error because the undefined isn't marked optional.

Note that the PR also simplifies some overly complicated logic for computing the minArgCount property in signatures.

@ahejlsberg

ahejlsberg commented Jan 9, 2017

Copy link
Copy Markdown
Member Author

@billti @mhegazy This is a fix for an issue Bill mentioned the other day.

@billti

billti commented Jan 9, 2017

Copy link
Copy Markdown
Member

Great!

Just playing around with this in TypeScript, and with strictNullChecks set, the below gives an error with the type annotation on b. Without the annotation on b it is fine (and also obviously if I pass undefined for the second arg).

Maybe this is "by design", and if you put the annotation you need to mark the param as optional (which also gets rid of the error), but intuitively it feels like this should be valid with the new inference. Thanks!

(function(a: string, b: undefined) {

})("test");

@ahejlsberg

ahejlsberg commented Jan 9, 2017

Copy link
Copy Markdown
Member Author

@billti Yes, this is by design. We only contextually type un-annotated parameters. Once a type annotation is present you must also add a ? modifier if you want the parameter to be considered optional.

mhegazy
mhegazy reviewed Jan 9, 2017
View reviewed changes
Comment thread src/compiler/checker.ts
links.resolvedSignature = anySignature;
const type = indexOfParameter < iife.arguments.length ?
getWidenedLiteralType(checkExpression(iife.arguments[indexOfParameter])) :
parameter.initializer ? undefined : undefinedWideningType;

@mhegazy mhegazy Jan 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sandersn this will need to be covered in your change for initialized parameters.

@ahejlsberg ahejlsberg merged commit ecb2115 into master Jan 9, 2017
@ahejlsberg ahejlsberg deleted the iifeMissingArguments branch January 9, 2017 19:30
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mhegazy mhegazy mhegazy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ahejlsberg @billti @mhegazy @msftclas