Skip to content

Only functions can be constructor functions#27369

Merged
sandersn merged 1 commit intorelease-3.1from
js/only-functions-can-be-constructor-functions
Oct 8, 2018
Merged

Only functions can be constructor functions#27369
sandersn merged 1 commit intorelease-3.1from
js/only-functions-can-be-constructor-functions

Conversation

@sandersn
Copy link
Copy Markdown
Member

@constructor put on anything incorrectly makes it a JS constructor. This is a problem for actual constructors, because getJSClassType doesn't work on actual classes. The fix is to make isJSConstructor require that its declaration is a function.

Fixes #27345

`@constructor` put on anything incorrectly makes it a JS constructor. This
is a problem for actual constructors, because getJSClassType doesn't
work on actual classes. The fix is to make isJSConstructor require that
its declaration is a function.
@sandersn sandersn added this to the TypeScript 3.1 milestone Sep 26, 2018
@weswigham
Copy link
Copy Markdown
Member

weswigham commented Sep 27, 2018

@sandersn What about an odd pattern like:

/**
* @constructor
*/
var MyClass = mixin(MyClassBaseCtor, MyMixin);

?

@sandersn
Copy link
Copy Markdown
Member Author

@weswigham That [1] might be supported in the future, but right now @constructor is only used to mark functions as constructable, essentially.

[1] It wouldn't make a difference for mixin as long as it typed its return type correctly as constructable. I assume that your odd example is trying to assert that the signature returned by mixin is constructable, even if it's just a normal call signature.

@weswigham
Copy link
Copy Markdown
Member

Something like that yeah. Or like mixin itself doesn't have a strong return type at all.

@sandersn
Copy link
Copy Markdown
Member Author

sandersn commented Oct 8, 2018

@weswigham Since you took a look earlier, mind signing off on this one?

Comment thread src/compiler/checker.ts
return false;
}
const func = isFunctionDeclaration(node) || isFunctionExpression(node) ? node :
isVariableDeclaration(node) && node.initializer && isFunctionExpression(node.initializer) ? node.initializer :
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about node.initializer = some Conditional expression that uses functionExpression in each branch?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, but that doesn't actually work today either, and you can put the annotation on the individual function expressions like so:

const Weird = b ?
    /** @constructor */
    function () { this. x = 1 } :
    /** @constructor */
    function () { this.y = 'yes' };

I'd rather address that as a new feature instead of as a 3.1 bugfix.

@sandersn sandersn merged commit 1cfab76 into release-3.1 Oct 8, 2018
@sandersn sandersn deleted the js/only-functions-can-be-constructor-functions branch October 8, 2018 17:01
@sandersn
Copy link
Copy Markdown
Member Author

sandersn commented Oct 8, 2018

This is now in master as well.

sandersn added a commit that referenced this pull request Oct 8, 2018
`@constructor` put on anything incorrectly makes it a JS constructor. This
is a problem for actual constructors, because getJSClassType doesn't
work on actual classes. The fix is to make isJSConstructor require that
its declaration is a function.
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
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