Flow: trying to silence third-party errors with [declarations] seems to re-raise ignored errors instead

Created on 9 Aug 2018  ยท  18Comments  ยท  Source: facebook/flow

I try to use flow in my project and suppress errors from dependencies.
My project uses react-native-firebase, when I run flow I get 3 errors inside node_modules/react-native-firebase.
I wanted to silence them and have extended the .flowconfig with

[declarations]
.*/node_modules/react-native-firebase/.*

But now I get new errors, which are normally silenced, for example:

Error โ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆโ”ˆ node_modules/react-native-firebase/dist/modules/firestore/DocumentReference.js.flow:54:53

Cannot call CollectionReference with parentPath bound to collectionPath because null [1] is incompatible with Path [2].

     node_modules/react-native-firebase/dist/modules/firestore/DocumentReference.js.flow
     51โ”‚   get parent(): CollectionReference {
     52โ”‚     const parentPath = this._documentPath.parent();
     53โ”‚     // $FlowExpectedError: parentPath can never be null
     54โ”‚     return new CollectionReference(this._firestore, parentPath);
     55โ”‚   }
     56โ”‚
     57โ”‚   get path(): string {

     node_modules/react-native-firebase/dist/modules/firestore/CollectionReference.js.flow
 [2] 29โ”‚   constructor(firestore: Firestore, collectionPath: Path) {

     node_modules/react-native-firebase/dist/modules/firestore/Path.js.flow
 [1] 36โ”‚   parent(): Path | null {

The 3 original errors are also still reported.

I am running flow 0.78.0

bug

Most helpful comment

The re-raising of ignored errors is due to https://github.com/facebook/flow/pull/4916/files#diff-c600001e37d25814dbc857496fecd26dR351-R359

Notice the intent was to either remove all errors, or scan for error & lint suppressions. But due to the semicolon (and indeed due to the lack of begin/end in the else branch, scan_for_lint_suppressions actually still runs. I believe, in fact, both should run. I'm trying to add some new failing tests now and see if I can get it in a workable position, but I'm not sure Context.remove_all_errors cx really does what we want it to.

All 18 comments

Use [untyped].

@vjpr [untyped] has a different purpose, it will ignore all the types of the matching modules and replace them with any. while [declarations] will use the module types, and ignore any error happening inside them

@timri Do you have $FlowExpectedError defined in your flowconfig? Out of the box there is only $FlowFixMe

@TrySound I got the same error in #7388 and I was using the default $FlowFixMe

I'm having the same problem. The desired behavior is to use any type information provided in node_modules when possible, but to never report errors that occur within the node_modules. I only want to see errors that occur in my project's source.

The documentation reads like [declarations] should do exactly what I want, but it seems to do the opposite. Is there any way to write the configuration now to get the behavior that I want? It seems to me like this should be the default behavior. We should be encouraging projects to publish their flow type annotations in their npm packages, and by default leverage this information to improve typing in a project. But this should not result in seeing error output for a project in external dependencies that can't be fixed.

something's broken, trying to figure out what happened. indeed, errors within [declarations] should not be reported, and the annotations should be taken at face value.

I think it's only a problem inside cycles.

Any update on this issue? I am seeing this on 0.92

Also seeing this problem on latest Flow, 0.94

Still reproducing on 0.104.0.

@mroch - could you explain what you mean by "inside cycles"? I'm wondering if knowing what this means would help me work around the issue.

Is it correct to infer this feature doesn't carry any unit test to make sure it doesn't break?

Id love to get this addressed!

The re-raising of ignored errors is due to https://github.com/facebook/flow/pull/4916/files#diff-c600001e37d25814dbc857496fecd26dR351-R359

Notice the intent was to either remove all errors, or scan for error & lint suppressions. But due to the semicolon (and indeed due to the lack of begin/end in the else branch, scan_for_lint_suppressions actually still runs. I believe, in fact, both should run. I'm trying to add some new failing tests now and see if I can get it in a workable position, but I'm not sure Context.remove_all_errors cx really does what we want it to.

If you need a repro, I posted one here a while ago.

@STRML - Much respect for your work, thank you for being active with regards to this issue.

I think this can be closed. I tried it locally and seems to be fixed on master.

Now that v110 of flow-bin is out, this can definitely be closed.

(As you can see from the spam above. Sorry about that.)

I am seeing this bug for v0.145.0:

npm run flow --version
5.8.0

In my use-case, I have *.js and *.js.flow files in the same directory (same example as documented). But if I remove the types from the *.js file, it still shows the error even though my *.js.flow file is correct.

Putting the *.js listed as [untyped] has no effect. Even though types are fine in *.js.flow I still get:

Cannot build a typed interface for this module. You should annotate the exports of this module with types.
Missing type annotation at array pattern: [signature-verification-failure]

 1โ”‚ export function isLeapYear(year) {
 2โ”‚   return year % 4 == 0; // yeah, this is approximate
 3โ”‚ }
 4โ”‚

The benefit to keeping all of my typing in the *.js.flow file is I can keep vanilla js in my *.js file for better portability.

So to work around this issue, my implementation code is littered with: // $FlowIssue[signature-verification-failure]

Was this page helpful?
0 / 5 - 0 ratings