Replace /* @flow */ with entry in .flowconfig

[cletusw]

When introducing Flow to legacy code, it's nice to specify on a per-file basis what files Flow should analyze. Thus, we have the /* @flow */ or /* @flow weak */ comments at the top of each file. Once Flow is widely used in a project, I imagine that this would become extremely tedious. Or worse, someone may accidentally fail to include the comment and then we'll have no protection on that file with no warning that we don't. I suppose flow check --all exists for these cases, but that's an extremely inflexible solution.

Couldn't all these cases (analyze a few files, most files, or all files from a project) be solved more simply by providing a way to specify a glob or regex in .flowconfig that determines what files to analyze? That's what I assumed the [include] option was for, but it looks like it still requires the files listed to include the /* @flow */ comment (so it's more for checking files in a folder outside the one where .flowconfig is located).

[cletusw]

Related: #154

[monolithed]

+1

[izaakschroeder]

+1

[jasonkuhrt]

I agree that this is an issue. Today Flow works such that UX for existing projects is better than UX for new projects. This is clearly a false dichotomy. With a simple toggle it should be possible to inverse the system such that e.g. /* @flowoff */ opts out.

[jeffmo]

We've talked about this a bit in team meetings as well but haven't come up with solutions to all of the issues yet. One of the most glaring concerns is how to deal with things like npm libraries that come in with this config assumed set (but the holistic project not -- or vice versa).

Managing nested .flowconfigs is one option, but will likely be a tricky thing to solve (For example: what if an outer .flowconfig says to ignore something that's inside the domain of an inner .flowconfig project? etc...).

Or worse, someone may accidentally fail to include the comment and then we'll have no protection on that file with no warning

One thing we did recently that should help with this is to make it a parse error for a file to have type syntax if it lacks a // @flow toggle -- so at least it should be made clear if you forgot the tag in all but the most edgy of cases. (Note that you can put // @noflow if you want to explicitly disable flow temporarily or whatever, but not get errors about type syntax)

All in all, I think we need to be a little careful with the decision-making here (both with regard to how shared flow libraries can interact with each other so as to not hurt the ecosystem, but also with regard to how best it makes sense to use Flow in various workflows like starting-from-scratch vs partial-conversion of a project)...but it's clear that it's something that people want, so we're definitely open to talking this through.

[cletusw]

I haven't checked, but now that we have @noflow, is it possible to do as @jasonkuhrt suggested and enable flow by default for all files that match a regex specified in .flowconfig with each file retaining the option to opt-out?

[caesarsol]

Hello, am I wrong or this is fixed already?

[options]
all = true

https://flowtype.org/docs/advanced-configuration.html#options

Seems that this also includes big directories such as node_modules, they may be excludable by [ignore]

[cletusw]

ignore gets you part of the way, but the use case I would like would respect include as well. Has that been fixed?

[kamek-pf]

I like the idea. If changing the behavior of include is too risky, maybe adding something like include-all or include-untagged could do the trick ?

[russellmcc]

I really want this! It stinks to have that ugly @flow comment and to be afraid you'll forget it and have horrible errors without knowing.

I agree that there's some complexities with libraries. However, I think it makes sense to add the option without coming up with a plan for flow-checked libraries; since it would still be useful for application developers. Library writers could continue to use the comments or the .js.flow methods.

Eventually, it seems like there'd have to be some sort of of recursive .flowconfig parsing, or else some sort of preprocessed module-like system

[xixixao]

I agree this sucks for new projects. We should add

[force]
dir/

that forces flow checking on all files in dir (non-recursive, you can use a glob for that, like other options). I don't think we necessarily have to fix the recursive .flowconfig issue first, library authors should be discouraged from using this flag, yet it would be still valuable for non-library projects.

[MoOx]

It seems that using --all + @noflow is not working as expected. Files containing @noflow are still analysed.

[kamek-pf]

I tried flow check --all again on 0.38 today, laptop ran out of memory and crashed.
Still no good solution for this.

[force] looks like a good idea though.

[eirikurn]

As a workaround, we created an eslint rule to check and remind developers to write the @flow comment. It even supports eslint --fix to automatically add it.

Eslint has systems to apply this rule to specific files or folders.

Hope this issue gets solved as I don't really care for this comment in our non-library source files.

EDIT:
eslint-plugin-flowtype also has a rule like this.

[ndbroadbent]

@eirikurn Thanks very much for that! eslint --fix saved me a ton of time, and I'm glad I won't forget to add this to any files.

But yeah, it would be better if I could just include every file in my src/ folder.

[iddan]

I created a POC that uses the AST to detect imports and follow them with Node.js
https://github.com/iddan/flow-imports

[iddan]

@jeffmo with projects like this: https://github.com/davidtheclark/cosmiconfig a lot of the work is already done. I'm sure we can come up with a solution to how to cascade configs.

Checking from entry should work well in standard JS module environments (i.e: ECMA modules) as modules are easy to follow.

Right now flow fails because of conflicting flow versions in third party modules and this is a BIG problem.

Entry based resolution will be a massive improvement to Flow performance and will prevent bloated projects with third party modules errors. Because of that I think this feature should be implemented ASAP.

[iddan]

Is 5.7.3 promotes this feature?

[TrySound]

@iddan 0.57.3 and nope. But node_modules are checked only if there is an import in non-node_modules files.

[iddan]

Can there be an option that focus-check will check all source files and only flow files from node_modules?

[alaboudi]

This feature makes a lot of sense. Not too sure about the similarities involved, but would it not be reasonable to backwards engineer how typescript does this? They seem to already have this feature

[gabrielenosso]

@TrySound The point is that using a comment (// @flow) in the begin of a file is not just "unelegant", but it brings different problems in big projects.
(same reasons why JS linters also abandoned that solution and started using a configuration file)

1. How can developers know which files are using types without opening them? (that's why TypeScript is using a different extension)
2. To force using types in all source files, the projects should use other tools or configurations

My use-case is easy:
I have a private package, which uses Flow types.
I am exporting the types in the library itself, using *.js.flow files.

The solution "all=true" doesn't work, cause when you use it, you have to ignore "./node_modules/", otherwise Flow will find a lot of errors in all the dependencies.

But if you ignore node_modules, then it doesn't find the "*.js.flow" files delivered with the private package.

All other JS tools (babel, webpack, eslint) have a way to select the destinations in which they need to be used.

[TrySound]

@gabrielenosso To force having // @flow you need to use this lint rule. Flow doesn't force to use custom extension to make flow covering less noisy and doubtful. Knowing which file has types and which not is not quite important. Try to cover as much as possible and then force everybody else.

[apsavin]

The solution "all=true" doesn't work, cause when you use it, you have to ignore "./node_modules/", otherwise Flow will find a lot of errors in all the dependencies.

BTW, you don't have to ignore node_modules. You can use declarations section. See #4916

[gabrielenosso]

@TrySound forcing developers to insert a comment at the begin of each file is not the same as having a configuration which includes the source files.

1. It needs to configure another eslint rule, with another eslint plugin
2. Developers would need to add // @flow even for a simple configuration file (and it would look horrible and without any sense).
   Imagine something like config.js: export default { setting1: true, setting2: 'yeah' };
   with a "// @flow" on it... A developer not familiar with Flow would go crazy
3. Tools shouldn't be configured in the files themselves. It's not just elegant, but extremely confusing for everyone who is not used to the tool itself.

For big teams, or in my case, where developers are coming from other languages, it results extremely confusing.

In my opinion, it's a game changer on the selection of the types tool.
And considering the number of reactions at the first comment, maybe it's not only my concern.

@apsavin I didn't know about the [declarations].
It should be added into the .flowconfig file that "flow init" creates.
Anyway, a configuration setting to force flow in specified folders seems still to be the most elegant solution.
I don't think it's a coincidence if all other JS most famous tools are using that approach.

[TrySound]

@gabrielenosso

1. Flow will force you, not eslint
2. This lint rule will prevent using untyped modules so you should specify // @flow at least in entry point. This means configurations won't be forced since they are entry points.
3. Annotations are used all the time. We disabling eslint rules in place and we specify types in place.

Please don't talk about elegancy, it's highly subjective term. My team lead at previous work said that short code is more elegant than readable. Well, I disagree that 60 lines of unreadable code are better than 100 lines of really simple code.

[gabrielenosso]

@TrySound Sorry, elegant was probably not the right term.
I was just talking about "understandability".

A comment at the top of the file is not the best way, imho, to explain that the file can contain type declarations in a language that doesn't have them.

I have this problem cause I am driving my company into the JS world, and all developers here are used to completely different languages (from C++, to native mobile languages, to .NET).

And I think having a configuration setting for that would be also a step to align to the other popular JS tools.

Anyway, I understand that it's not a "real problem" (especially in the new version where [declarations] seems to be a workaround for my case), but it would be surely a game changer to let more people choose Flow as the JS types tool.

Thank you for your time :)

[apsavin]

@mrkev what do you think about the topic?

[mrkev]

Hmmm, radom thoughts here:

Addressing @gabrielenosso's comments:

1. How can developers know which files are using types without opening them? (that's why TypeScript is using a different extension)

If you really want to, you can configure your own extensions for flow too ¯_(ツ)_/¯.

Imagine something like config.js: export default { setting1: true, setting2: 'yeah' }; with a "// @flow" on it... A developer not familiar with Flow would go crazy

I don't think it's as big of a deal, no one should go "crazy". And if a comment line on the top of the file does make a dev in your team go crazy, I doubt the // @flow is the issue in this situation. That dev needs vacations.

Tools shouldn't be configured in the files themselves. It's not just elegant, but extremely confusing for everyone who is not used to the tool itself.

#!/bin/bash is pretty standard. The filename is part of the "file", and it also configures which tools are necessary to work with it.

Anyway, I understand that it's not a "real problem" [...] but it would be surely a game changer to let more people choose Flow as the JS types tool.

I wouldn't go as far as to say game-changer, but I suppose it'd fit some people's use cases nicely, yeah.

@flow makes it easy to gradually add JS to a codebase because:

1. Unlike a file extension, it allows you to be more expressive: @flow weak, @flow, @flow strict and whatever comes in the future would be much more annoying to use otherwise.

2. You don't need to rename files or worry about progressively longer lists and more complex globs to match exactly what you want typed vs untyped.

3. The behavior all determined in the file your'e on. If a developer sends a PR and it doesn't build, it's always due to stuff in that file, and not some change on a completely unrelated file (the .flowconfig).

4. It's easy to read: someone working on a file will know right away if it's typed or not, without having to navigate a .flowconfig, or learn how globs work.

5. It's easy to explain how to turn on typing on files. Plus you'll get 99% less merge issues, because typing a codebase won't require everyone to modify the same .flowconfig.

6. It's easy to find all files that are typed/untyped using standard file-searching operations. Untyped files? find . -type f -name "*.js" | xargs grep -L "@flow.

[gabrielenosso]

@mrkev First, thank you for your time :)

I try to reply to some of your comments:

If you really want to, you can configure your own extensions for flow too ¯_(ツ)_/¯.
I didn't know it :) How?

I don't think it's as big of a deal, no one should go "crazy". And if a comment line on the top of the file does make a dev in your team go crazy, I doubt the // @flow is the issue in this situation. That dev needs vacations.
That's ok, it was just to highlight that in same case it could result "strange".
But we are not talking about removing the possibility of using "// @flow", we are just talking about adding a way to configure Flow globally in a project in an easy way.

#!/bin/bash is pretty standard. The filename is part of the "file", and it also configures which tools are necessary to work with it.
#!/bin/bash defines how to execute a single script. It's a completely different use-case, compared to a JS project, where you can have a lot of files. And anyway, it's quite old :)

I wouldn't go as far as to say game-changer, but I suppose it'd fit some people's use cases nicely, yeah.
I searched for a solution about that, and I found a more issues on GitHub waiting for this
(
#284,
#869,
#4916 (declarations)
)
You can clearly see that more people are waiting for the "declarations" feature as a workaround for this.

About your points in the list: I completely agree with you on all of them.
I am absolutely not saying that Flow should change the way it works.
I am just saying that if it would support a feature to configure it globally in an easy way, aligned with the other popular JS tools, it would gain for sure more popularity. :)

[mrkev]

Yup, it would be an interesting feature to support, but I that sentiment I outlined is why it's not a priority. For using custom extensions btw: https://flow.org/en/docs/config/options/#toc-module-file-ext-string

[alexandersorokin]

I use following solution which work great:

1. .flowconfig file is placed to src folder.
2. node_modules and flow-typed folders resides in root folder.
3. Option all=true is enabled in .flowconfig file.
4. flow-typed typings is included in .flowconfig file.
5. Well-typed modules from node_modules folder that contain module own typings is included to .flowconfig file manually.

So .flowconfig file look like that:

[include]
../node_modules/mobx
../node_modules/other-well-typed-module-with-typings

[libs]
../flow-typed/npm
../flow-typed-generated-stubs/npm

[ignore]
# Nothing ignored

[options]
all=true

As a result:

1. I do not need annotate any file with //@flow header. All my files are type-checked by default. So there are no more mess with forgotten headers and untyped files.
2. Type declatations from flow-typed repository is supported. They are placed to flow-typed folder and can be downloaded automatically on each build.
3. Well-typed modules that includes module own typings is also supported. Path to such modules is included in .flowconfig file.
4. flow-typed declarations stubs is used for bad-typed modules. Generated or created declarations is placed to flow-typed-generated-stubs. So bad-typed modules do not affect type checking.
5. No overhead on type-checking entire node_modules directory.
6. I do not ignore anything by default. And that is good because in future I can use [ignore] section for excluding some module js-files if only some of module js-files is well-typed and other is bad-typed.

There are one disadvantage of described solution. I need to include all well-typed modules (in node_modules folder) to my .flowconfig file. It is not a big issue because there are only a few well-typed modules currently.

P.S. Do not try specifying more exact path for well-typed modules. For example, including ../node_modules/mobx/lib instead of ../node_modules/mobx is bad idea. It will not work. Currently I do not know the reason of that behavior.

[cletusw]

@alexandersorokin yeah, I mentioned --all in the question which I'm assuming is the same as all=true in the config. My point was more where you still need the ability to only type-check a subset of your files, but you'd rather specify which in one place (for example, src/app/* is typed but src/util/* is not).

[schumannd]

I still think it is an important feature to enable type checking in a whole folder/project by default. Despite reading this thread, I don't understand why it is not done.

Especially since any RN project comes with a default .flowconfig where all=true is unusable without going through all dependencies.

[vicapow]

Hello all,

I believe the existing mechanisms using,
all=error or eslint-plugin-flowtype and the require-valid-file-annotation rule should address all of the use cases but let feel free to re-open or bump this thread with your use case, if not.

[Hubro]

Hello all,

I believe the existing mechanisms using,
all=error or eslint-plugin-flowtype and the require-valid-file-annotation rule should address all of the use cases but let feel free to re-open or bump this thread with your use case, if not.

I disagree.

I want flow to run as if I had // @flow in all my source files. It's just absolutely stupid to have to include a tag in all your source files.

all=error makes it run as if I had // @flow in all my source files and all source files under node_modules/, which results in tons of errors. Adding node_modules to [ignore] makes flow completely ignore those source files, even when they are imported by my code.

As far as I can discern from this issue, there's currently no way to achieve this, which blows my mind. It sounds so simple. Just add a config option to specify files that flow should treat as if they included // @flow.

[check]
./src
./tests

I don't understand what the problem is.

[xixixao]

I absolutely second your explanation @Hubro. In fact nothing has changed here since jeff's comment three years ago (which explains "what the problem is" and my comment two years ago (which had the same suggestion)...

[Hubro]

I absolutely second your explanation @Hubro. In fact nothing has changed here since jeff's comment three years ago (which explains "what the problem is" and my comment two years ago (which had the same suggestion)...

None of the problems he lays out applies to my proposed solution though, unless I'm missing something. Just add a setting that will sneak in // @flux to all matching source files before type checking starts, like a pre-processor.

[goodmind]

@Hubro what's wrong with ignoring node_modules, you can ignore everything and then use negation to un-ignore

[Hubro]

@Hubro what's wrong with ignoring node_modules, you can ignore everything and then use negation to un-ignore

That is not equivalent to adding // @flow to all files in a project.

Quoting @gabrielenosso from higher up in the issue:

The solution "all=true" doesn't work, cause when you use it, you have to ignore "./node_modules/", otherwise Flow will find a lot of errors in all the dependencies.

But if you ignore node_modules, then it doesn't find the "*.js.flow" files delivered with the private package.

[goodmind]

It's a workaround, but it works

[ignore]
./node_modules
!./node_modules/immutable

[options]
all=true

[Hubro]

@goodmind A feature this basic should absolutely not require a workaround.

[goodmind]

@Hubro this PR #7317 was specifically made to solve it, sorry, it's not a workaround

[Hubro]

@goodmind That's still nowhere near equivalent to adding // @flow to all project files, it's absolutely a workaround. With this approach you have the chore of keeping the node_modules whitelist up to date as the project evolves.

[goodmind]

Ok, makes sense. I just don't think adding new section makes sense, maybe [include] can be fixed with all=true

[FezVrasta]

Shouldn't a configuration like this work?
When I try it, Flow takes ages to run, I didn't have enough time to let it finish, but I assume declarations is not respected.

[options]
all=true

[declarations]
<PROJECT_ROOT>/node_modules