Flow: Flow 0.93: *EVERYTHING* is a "dynamic-export" issue all of a sudden

[lll000111]

Flow version: 0.93

Configuration contains

[lints]
all=error

Expected behavior

No or a few errors.

Actual behavior

From zero to 21350 errors going from Flow 0.92.1 to 0.93, and ALL of them are from "dynamic-export" for a very well-maintained project with "very clean Flow types" (spent a lot of time on it and religiously update and improve everything) with just about 6,000 lines of code. I only use any for a few type casts, when Flow does not understand something. There are no other any apart from that handful(!) of type casts. I also don't use Function or Object since it was deprecated. I don't have a single "stand alone" any, only as part of type casts, so it does not propagate anywhere.

Example:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ test/chum-sync-alice-test.js:27:1

Dynamic function type [1] unsafely appears in exported ProcessComResponse [2].
This can cause importing modules to lose type coverage! (dynamic-export)

     test/chum-sync-alice-test.js
      24│     data?: mixed
      25│ |};
      26│
 [2]  27│ export type ProcessComResponse = {|
      28│     id: number,
      29│     data?: mixed,
      30│     error?: Error
      31│ |};
      32│ */
      33│
      34│ const chai = require('chai');

     /tmp/flow/flowlib_3d06aeae/core.js
 [1] 445│     static captureStackTrace(target: Object, constructor?: Function): void;

This error message does not make any sense!

_None_ of the 21350 errors makes any sense. _None_ of those are even close to looking suspicious or being errors.

This new "dynamic-export" check reports _every single item_ in the entire code base! And it complains about issues with the built-in lib definitions, not even about the code.

Here is an example that shows that Flow reports its own library (where the any is located):

Error --------------------------------------------------------------------- src/access.js:279:23

Dynamic explicit 'any' [1] unsafely appears in exported async function [2]. This can cause
importing modules to lose type coverage! (`dynamic-export`)

   src/access.js:279:23
   279| export async function createObjects (
                              ^^^^^^^^^^^^^

References:
   /tmp/flow/flowlib_3d06aeae/core.js:697:91
   697|     map(callback: (currentValue: number, index: number, array: this) => number, thisArg?: any): this;
                                                                                                  ^^^ [1]
   src/access.js:279:14
                     v-----------------------
   279| export async function createObjects (
   280|     WriteStorage: WriteStorageApi,
   281|     accessRequests: Array<SetAccessParam>
   282| ): Promise<Array<VersionedObjectResult<AccessObj | IdAccessObj>>> {
        ----------------------------------------------------------------^ [2]

The createObjects function on my side of the code has a single array.map(someDate => someFunction(someData)) statement whose result is returned and no any anywhere, it all has concrete types.

Workaround

After adding dynamic-export=off to the [lints] section of .flowconfig all 21350 errors are gone.

[jbrown215]

cc @dsainati1

[dsainati1]

Hi. I'm sorry you've been having trouble with the new lint. This is in fact working as intended, but I can see how it is confusing. What's going on here is that the types you're exporting are defined in our libdefs to include any. In your first example, for instance, the type Error is defined in core.js to have the static function static captureStackTrace(target: Object, constructor?: Function): void, so you are implicitly exporting an any type here. The intention was not for this new lint to be enabled for all the files in a codebase, as it is quite noisy as you have seen. Rather we hoped that users would turn it on on a per-file or even per-line basis using flowlint comments where they wanted to be especially sure that no any types were exported from files. I will update the documentation to reflect this.

[Bannerets]

@dsainati1 dynamic-export still is working wrong, it seems.

Please reconsider.

Also export const n: number = 2 is working ok.

[dsainati1]

Hmm this second example definitely is wrong. I will look into this and get back to you.

[dsainati1]

I think the lint is complaining because we implicitly model this types as any. Unfortunately the new version just released, but I will try to get the fix for this into the next one. In the mean time, I'd just say turn this off.

[samwgoldman]

@dsainati1 Can we exclude this lint from the "all" setting?

[dsainati1]

Yeah I am working on a couple of improvements to the lint; and will include that as one of them.

[dsainati1]

Okay the referenced issue should fix the issue with being unable to export functions and classes once 0.94 releases; as far as the libdefs triggering the lint; I'm collecting data on this at the moment. It's possible we want this behavior in some scenarios.

[cprodescu]

Are there any plans to disable this by default (as part of [lints] all=error)?
In our experience, any flow-typed library will trigger tons of these issues.

[dsainati1]

I think it's pretty confusing for all=error to not actually set all the lints to be errors, so I don't know that we would want to ignore it as part of that setting. I do recognize though that the lint is difficult to use at the moment and I have some in progress work to make it less noisy (and hopefully easier to enable).

[samwgoldman]

I think it's pretty confusing for all=error to not actually set all the lints to be errors

There is some reasonable precedent for this. -Wall, for example, does not treat all warnings as errors. There is -Wall -Wextra for that, but even that does not treat 100% of warnings as errors.

Clang has a non-standard -Weverything flag, but admonishes users to never actually use it, explaining that it is designed for implementors only.

I think it's fair to imagine "all" as an alias for "all the lints that are reasonable to enable at once."

[nmote]

This was addressed in 91a49518d6053880b8b162a7a1a5aa5962ed4ea4, which will come out shortly with v0.98