-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Try making parenthesizeConstituentTypeOfUnionType a no-op #57900
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
Conversation
@typescript-bot run dt |
Hey @jakebailey, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: eslint
Package: ember__array/v3
Package: parse
Package: react/v16
Package: ember__array
Package: react/v17
Package: react
Package: gulp-run
Package: wordpress__editor
|
@jakebailey Here they are:
tscComparison 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: |
Actually not that bad |
Only 9 breaks on DT? Really not that bad. |
@@ -23,7 +23,7 @@ function f() { | |||
|
|||
function f() { | |||
let a = 1; | |||
let x: (/*A*/ "a" /*B*/ | /*C*/ 'b' /*D*/) | undefined; | |||
let x: /*A*/ /*A*/ "a" /*B*/ | /*C*/ 'b' /*D*/ | undefined; |
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.
This extra /*A*/
is curious
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.
Presumably we added logic to copy the leading comment range from the union itself onto the inner "a"
when we added the ()
, but with the ()
no longer being added, we also no longer need to copy the comment range from the union node to the "a"
, since the union node isn't getting remade.
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.
Any guesses as to where that logic might be?
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.
Based on how extractSymbol
works, probably some of the comment suppression/copying logic for getSynthesizedDeepClone
or getTypeDeepCloneUnionUndefined
.
Curious about #57890 (comment) on DT