facebookarchive / ide-flowtype

Flow support for Atom IDE
Other
178 stars 17 forks source link

Doesnt produce full stack trace -- creates confusing errors #39

Open bradennapier opened 6 years ago

bradennapier commented 6 years ago

So I just thought I ran into a flow bug and until i ended up trying it in the flow editor I didn't realize it was just flow-ide completely omitting the errors that actually matter. Definitely appears to be a fairly straight forward UI issue.

I was trying to see why interface/implements did not seem to actually do anything. I have ran into this in the past with confusing error messaging - the new debug panel has helped and a few other improvements have come along that have been great.

However, this is one that really causes problems. Until recently I hadn't thought to always run flow separately when the errors come up to see what is actually happening and I would just have to "figure out" why the errors occurred by studying the code.

interface Foo {
  channel?: number,
  test(foo: number): number,
}

export class Bar implements Foo {
  constructor() {
    this.channel = 2;
  }
  test(foo) {
    return 'hi';
  }
}

Now the problem in this example is pretty obvious. Aside from the annoyance that interface doesn't automatically type channel property in this case, I can at least understand that.

Here is what flow-ide shows as the errors. It is enough to understand there is a problem, but it is missing the actual information about the problem. It appears it simply runs out of space to report, so I definitely understand it's simply a UI issue that needs to be considered.

image

image

Here is the piece it is missing which is what I would like to see available:

export class Bar implements Foo {
                               ^ Foo
Property `test` is incompatible:
12:   test(foo) {      ^ function. This type is incompatible with
5:   test(foo: number): number,
     ^ function type
This parameter is incompatible:
13:     return 'hi';
               ^ string. This type is incompatible with
5:   test(foo: number): number,
                        ^ number
bradennapier commented 6 years ago

On re-enabling nuclide I realized that it has solved this already and it's done so in an awesome way actually. Hoping this can make it into flowtype-ide! In the meantime I will just switch back to nuclide.

image

image

wbinnssmith commented 6 years ago

hi @bradennapier -- we're aware of this and I believe @calebmer is working on a solution. thanks for the report :)

wbinnssmith commented 6 years ago

oh, just saw that your concerns have been addressed in Nuclide. I'll see if I can pinpoint the change and port those changes (and any others) here.

adamterlson commented 6 years ago

I noticed something similar where the error shown in ide-flowtype omits the file paths (of source and usage) in the error report where it's present both on the command line and in nuclide. In my case, the error is in passing invalid props to a react component.

Is this caused by the same error, or is that a separate issue?

Soreine commented 6 years ago

@wbinnssmith that would be very nice. It looks like the missing piece to achieve full functionality.