-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Ugly formatting of nested conditional types [TypeScript] #7948
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
The old (i.e. current) formatting looks more readable if the constituent types aren't as short and simple as in your examples. Prettier pr-7948 --parser typescript Input / Output, v2.0.2: export type ReadonlyDeep<T> = T extends
| Primitive
| ((...arguments: any[]) => unknown)
? T
: T extends ReadonlyMap<infer KeyType, infer ValueType>
? ReadonlyMapDeep<KeyType, ValueType>
: T extends ReadonlySet<infer ItemType>
? ReadonlySetDeep<ItemType>
: T extends object
? ReadonlyObjectDeep<T>
: unknown; Output, this PR: export type ReadonlyDeep<T> =
T extends Primitive | ((...arguments: any[]) => unknown) ? T :
T extends ReadonlyMap<infer KeyType, infer ValueType> ?
ReadonlyMapDeep<KeyType, ValueType> :
T extends ReadonlySet<infer ItemType> ? ReadonlySetDeep<ItemType> :
T extends object ? ReadonlyObjectDeep<T> :
unknown; |
I think it still looks more readable including changes from this PR. Input: export type ReadonlyDeep<T> = T extends Primitive | ((...arguments: any[]) => unknown) | SuperLongTypeVeryLong
? T
: T extends ReadonlyMap<infer KeyType, infer ValueType>
? ReadonlyMapDeep<KeyType, ValueType>
: T extends ReadonlySet<infer ItemType>
? ReadonlySetDeep<ItemType>
: T extends object
? ReadonlyObjectDeep<T>
: unknown; PR: export type ReadonlyDeep<T> =
T extends
| Primitive
| ((...arguments: any[]) => unknown)
| SuperLongTypeVeryLong ? T :
T extends ReadonlyMap<infer KeyType, infer ValueType> ?
ReadonlyMapDeep<KeyType, ValueType> :
T extends ReadonlySet<infer ItemType> ? ReadonlySetDeep<ItemType> :
T extends object ? ReadonlyObjectDeep<T> :
unknown; master: export type ReadonlyDeep<T> = T extends
| Primitive
| ((...arguments: any[]) => unknown)
| SuperLongTypeVeryLong
? T
: T extends ReadonlyMap<infer KeyType, infer ValueType>
? ReadonlyMapDeep<KeyType, ValueType>
: T extends ReadonlySet<infer ItemType>
? ReadonlySetDeep<ItemType>
: T extends object
? ReadonlyObjectDeep<T>
: unknown; |
Alright! Would you mind pasting the second example with your suggested formatting? |
The current formatting works great for that example. |
Indeed. But what if you have both long and short types inside a nested conditional? |
Ideally, the new formatting should be used only if each condition with its "true" branch fits on one line like in your original example, but as far as I understand Prettier's printer algorithm doesn't support such a trick (changing the formatting of previous lines if the current line exceeds the print width). This means we'll have to go for a less perfect solution: a heuristic that would check the AST and decide whether the new formatting would fit within the print width. |
type Foo = ( | ||
ThingamabobberFactory extends AbstractThingamabobberFactory | ||
? GobbledygookProvider | ||
: CompositeGobbledygookProvider | ||
) extends DoubleGobbledygookProvider | ||
? UniqueDalgametreService | ||
: CompositeZamazingoResolver; | ||
|
||
type Foo2 = DoubleGobbledygookProvider extends ( | ||
ThingamabobberFactory extends AbstractThingamabobberFactory | ||
? GobbledygookProvider | ||
: CompositeGobbledygookProvider | ||
) | ||
? UniqueDalgametreService | ||
: CompositeZamazingoResolver; | ||
|
||
type Foo3 = ( | ||
ThingamabobberFactory extends AbstractThingamabobberFactory | ||
? GobbledygookProvider | ||
: CompositeGobbledygookProvider | ||
) extends ( | ||
DoubleGobbledygookProvider extends MockGobbledygookProvider | ||
? MockThingamabobberFactory | ||
: ThingamabobberFactory | ||
) | ||
? UniqueDalgametreService | ||
: CompositeZamazingoResolver; | ||
type Foo = | ||
( | ||
ThingamabobberFactory extends AbstractThingamabobberFactory ? | ||
GobbledygookProvider : | ||
CompositeGobbledygookProvider | ||
) extends DoubleGobbledygookProvider ? UniqueDalgametreService : | ||
CompositeZamazingoResolver; | ||
|
||
type Foo2 = | ||
DoubleGobbledygookProvider extends ( | ||
ThingamabobberFactory extends AbstractThingamabobberFactory ? | ||
GobbledygookProvider : | ||
CompositeGobbledygookProvider | ||
) ? UniqueDalgametreService : | ||
CompositeZamazingoResolver; | ||
|
||
type Foo3 = | ||
( | ||
ThingamabobberFactory extends AbstractThingamabobberFactory ? | ||
GobbledygookProvider : | ||
CompositeGobbledygookProvider | ||
) extends ( | ||
DoubleGobbledygookProvider extends MockGobbledygookProvider ? | ||
MockThingamabobberFactory : | ||
ThingamabobberFactory | ||
) ? UniqueDalgametreService : | ||
CompositeZamazingoResolver; |
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.
These should remain unchanged.
FYI #9561 contains proposals to use this style of formatting in TS Conditionals as well as, possibly, some nested ternaries. I wrote it before I saw this PR, but I might just close mine in favor of this. Do you still plan to work on it? Would you like help handling cases where the clauses are long? |
This is something that hurts me since I started using Prettier on TypeScript libraries. Usage of conditional types in this domain happens a lot more than in classical Applications, and I also think that the output is ugly. It does not make any sense that the first condition is always next to export type SchemaToType<S extends Schema> = S extends SchemaLiteral
? SchemaLiteralToType<S>
: S extends SchemaPrimitive
? SchemaPrimitiveToType<S>
: S extends Schema.Record
? SchemaRecordToType<S>
: S extends Schema.Array
? SchemaArrayToType<S>
: S extends Schema.Intersection
? SchemaIntersectionToType<S>
: S extends Schema.Union
? SchemaUnionToType<S>
: S; Just allowing to have the first condition on the next line (maybe through an config option?), would be a big win IMO. export type SchemaToType<S extends Schema> =
S extends SchemaLiteral
? SchemaLiteralToType<S>
: S extends SchemaPrimitive
? SchemaPrimitiveToType<S>
: S extends Schema.Record
? SchemaRecordToType<S>
: S extends Schema.Array
? SchemaArrayToType<S>
: S extends Schema.Intersection
? SchemaIntersectionToType<S>
: S extends Schema.Union
? SchemaUnionToType<S>
: S; It makes it much easier to read, particularly when there are multiple generics (with defaults etc...). |
Any news on this @mmiszy @thorn0 ? |
Doing a clean up of stale PRs, closing this. It's linked to the issue so we can revive it if necessary. |
Fixes #7940
My intention was to only affect nested conditional types in TypeScript and not to introduce any changes to how ternary expressions are formatted. This allows for a smoother transition, however, it might yield seemingly inconsistent results (see this example).
docs/
directory)changelog_unreleased/*/pr-XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