Only functions can be constructor functions #27369
Conversation
`@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 What about an odd pattern like: /**
* @constructor
*/
var MyClass = mixin(MyClassBaseCtor, MyMixin);? |
|
@weswigham That [1] might be supported in the future, but right now [1] It wouldn't make a difference for |
|
Something like that yeah. Or like |
|
@weswigham Since you took a look earlier, mind signing off on this one? |
| return false; | ||
| } | ||
| const func = isFunctionDeclaration(node) || isFunctionExpression(node) ? node : | ||
| isVariableDeclaration(node) && node.initializer && isFunctionExpression(node.initializer) ? node.initializer : |
There was a problem hiding this comment.
What about node.initializer = some Conditional expression that uses functionExpression in each branch?
There was a problem hiding this comment.
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.
|
This is now in master as well. |
`@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.
@constructorput 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