Skip to content

Allow JS constructor function to return non-void#16196

Merged
rbuckton merged 4 commits into
masterfrom
fix15959
Jun 6, 2017
Merged

Allow JS constructor function to return non-void#16196
rbuckton merged 4 commits into
masterfrom
fix15959

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton commented Jun 1, 2017

This loosens the restriction that a function must return void if it is to be used as a constructor when the function is defined in a JavaScript file.

Fixes #15959

@rbuckton rbuckton requested a review from mhegazy June 1, 2017 19:12
@rbuckton rbuckton requested review from sandersn and yuit June 1, 2017 20:38
Comment thread src/compiler/checker.ts Outdated
if (callSignatures.length) {
const signature = resolveCall(node, callSignatures, candidatesOutArray);
if (getReturnTypeOfSignature(signature) !== voidType) {
if (!isInJavaScriptFile(signature.declaration) && getReturnTypeOfSignature(signature) !== voidType) {
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.

should not we check functionSymbol.members here to make sure the function has a this.prop = .. somewhere, and avoid something like new getNumber()

Copy link
Copy Markdown
Contributor Author

@rbuckton rbuckton Jun 6, 2017

Choose a reason for hiding this comment

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

Probably, but I need to fix #14056 first to properly resolve the underlying function to get to the symbols. Right now the symbol points to the local var declaration and doesn't have this information.

Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

Can you add some conformance tests for these patterns also the use of @class

@rbuckton rbuckton merged commit 70c1c57 into master Jun 6, 2017
@rbuckton rbuckton deleted the fix15959 branch June 6, 2017 22:27
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants