Treat contextually typed functions in JS files as typed #56907
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
@typescript-bot test top200 @typescript-bot perf test this |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 9a0c1d6. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the regular perf test suite on this PR at 9a0c1d6. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 9a0c1d6. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 9a0c1d6. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 9a0c1d6. You can monitor the build here. Update: The results are in! |
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@jakebailey Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
Hey @jakebailey, the results of running the DT tests are ready. |
|
@jakebailey Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
StartupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
src/compiler/checker.ts
Outdated
| @@ -15107,7 +15107,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| isInJSFile(declaration) && | |||
| isValueSignatureDeclaration(declaration) && | |||
| !hasJSDocParameterTags(declaration) && | |||
| !getJSDocType(declaration); | |||
| !getJSDocType(declaration) && | |||
| !getContextualSignatureForFunctionLikeDeclaration(declaration); | |||
There was a problem hiding this comment.
Does this change the semantics of @type, or is it specific to @satisfies? For example, in
/** @type {(uuid: string) => string}
const f = uuid => uuidI recall that @type intentionally is not treated as a contextual signature in that case, but I could be mistaken. I believe that instead the @type tag is looked up and treated the same as if it were written:
const f = /** @type {(uuid: string) => }*/(uuid => uuid)That is, treated like a cast. Probably @satisfies should be the same.
There was a problem hiding this comment.
It changes the semantics of the second example but not the first one. @type above the declaration is treated as the contextual signature (getContextualSignature calls getSignatureOfTypeTag and returns that if it's found).
That said, I'm not sure if this is a bad change. This flag has one purpose - it changes how getMinArgumentCount gets computed. If the types of parameters are assigned in just any way, I think it's surprising that the resulting signature might get treated as "untyped" and that it might allow calls without all (seemingly) required parameters. Just take a look at the situation reported in this thread and at the discussion happening there. While this PR here doesn't solve the problem described there with @type applied to parameters, the root cause is similar - despite the call being typed it's treated as untyped. This creates weird discrepancies between TS and JS files.
There was a problem hiding this comment.
An extra data point:
const f1 = /** @type {(uuid: string) => {}}*/(uuid => uuid)
f1() // error
const f2 = /** @satisfies {(uuid: string) => {}}*/(uuid => uuid)
f2() // ok, wat?The reason behind the difference here is that with the cast the inner signature is untyped but the final type of f1 is sourced from the casted type itself. Since that's a type node~ it can't be untyped.
To give more context on the above:
checkAssertionWorkercomputesexprTypewith an untyped signature (but its parameters are typed!)- but it returns
getTypeFromTypeNode(type)
This can't work like that with @satisfies as the @satifies merely provides some contextual typing - it doesn't "override" the expression type.
There was a problem hiding this comment.
Thanks, that explanation helps. I was getting side-tracked by having the jsdoc on the declaration instead of directly on the arrow.
typescript-bot
added
For Backlog Bug
src/compiler/checker.ts
Outdated
| @@ -15107,7 +15107,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| isInJSFile(declaration) && | |||
| isValueSignatureDeclaration(declaration) && | |||
| !hasJSDocParameterTags(declaration) && | |||
| !getJSDocType(declaration); | |||
| !getJSDocType(declaration) && | |||
| !getContextualSignatureForFunctionLikeDeclaration(declaration); | |||
There was a problem hiding this comment.
Thanks, that explanation helps. I was getting side-tracked by having the jsdoc on the declaration instead of directly on the arrow.
|
@Andarist can you merge this from main and then I'll run typescript-bot test it before merging it. |
…d-fns-as-typed-in-js
|
@sandersn done |
…d-fns-as-typed-in-js
|
@jakebailey could u rerun extended tests here? |
|
@typescript-bot test it |
|
@jakebailey Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
The webpack break above is correct in a sense that it brings the JS behavior closer to the TS behavior:
|
fixes #56900