Improve JavaScript type from constructor imported via require #16686
Conversation
src/compiler/checker.ts
Outdated
| @@ -16089,6 +16089,21 @@ namespace ts { | |||
| return false; | |||
| } | |||
|
|
|||
| function getJavaScriptClassType(symbol: Symbol): Type | undefined { | |||
| if (symbol && isDeclarationOfFunctionOrClassExpression(symbol)) { | |||
There was a problem hiding this comment.
the caller just checked that symbol is defined, so symbol && shouldn't be needed.
There was a problem hiding this comment.
Or it should be Symbol | undefined then
| if (isJavaScriptConstructor(symbol.valueDeclaration)) { | ||
| return getInferredClassType(symbol); | ||
| } | ||
| if (symbol.flags & SymbolFlags.Variable) { |
There was a problem hiding this comment.
what's an example for this case? it looks like the const B = function ... case in the new test file, but I expected that to be imported from a new, third file instead of just being in the original file.
There was a problem hiding this comment.
The const A = case tests the from an external file, while the const B = case tests the use locally. Both cases are relevant. Are you talking about a third case where we import something re-exported from a third file?
There was a problem hiding this comment.
Does line 16096 handle the const A = test (external) and 16099 handle the const B = test (local)? The two tests differ in syntax, so I thought the difference in the two cases in getJavascriptClassType was syntactic, and that both needed to come from external files. I guess that's wrong, and the difference is external vs local?
There was a problem hiding this comment.
Line 16096 handles the case where symbol refers directly (globally or within the same file) to a function or a variable declaration whose initializer is a function.
Line 16099 handles the case where symbol refers indirectly to a function or variable declaration whose initializer is a function. This is usually the case for imports like const C = require("./c") or const { C } = require("./c"), but also cases like const C = B where B is also local and is a function or variable declaration whose initializer is a function.
|
🔔 Status? |
|
We will need to port this to release-2.5 |
|
This is now also in release-2.5. |
This improves the type we resolve for a
newexpression whose constructor was imported from a JavaScript file via a CommonJSrequirecall.Fixes #16467