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

Report error which Flow does not report #71

Open
pizzafroide opened this issue Mar 10, 2017 · 8 comments
Open

Report error which Flow does not report #71

pizzafroide opened this issue Mar 10, 2017 · 8 comments

Comments

@pizzafroide
Copy link

This use case is where, in C/C++, a forward declaration would be needed.

A.js:

// @flow

import B from './B';

export default class A {
  a() { return new B(this); }
}

B.js:

// @flow

import type A from './A';

export default class B {
  a: A;

  constructor(a: A) {
    this.a = a;
  }
}

Flow says No errors! but flowtype-errors/show-errors reports the following in A.js:

6:23  A:  This type is incompatible with the expected param type of A. See ./B.js:8
@amilajack
Copy link
Owner

This use case is where, in C/C++, a forward declaration

Not sure what C/C++ has to do with flow. Can you please elaborate on this. Is there a feature of C that you want in flow?

@pizzafroide
Copy link
Author

pizzafroide commented Mar 10, 2017

Is there a feature of C that you want in flow?

Not at all. I'd just like flowtype-errors to not report an error when Flow does not. The C++ reference was just intended to explain the use case. More specifically, type checking class A needs B type to be known, and type checking class B needs A type to be known. (That's where, in C++, a forward declaration is needed.)

Again, Flow says No errors! while flowtype-errors/show-errors reports something.

@amilajack
Copy link
Owner

amilajack commented Mar 25, 2017

@pizzafroide would be great if you could make a PR for this

Here's an example
#68
#74

@pizzafroide
Copy link
Author

@amilajack I'm a little busy right now and have no time to address this issue. For now, I've just added a note telling that this kind of error is actually OK. But I'll consider making a proper PR later on. Thanks for the links to the related issues anyway.

@jdmota
Copy link
Collaborator

jdmota commented Mar 27, 2017

It is an issue with flow. Not sure if we can find a work around...

@amilajack
Copy link
Owner

Closing this, as we're waiting on flow. @jdmota Thanks for finding this!

@micaste
Copy link

micaste commented Oct 16, 2017

Hi,

It's been more than a year since the issue was reported to Flow and it is still open.
Could we think about a way of solving it from this side ?

I understand that the first purpose was to save processing time by running Flow on only one file. But the Flow process is actually blazingly fast, so wouldn't it be conceivable to spawn the Flow process once at the start of the eslint process, then filter the output for each file ?

I tried to run Flow without the check-contents flag (which is the one causing the issue), and thanks to the --json option we get the errors that we can filter by file, in order to only keep the ones for the current file !

This would solve the issue at stake here. The difference I see is that we are letting Flow reading the content of the file from the filesystem, instead of feeding it from context.getSourceCode().getText(). Does anyone know if there is a feature of eslint for which it could be an issue ?
I heard that the IDEs use the check-contents feature to run Flow on unsaved files. Is that possible with eslint ?

What is your opinion @amilajack ?

@murrayju
Copy link

murrayju commented Sep 8, 2018

Another year has gone by, and I'm still running into this issue. I've resorted to sticking // $FlowFixMe in places where it isn't really needed by Flow. Does anyone else have a better workaround?

@amilajack amilajack reopened this Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants