Use the width of a type to add newlines in type assignment error messages

[orta]

Today we ship a one-type-fits-all style for printing type is not assignable to type messages. We'd like to improve this in a pretty simple manner: by occasionally adding newlines. For example with this arbitrary comparison:

let a = { b: { c: { e: { f: 123 } } } };
let b = { b: { c: { e: { f: "123" } } } };
a = b;

Looks like this today:

src/vendor/ata/index.ts(12,1): error TS2322: Type '{ b: { c: { e: { f: string; }; }; }; }' is not assignable to type '{ b: { c: { e: { f: number; }; }; }; }'.
The types of 'b.c.e.f' are incompatible between these types.
    Type 'string' is not assignable to type 'number'.

I propose that because both of the printed types are longer than 20-30 chars, then we switch to:

src/vendor/ata/index.ts(12,1): error TS2322: Type: 
{ b: { c: { e: { f: string; }; }; }; }

is not assignable to type:
{ b: { c: { e: { f: number; }; }; }; }

The types of 'b.c.e.f' are incompatible between these types.
    Type 'string' is not assignable to type 'number'.

Key details

• Strip the quotes around the type
• Add colons and newlines if the type is over a certain size. Let's call it 30 characters for now, and make it easy to change and we can experiment at the final stages of review.

Some Examples

No changes

let a = 123
let b = "abc"
a = b

• Before: src/index.ts(21,1): error TS2322: Type 'string' is not assignable to type 'number'.
• After: src/index.ts(21,1): error TS2322: Type 'string' is not assignable to type 'number'.

No change! (Though a part of me does really want to drop the quotes for primitives)

window = {}

Type '{}' is not assignable to type 'Window & typeof globalThis'.
  Type '{}' is missing the following properties from type 'Window': clientInformation, closed, customElements, devicePixelRatio, and 189 more. (2322)

No change! {} and Window & typeof globalThis are too small.

Changing one

window = addEventListener

src/index.ts(1,1): error TS2322: Type '{ <K extends keyof WindowEventMap>(type: K, listener: (this: Window, ev: WindowEventMap[K]) => any, options?: boolean | AddEventListenerOptions | undefined): void; (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | ... 1 more ... | undefined): void; }' is not assignable to type 'Window & typeof globalThis'.
   Type '{ <K extends keyof WindowEventMap>(type: K, listener: (this: Window, ev: WindowEventMap[K]) => any, options?: boolean | AddEventListenerOptions | undefined): void; (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | ... 1 more ... | undefined): void; }' is missing the following properties from type 'Window': HTMLDocument, closed, customElements, devicePixelRatio, and 187 more.

src/vendor/ata/index.ts(12,1): error TS2322: Type: 
{ <K extends keyof WindowEventMap>(type: K, listener: (this: Window, ev: WindowEventMap[K]) => any, options?: boolean | AddEventListenerOptions | undefined): void; (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | ... 1 more ... | undefined): void; }

is not assignable to type:
Window & typeof globalThis

  Type '{ <K extends keyof WindowEventMap>(type: K, listener: (this: Window, ev: WindowEventMap[K]) => any, options?: boolean | AddEventListenerOptions | undefined): void; (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | ... 1 more ... | undefined): void; }' is missing the following properties from type 'Window': HTMLDocument, closed, customElements, devicePixelRatio, and 187 more.

Changing both

Animation = AnalyserNode

index.ts(12,1) Type '{ new (context: BaseAudioContext, options?: AnalyserOptions | undefined): AnalyserNode; prototype: AnalyserNode; }' is not assignable to type '{ new (effect?: AnimationEffect | null | undefined, timeline?: AnimationTimeline | null | undefined): Animation; prototype: Animation; }'.
  Types of property 'prototype' are incompatible.
    Type 'AnalyserNode' is missing the following properties from type 'Animation': currentTime, effect, finished, id, and 18 more.

index.ts(12,1) Type:
{ new (context: BaseAudioContext, options?: AnalyserOptions | undefined): AnalyserNode; prototype: AnalyserNode; }

is not assignable to type:
{ new (effect?: AnimationEffect | null | undefined, timeline?: AnimationTimeline | null | undefined): Animation; prototype: Animation; }

  Types of property 'prototype' are incompatible.
    Type 'AnalyserNode' is missing the following properties from type 'Animation': currentTime, effect, finished, id, and 18 more.

[heyAyushh]

Hey @orta, I am a typescript beginner and want to contribute here, could you guide me where to start?

[orta]

Sure, first read the READMEs/contributing guides - I think all of the changes will probably live inside reportRelationError(message: DiagnosticMessage | undefined, source: Type, target: Type in checker.ts - somewhere around lines 17790.

sourceType and targetType are the strings, you need to use the length of to determine what to do

[fatcerberus]

Though a part of me does really want to drop the quotes for primitives

Then it becomes a garden path sentence:

Type string is not assignable to type number

"What's a type string? Does that mean literal string types? Oh wait, it means the actual type 'string'."

It's better with the quotes.

[orta]

Yeah, exactly, I don't disagree - just pining for a better way to visually distinguish between type and message e.g.

[prabhatmishra33]

Hello ! @heyAyushh Please let me know if you are not working on this anymore so that I can take this forward 😊

[heyAyushh]

@prabhatmishra33 I think @cytler is already working on it,

[ben-m-j-taylor]

Hi @DanielRosenwasser, I'm just looking at how I might implement a change for this issue and noticed your comment on the PR that was previously opened for this issue.

I wanted to clarify your thoughts on one point that you made.

I think the right fix here is actually to create new diagnostics for specific well-known ones and swap them out; especially since a diagnostic might mention the same type multiple times.

I agree that this seems like a sensible option, however, with the way that the diagnostic message generation works (at least from my current understanding) to introduce a second "pretty" message it would need to be created with a new error code as the generation script will error out if it finds duplicate codes.

Would you be comfortable with introducing a new error code for the "pretty" messages? Personally, that doesn't feel like something that should be done for the sake of introducing some nicer formatting, but I'd be interested to hear your thoughts on this as I might be missing some context/domain knowledge that makes the answer clearer.

[DanielRosenwasser]

Sorry for the delay - yup, adding a new error code is fine.