Skip to content

Provide flattened.error.reporting(...).for(...).nested.incompatible.types #33361

Closed

Description

let x = { a: { b: { c: { d: { e: { f() { return { g: "hello" }; } } } } } } };
let y = { a: { b: { c: { d: { e: { f() { return { g: 12345 }; } } } } } } };
x = y;

Existing Behavior

Type '{ a: { b: { c: { d: { e: { f(): { g: number; }; }; }; }; }; }; }' is not assignable to type '{ a: { b: { c: { d: { e: { f(): { g: string; }; }; }; }; }; }; }'.
  Types of property 'a' are incompatible.
    Type '{ b: { c: { d: { e: { f(): { g: number; }; }; }; }; }; }' is not assignable to type '{ b: { c: { d: { e: { f(): { g: string; }; }; }; }; }; }'.
      Types of property 'b' are incompatible.
        Type '{ c: { d: { e: { f(): { g: number; }; }; }; }; }' is not assignable to type '{ c: { d: { e: { f(): { g: string; }; }; }; }; }'.
          Types of property 'c' are incompatible.
            Type '{ d: { e: { f(): { g: number; }; }; }; }' is not assignable to type '{ d: { e: { f(): { g: string; }; }; }; }'.
              Types of property 'd' are incompatible.
                Type '{ e: { f(): { g: number; }; }; }' is not assignable to type '{ e: { f(): { g: string; }; }; }'.
                  Types of property 'e' are incompatible.
                    Type '{ f(): { g: number; }; }' is not assignable to type '{ f(): { g: string; }; }'.
                      Types of property 'f' are incompatible.
                        Type '() => { g: number; }' is not assignable to type '() => { g: string; }'.
                          Type '{ g: number; }' is not assignable to type '{ g: string; }'.
                            Types of property 'g' are incompatible.
                              Type 'number' is not assignable to type 'string'.

Proposed

Type '{ a: { b: { c: { d: { e: { f(): { g: number; }; }; }; }; }; }; }' is not assignable to type '{ a: { b: { c: { d: { e: { f(): { g: string; }; }; }; }; }; }; }'.
    'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'

Activity

  1. orta commented on Sep 11, 2019

    @orta
    Contributor

    I explored this a bit with Eloy Durán (@alloy) when thinking about whether we could also use bold/italics to also provide emphasis, instead we used { ..., b: { ... c: } } to provide focus while keeping the pyramid of nested errors.

    Screen Shot 2019-09-11 at 7 58 00 AM

    I prefer the version in OP. I wonder if turning it into three lines would make it more readable:

    Type '{ a: { b: { c: { d: { e: { f(): { g: number; }; }; }; }; }; }; }' 
    is not assignable to type '{ a: { b: { c: { d: { e: { f(): { g: string; }; }; }; }; }; }; }'.
    
            'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    Then you get to see the full message without scrolling, and can more easily compare the types.

    Ideally, if we're in an environment where we can use bold:

    Type '{ a: { b: { c: { d: { e: { f(): { **g: number**; }; }; }; }; }; }; }' 
    is not assignable to type '{ a: { b: { c: { d: { e: { f(): { **g: string**; }; }; }; }; }; }; }'.
    
            'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    Which would really help lead someone's eyes to the right spots to make their own decisions.

  2. dhruvrajvanshi commented on Sep 11, 2019

    @dhruvrajvanshi
    Contributor

    I'd like to try this.

  3. DanielRosenwasser commented on Sep 11, 2019

    @DanielRosenwasser
    MemberAuthor

    Dhruv Rajvanshi (@dhruvrajvanshi) Go for it! It might be a little bit tricky because this is deep in the logic for checkTypeRelatedTo.

    To scope the problem space, I'll call the special diagnostic

    'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    a member chain diagnostic.

    A member chain diagnostic is created by tracking checks between properties and call signatures, and determining if multiple relevant properties/call signatures are encountered consecutively. A member is relevant for these checks if the member is

    • a property that is a valid IdentifierNames (i.e. not Symbol.iterator or "hello there")
    • a non-generic call signature, where each containing type has only one call signature (so signatures from { (): number; (x: any): string } and () => boolean shouldn't be included)

    I like that Orta Therox (@orta). Maybe that can be layered on top for the first message, even if we can't do styling on diagnostics yet. I'd open up a separate issue if you're interested!

  4. DanielRosenwasser commented on Sep 11, 2019

    @DanielRosenwasser
    MemberAuthor

    (Please forgive me for my poorly written spec 😅)

  5. changed the title [-]Provided flattened.error.reporting(...).for(...).nested.incompatible.types[/-] [+]Provide flattened.error.reporting(...).for(...).nested.incompatible.types[/+] on Sep 13, 2019
  6. DanielRosenwasser commented on Sep 13, 2019

    @DanielRosenwasser
    MemberAuthor

    By the way, we'd like to get this in for the beta if possible. Do you think you can get this done by the end of the month Dhruv Rajvanshi (@dhruvrajvanshi)?

  7. dhruvrajvanshi commented on Sep 14, 2019

    @dhruvrajvanshi
    Contributor

    Daniel Rosenwasser (@DanielRosenwasser) In that case, you might want to assign this to someone else. I can't really commit to a deadline right now.

  8. austincummings commented on Sep 15, 2019

    @austincummings
    Contributor

    Daniel Rosenwasser (@DanielRosenwasser) I think I could commit to getting this done by then. Is there someone/somewhere in particular I should ask for guidance on this? Or just here on the issue?

  9. orta commented on Sep 16, 2019

    @orta
    Contributor

    I think this issue is a good place to leave notes and questions 👍

  10. austincummings commented on Sep 17, 2019

    @austincummings
    Contributor

    Just wanted to give a small update and see if I'm on the right approach.

    So far I've mostly just been getting an understanding of how checkTypeRelatedTo works and playing with how to track the property checks.

    A member chain diagnostic is created by tracking checks between properties and call signatures

    Could this be done by keeping a linked list structure (or even just an array) defined in checkTypeRelatedTo, and appending the targetProp Symbol in propertyRelatedTo when it reports the Diagnostics.Types_of_property_0_are_incompatible error? (maybe also include a reference to the current errorInfo so I can match the message chain element and the member chain element?).

    Assuming that is correct, and once we have a "member chain" built, before createDiagnosticForNodeFromMessageChain is called is where I was thinking I could "collapse" the errorInfo down into the desired diagnostic (which I haven't yet considered how to do.).

    Hopefully I'm on the right track, and I may open a draft PR if that is ok to sort of show the work being done.

  11. weswigham commented on Sep 17, 2019

    @weswigham
    Member

    Austin Cummings (@austincummings) sorry my friend, but as of earlier today, we have a pretty much complete implementation that we should have a PR up for by EoD tomorrow - the assignment was supposed to indicate that~

    Hopefully it'll at least be educational? 🤷‍♀

  12. austincummings commented on Sep 17, 2019

    @austincummings
    Contributor

    Wesley Wigham (@weswigham) no worries. I'm sure it will be. Thanks for letting me know.

  13. elektronik2k5 commented on Sep 26, 2019

    @elektronik2k5

    May I suggest a small whitespace tweak to make the new and awesome error a bit more readable?
    Instead of

    Type '{ a: { b: { c: { d: { e: { f(): { **g: number**; }; }; }; }; }; }; }' 
    is not assignable to type '{ a: { b: { c: { d: { e: { f(): { **g: string**; }; }; }; }; }; }; }'.
    
            'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    move the is not assignable to to the end of the first line, so the types vertically align for easier scanning and vertical comparison:

    Type '{ a: { b: { c: { d: { e: { f(): { **g: number**; }; }; }; }; }; }; }' is not assignable to 
    type '{ a: { b: { c: { d: { e: { f(): { **g: string**; }; }; }; }; }; }; }'.
    
            'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    or, to minimize horizontal scrolling/overflow place it in a line in between the types:

    Type '{ a: { b: { c: { d: { e: { f(): { **g: number**; }; }; }; }; }; }; }'
            is not assignable to 
    type '{ a: { b: { c: { d: { e: { f(): { **g: string**; }; }; }; }; }; }; }'.
    
            'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    Happy to make such a PR :)

  14. weswigham commented on Sep 26, 2019

    @weswigham
    Member

    Individual multiline diagnostic messages I think require a couple changes to our diagnostic message chain builder, but I'd be down for considering it, personally. It's worth remembering that it could still occur in a pyramid, so you could have something like

    Type '{[idx: string]: { a: { b: { c: { d: { e: { f(): { g: number; }; }; }; }; }; }; }}'
            is not assignable to 
    type '{[idx: string]: { a: { b: { c: { d: { e: { f(): { g: string; }; }; }; }; }; }; }}'.
        
        String index signatures are incompatible.
            Type '{ a: { b: { c: { d: { e: { f(): { g: number; }; }; }; }; }; }; }'
                    is not assignable to 
            type '{ a: { b: { c: { d: { e: { f(): { g: string; }; }; }; }; }; }; }'.
    
                    'a.b.c.d.e.f(...).g' is incompatible between these types because 'number' is not assignable to type 'string'
    

    which is.... maybe better? I dunno, linebreaks, man. I know I probably don't wanna see

    Type 'number'
        is not assignable to
    type 'string'
    

    so there'd probably need to be some kind of length heuristic, at least.

  15. karlhorky commented on Sep 26, 2019

    @karlhorky
    Contributor

    Brainstorming about the (currently) impossible here, I would say that the thing that git does on highlighting diffs (via diff-highlight) would be super interesting here:

    Screen Shot 2019-09-26 at 10 01 51

    Would the TypeScript team be open to a pipe dream issue about this for some future?

  16. weswigham commented on Sep 26, 2019

    @weswigham
    Member

    So while that's pretty ok in the extreme example from the op, most[citation needed] types involved in larger errors aren't deeply nested anonymous objects, but are instead interfaces within interfaces - so your top level type error is usually something like A<Foo<Q>> is not assignable to B<Bar<U>> - so the "diff" is non-existent (since the whole thing is different). With this change and other changes we've made in the past (diving down to exact properties to issue errors on, for instance), I think we've got the anonymous object case handled pretty well now (at least enough that I think attempting to colorize textual diffs is only a marginal improvement (if at all - it also could border on misleading in some cases, like when structurally near-idential types with differing names are used) on the path callout on the following line) - it's the opaque class/alias (usually signature involving) structures that we really do not as much have down.

  17. karlhorky commented on Sep 26, 2019

    @karlhorky
    Contributor

    Ok, then I'll avoid creating a new issue for it. Thanks for the feedback!

  18. karol-majewski commented on Sep 26, 2019

    @karol-majewski

    It would be wonderful to have more approachable error messages in situations like this:

    https://imgur.com/F4Ov3Z7

    Here, the problem is related to index signatures. The expected type has one, the provided type doesn't.

    In order to see what the real problem is, one needs to read a pretty big and intimidating error message in which the important message is on the bottom.

  19. RyanCavanaugh commented on Sep 26, 2019

    @RyanCavanaugh
    Member

    Karol Majewski (@karol-majewski) "just print the error messages in reverse order" is a common suggestion; see #29759 (comment) . TL;DR it makes some things better and other things worse

  20. karol-majewski commented on Sep 27, 2019

    @karol-majewski

    Ryan Cavanaugh (@RyanCavanaugh) Thank you for your reply. Reversing the order is one dimension and I agree this is not a perfect solution everywhere.

    What I had in mind was making the error message say which object is missing the index signature. Right now we only know one of them is missing it.

    Expected a type with an index signature, but received one without it.

    My hope is this could point the users into the right direction: finding a way to update the object in a way that preserves its index signature (which the spread operator ... doesn't do).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Domain: Error MessagesThe issue relates to error messagingEffort: ModerateRequires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual".Experience EnhancementNoncontroversial enhancementsFixedA PR has been merged for this issueSuggestionAn idea for TypeScript

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions