-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Cherry pick PR #57887 into release-5.4 #57898
Cherry pick PR #57887 into release-5.4 #57898
Conversation
…c SymbolFlags meaning (microsoft#57887)
@@ -2,6 +2,6 @@ | |||
|
|||
=== ParameterList5.ts === | |||
function A(): (public B) => C { | |||
>A : () => (B: any) => C | |||
>A : () => (public B) => C |
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.
Changes like this one aren't in main
because #57772 added checks/transforms to prevent them. Here, for example, now we lookup B
successfully and think "hey, I'll just reuse the node" (which also happens with the isDeclarationName
fix added in #57772, for this specific case), but the node's got a modifier that needs deleting and a : any
that needs adding that it doesn't add, while in #57772 I added a line to the visitor to add the missing : any
and drop the public
from existing nodes in the node builder. There's a couple changes like this in here. They're not that bad a change because it's not like this node's form nowhere - it's the input, so it's at least no worse than the input - but seeing an error in the input (like this) and just dumping the erroneous node into the output is something we usually try to avoid. We could also backport the changes to visitExistingNodeTreeSymbols
from that PR if we want to minimize the diff.
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.
Does that mean that this PR is wrong without that other PR? Or is this PR safe to include in a patch by itself?
See #57887
I have no idea why the baselines are so big in the cherry pick compared to main, but they're all improvements?