Skip to content
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

Error TS2322: Improve nested interface message handling #20741

Closed
orta opened this issue Dec 17, 2017 · 7 comments
Closed

Error TS2322: Improve nested interface message handling #20741

orta opened this issue Dec 17, 2017 · 7 comments
Labels
Domain: Error Messages The issue relates to error messaging Suggestion An idea for TypeScript

Comments

@orta
Copy link
Contributor

orta commented Dec 17, 2017

Actual behavior:

interface Nestedthing {
  a: {
    b: {
      c: string,
      d: number
    }
  }
}

const _example: Nestedthing = {
  a: {
    b: {
      c: "hi"
      // missing: d
    }
  }
}

Gives an error like:

$ /Users/orta/dev/projects/typescript/ts_examples/nested_interfaces/node_modules/.bin/tsc
example.ts(10,7): error TS2322: Type '{ a: { b: { c: string; }; }; }' is not assignable to type 'Nestedthing'.
  Types of property 'a' are incompatible.
    Type '{ b: { c: string; }; }' is not assignable to type '{ b: { c: string; d: number; }; }'.
      Types of property 'b' are incompatible.
        Type '{ c: string; }' is not assignable to type '{ c: string; d: number; }'.
          Property 'd' is missing in type '{ c: string; }'.

This example is pretty small, but the deeper the nesting, the longer the error message. These massages can get quite hard to dig right to the point:

An example from our codebase

src/lib/Components/Gene/__tests__/About-tests.tsx(45,40): error TS2322: Type '{ gene: { description: string; trending_artists: { __id: string; href: string; name: string; coun...' is not assignable to type 'IntrinsicAttributes & RelayProps & { children?: ReactNode; }'.
  Type '{ gene: { description: string; trending_artists: { __id: string; href: string; name: string; coun...' is not assignable to type 'RelayProps'.
    Types of property 'gene' are incompatible.
      Type '{ description: string; trending_artists: { __id: string; href: string; name: string; counts: { fo...' is not assignable to type '{ trending_artists: (string | number | boolean)[]; }'.
        Types of property 'trending_artists' are incompatible.
          Type '{ __id: string; href: string; name: string; counts: { for_sale_artworks: number; artworks: number...' is not assignable to type '(string | number | boolean)[]'.
            Type '{ __id: string; href: string; name: string; counts: { for_sale_artworks: number; artworks: number...' is not assignable to type 'string | number | boolean'.
              Type '{ __id: string; href: string; name: string; counts: { for_sale_artworks: number; artworks: number...' is not assignable to type 'false'.

We started teaching JS developers learning TypeScript to always "start at the end of a compiler message" - which isn't great ( this is compounded by VS Code only supporting single-line error messages in the problems section )

I wonder if it's possible to improve the error messaging to include a summary before the tree of 'how we got to that error'?

For example:

A pretty simple attack could be to take all the leaf nodes and show them beforehand in a separate section:

/Users/orta/dev/projects/typescript/ts_examples/nested_interfaces/node_modules/.bin/tsc
example.ts(10,7): error TS2322: Type '{ a: { b: { c: string; }; }; }' is not assignable to type 'Nestedthing'.

  example.ts(13,4): Property 'd' is missing in type '{ c: string; }'

  From incompatible property `a` in:
    Type '{ b: { c: string; }; }' is not assignable to type '{ b: { c: string; d: number; }; }'.
     
    From incompatible property `b` in:
        Type '{ c: string; }' is not assignable to type '{ c: string; d: number; }'.

