New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed incorrect SignatureFlags.HasRestParameter
propagation when combining signatures
#58440
base: main
Are you sure you want to change the base?
Fixed incorrect SignatureFlags.HasRestParameter
propagation when combining signatures
#58440
Conversation
…mbining signatures
@jakebailey could you prepare a TS playground for this? |
@typescript-bot test it |
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 |
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
@jakebailey Here are the results of running the user tests comparing Everything looks good! |
@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 comparing Something interesting changed - please have a look. Details
|
|
Are you saying |
Oops, I can see how my comment was confusing 😅 I meant that the behavior change with my PR was definitely not OK in the We can see here, on the second one, that the error is reported. On the first one, we pass the same argument - but the function(s) with On top of that, we can remove I pushed out a fix for |
@typescript-bot test it |
@@ -31261,12 +31266,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
const paramSymbol = createSymbol( | |||
SymbolFlags.FunctionScopedVariable | (isOptional && !isRestParam ? SymbolFlags.Optional : 0), | |||
paramName || `arg${i}` as __String, | |||
isRestParam ? CheckFlags.RestParameter : isOptional ? CheckFlags.OptionalParameter : 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding CheckFlags.RestParameter
here is based on the similar change from #55625
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 |
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
@jakebailey Here are the results of running the user tests comparing Everything looks good! |
@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 comparing Something interesting changed - please have a look. Details
|
fixes #58371