Don't treat object properties as potential JS contructors without JSDoc class tag#49735
Don't treat object properties as potential JS contructors without JSDoc class tag#49735jakebailey merged 5 commits intomicrosoft:mainfrom
Conversation
| >this.c = 'nested object' : "nested object" | ||
| >this.c : any | ||
| >this : this | ||
| >this : { C: typeof C; } |
There was a problem hiding this comment.
This test is explicitly the case that is changing:
var o = {
C: function () {
this.c = 'nested object'
}
}
var og = new o.C();So, I'm not sure if that's good or bad.
andrewbranch
left a comment
There was a problem hiding this comment.
While I can see in the existing code that this probably already works, there doesn’t seem to be a test (at least in ones you added or changed) that uses /** @class */ inside an object.
Come to think of it, what should the behavior of
const o = {
SomeClass() {
this.x = 0;
}
}be (with and without @class)? I don’t know, but let’s add it to tests too so we have a documented position.
|
I think that https://github.com/microsoft/TypeScript/pull/49735/files#diff-dbf882263a483218df32c53af90df429250234fb325ba71f8dd34bcbe165fe54 is that case, but you're right that I forgot one that's tagged (I don't know if there is one; I would assume so but I will check). |
|
I missed that you had a method declaration there and not a function expression. The runtime behavior is the same, so I'm inclined to make it the same. I'll add an explicit test. |
|
Handled that case. I'm a little bit unsure what the behavior should be for some of those things that changed. #27561 is relevant. |
|
I’m not sure I think it’s very important to make object literal method declarations classable. That code looks bananas to me. |
andrewbranch
left a comment
There was a problem hiding this comment.
Looks good, but I think we decided to wait until 4.9 for this, right?
|
In #48915 (comment), I think so, yes. I don't know if this is urgent enough to go into 4.8. |
|
I think the breakiness is fairly low, but the bug is not a recent regression, has no upvotes, and can be worked around, so for me it doesn’t meet the bar for considering it now. |
Fixes #48915
This is probably breaking, so for 4.9 if we want it.