This:

  • Highlights the exact place the problem occurs first as you're scanning
  • Could (one day) work nice with (Multi-line diagnostics vscode#1927), which means editors could highlight the exact place where something needs work
  • Is less visually dense, but still shows the same error

A similar example based error output based on the real life error include above also:

src/lib/Components/Gene/__tests__/About-tests.tsx(45,40): error TS2322: Type '{ gene: { description: string; trending_artists: { __id: string; href: string; name: string; coun...' is not assignable to type 'IntrinsicAttributes & RelayProps & { children?: ReactNode; }'.

  Type '{ __id: string; href: string; name: string; counts: { for_sale_artworks: number; artworks: number...' is not assignable to type 'false'.
  Type '{ __id: string; href: string; name: string; counts: { for_sale_artworks: number; artworks: number...' is not assignable to type 'string | number | boolean'.
  Type '{ __id: string; href: string; name: string; counts: { for_sale_artworks: number; artworks: number...' is not assignable to type '(string | number | boolean)[]'.

  From incompatible property `trending_artists` in:
    Type '{ description: string; trending_artists: { __id: string; href: string; name: string; counts: { fo...' is not assignable to type '{ trending_artists: (string | number | boolean)[]; }'.

    From incompatible property `gene` in:
      Type '{ gene: { description: string; trending_artists: { __id: string; href: string; name: string; coun...' is not assignable to type 'RelayProps'.

We're happy to take a stab at implementing this, but you all have the most context around edge cases and what you want the messages to say.

/cc @damassi @alloy

@orta
Copy link
Contributor Author

orta commented Dec 24, 2017

Sorry for poking over the holidays on this issue, but I was interested in trying to get this done during the holidays when (my) workload is low 💃 🕺

@mhegazy
Copy link
Contributor

mhegazy commented Jan 11, 2018

Given the way structural type checks happen, it is not clear that reversing the order of the messages would be ideal in the general case. in this specific case the absence of d was the interesting part. but consider another case where assigning a value of type A to another of type B, having the top of the stack something like property x is missing might not be what you want.

We are always looking for ways to make the errors more useful/approachable. so we are open to experimentation with new approaches.

//CC @DanielRosenwasser

@mhegazy mhegazy added Suggestion An idea for TypeScript Domain: Error Messages The issue relates to error messaging labels Jan 11, 2018
@arciisine
Copy link

I'm facing similar issues, and I would imagine with the addition of conditional typing, this problem is only going to increase in complexity.

Given the example in the original problem space, would it be possible to indicate the specific problems as individual errors? This would easily allow indicating the errors as close to it's source as possible.

Meaning, for an error on a nested property, you might see:

const _example: Nestedthing = {
  //  Types of property 'a' are incompatible
  a: { // Type '{ b: { c: string; }; }' is not assignable to type '{ b: { c: string; d: number; }; }'.
    // Types of property 'b' are incompatible, 
    b: { // Type '{ c: string; }' is not assignable to type '{ c: string; d: number; }'
      // Property 'd' is missing in type '{ c: string; }'.
      c: "hi"
    }
  }
}

As I think on this more, this should be doable in most editors assuming the error messages are structured in a way to allow for this, though output from tsc might be more useful in the format above. This may also have some overlap with #22170 as they are both trying to bring clarity to error messages in nested objects.

@orta
Copy link
Contributor Author

orta commented Mar 21, 2018

vscode is going to support multi-lined errors - so also thinking about how this could be shown as a hierarchy of errors could be useful too.

@alloy
Copy link
Member

alloy commented Mar 21, 2018

Might also be a good idea to see if there’s inspiration that could be drawn from Flow’s new error messages https://medium.com/flow-type/better-flow-error-messages-for-the-javascript-ecosystem-73b6da948ae2.

@spion
Copy link

spion commented May 10, 2018

A concrete suggesion I think works well: Ignore all parts of the type structures that are compatible until the first incompatible type with 2 or more incompatibilities. Show the types along the path that have a name, hide those that don't.

example.ts(10,7): error TS2322: Type '{ a: { b: { c: string; }; }; }' is not assignable to type 'Nestedthing'.
  Types of property 'a' are incompatible.
    Type '{ b: { c: string; }; }' is not assignable to type '{ b: { c: string; d: number; }; }'.
      Types of property 'b' are incompatible.
        Type '{ c: string; }' is not assignable to type '{ c: string; d: number; }'.
          Property 'd' is missing in type '{ c: string; }'.

becomes:

example.ts(10,7): error TS2322: Incompatible assignment: the property path 'a.b.d' of type 'number' exists in 'Nestedthing' but not in '{ a: { b: { c: string; }; }; }'
  Expected types along the property path of Nestedthing:  
    {...}, {...}, number
  Found types along the property types of '{ a: { b: { c: string; }; }; }'
    {...}, {...}, void

@orta
Copy link
Contributor Author

orta commented Aug 14, 2018

The TS 3.0 error messages fix this well! I'm going to close this issue 👍

Thanks all.

@orta orta closed this as completed Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Error Messages The issue relates to error messaging Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants