Flow: Don't typecheck or load files under node_modules/ unless they're imported by flow-typed files

Created on 28 Sep 2015  Âˇ  186Comments  Âˇ  Source: facebook/flow

Most of the files under node_modules aren't the responsibility of my project, errors in them are probably due to them being written for a different Flow version, and the only reason to parse them would be if my project has flow-typed files referring to types in them.

It seems like a common issue is that Flow takes longer to start up than it ought to because it reads all of the javascript files under node_modules/ when the user isn't interested in typechecking most of them (#863). Tons of files under node_modules/ are sub-dependencies of other non-Flow-typed dependencies. People can manually ignore individual dependencies to speed things up (example), but this loses its effectiveness with the newly-released npm 3 because now dependencies aren't usually nested. (Browserify's many dependencies are now placed directly under node_modules/ instead of node_modules/browserify/node_modules/, so ignoring .*node_modules/browserify.* is no longer nearly as effective.)

It makes sense to check the flow types of files under node_modules/ if they're imported by flow-typed files in the project itself, and maybe the types of the files those ones include if it's necessary to fully figure out their types, but I think most/all users would prefer that files under node_modules/ would not be even loaded for typechecking unless they're imported by a flow-typed file in the project proper. Maybe this could be a (default-on?) setting in .flowconfig.

DX

Most helpful comment

Just adding my 2¢ - this is a big oversight. It seems to me I currently have two options:

  1. Go with the default settings so that imported npm modules get detected correctly, but see lots of flow errors from other packages that I do not maintain.
  2. Add .*/node_modules/.* to the [ignore] section of my flow config so that I no longer see those errors, but instead get a different flow error that it can't detect any npm packages I import.

Both of these seem pretty unacceptable to me. Is there any workaround?

All 186 comments

Also #719 and #835

This seems like a good idea to me, but it also sounds really hard, given how things currently work.

Also #863 for an easy way to make this problem more user-visible.

When I test my own code with Flow I'm not interested at all in type errors in the node_modules directory. I can't change anything about it anyway. But I do like to be able to import a module without having to declare the module interface for every NPM module I use. This is suggested in #719. I find it hard to accept this as a workaround... That means a lot of manual work for anybody using Flow.

Shouldn't node_modules be treated as a library in some way? As in: do not type check them or report on errors in them, but do find them and use them when imported.

I think most/all users would prefer that files under node_modules/ would not be even loaded for typechecking unless they're imported by a flow-typed file

I agree, but do you expect Flow to show you errors in those imported files? Personally I only want errors from my own code. Flow should be smart enough to understand that code in node_modules isn't something you have control over.

We made a big change here in the last release. Only @flow files will be parsed, which I suspect will improve this situation considerably.

@samwgoldman, thanks for the update! Just tested it and t does check a lot less files which is good! An option to totally switch it off for node modules would be great though.

@jamiter It's not (yet) very common, but files under node_modules can have type definitions that are used by your project. For example, my react-save-refs and ud modules both do, so by default you get type-checking on calls to them without any configuration needed. (babel-plugin-transform-flow-comments can be used to do this, though it has issues like T7054. Instead, I just include the babel-transpiled code in the NPM package, and then include the original source with the ".flow" suffix to the filename, which Flow reads instead. This is set up by the "prepublish" commands in the scripts section of the modules' package.json.)

@AgentME, thank you for the clarification. I'm new to FlowType and still getting to know all implementation details. I understand your use case, but it could lead to issues if some module is running another (out of date) flow version, right? In your case you can update your own module and push it to NPM, but it won't always be that easy.

So I still think that Flow should understand the types defined in node modules, but not show any errors in the command line about them. Only when your own code doesn't match any defined types (in node modules or your own code). How can you use Flow in CI when it throws errors about code you do not own?

If a newer version of Flow caused it to not understand the types in one of my modules, the error message would be more useful than silently failing so I knew there was something wrong. I can always add the module to my .flowconfig's [ignore] section. (I currently have to do that with the fbjs module and some babel modules which seem to have some invalid flow-typed code.)

Just to get this right: You can add it to your ignore section, but then you will get Required module not found errors. So you not only have to ignore them but also create a definition file where you declare the ignored module, correct?

The new version seems to ignore most node_module files now, which means none of them are found anymore. Which means a zillion declarations have to be written... And it seems like this is by design. So apologies for highjacking this thread, I'm just a little surprised that this is the case.

I'm very interested in using flow but I'm running into this issue as well. My project is reporting errors about files I can't really do much about. I feel like there should be an option to ignore errors in the "node_modules" folder. At first I thought [ignore] was exactly what I needed until I ran into "Required module not found" errors all over. I realize that flow needs to have knowledge of the files included in a project, but it would be nice if there was an option to silence errors found in the node_modules files.

If I submitted a PR for this, would it be considered?

@toddw What errors are you running into specifically?

node_modules/fbjs/lib/PromiseMap.js.flow:15
 15: var Deferred = require('Deferred');
                    ^^^^^^^^^^^^^^^^^^^ Deferred. Required module not found

node_modules/fbjs/lib/fetchWithRetries.js.flow:16
 16: var ExecutionEnvironment = require('ExecutionEnvironment');
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ExecutionEnvironment. Required module not found

node_modules/fbjs/lib/fetchWithRetries.js.flow:19
 19: var sprintf = require('sprintf');
                   ^^^^^^^^^^^^^^^^^^ sprintf. Required module not found

node_modules/fbjs/lib/fetchWithRetries.js.flow:20
 20: var fetch = require('fetch');
                 ^^^^^^^^^^^^^^^^ fetch. Required module not found

node_modules/config-chain/test/broken.json:11
 11:   "dependencies": {
       ^^^^^^^^^^^^^^ Unexpected string

node_modules/npmconf/test/fixtures/package.json:1
  1: 
     ^ Unexpected end of input

node_modules/y18n/test/locales/bad-locale.json:1
  1: {"hello": "worl}
     ^ Unexpected token ILLEGAL


Found 7 errors

@toddw Thanks! The solution in this case is to ignore the specific files that are problematic:

I haven't tested it, but this should work:

[ignore]
.*/node_modules/fbjs/.*
.*/node_modules/config-chain/test/broken.json
.*/node_modules/npmconf/test/.*

I've also sent an upstream change to fix fbjs, so the next version should no longer cause this issue. We're also working to fix the invalid JSON errors, so that shouldn't be an issue soon.

Ignoring everything in node_modules is problematic, because we look in there to a) ensure you've actually installed your dependencies and b) find Flow types for packages which might have included them.

Thanks @samwgoldman, if I use fbjs will I be getting the Required module not found error?

I've never written anything in OCaml so this is taking me longer than I hoped but I was thinking like the following:

screen shot 2016-03-04 at 5 58 35 pm

(sorry for the screenshot and not a legit PR but I'm just throwing around ideas)

Well, using fbjs isn't really advisable in general, but if you do want to I'd recommend waiting for the new release, which fixes the issue you ran into.

Regarding suppressing, I'm not sure how I feel about it. On one hand, we want to _fix_ those issues, not suppress them. On the other hand, lots of new users run into issues and making their path easier is important.

I don't want to discourage you from trying, but I'm not confident that a PR adding this feature would be merged.

Fair enough. fbjs is probably not the best example. I'm imagining a scenario where some other 3rd party component has flow errors. As flow gets wider adoption, as I'm sure it will, it seems inevitable that people will run into more issues. I didn't think [ignore] worked the way it does. I expected it to work as [supress] which I think should be considered.

Either way, I appreciate the time you took to address my issue and I should be unblocked by your suggestion.

Update, I've been using flow for the past 20 days and this continues to be an annoying problem.

node_modules/react-motion/src/spring.js:7
  7:   config = presets.noWobble): SpringConfig {
       ^^^^^^ parameter `config`. Missing annotation

I do use spring from react-motion but I don't want warnings from packages that I'm not maintaining. Has there been any more thought about this?

Just adding my 2¢ - this is a big oversight. It seems to me I currently have two options:

  1. Go with the default settings so that imported npm modules get detected correctly, but see lots of flow errors from other packages that I do not maintain.
  2. Add .*/node_modules/.* to the [ignore] section of my flow config so that I no longer see those errors, but instead get a different flow error that it can't detect any npm packages I import.

Both of these seem pretty unacceptable to me. Is there any workaround?

This is a pretty big pain point. Flow seemed like the nicest way to get some sort of static typechecking incrementally added to our JS codebase, but both "ignore all the module not found errors" and "ignore all the errors coming out of node_modules" aren't really things I want to say when trying to sell the team on Flow.

It's not just fbjs that's an issue:

node_modules/cjson/test/fixtures/templates/conf11tmpl.json:2
  2:     "subobject": {{subobject}},
                       ^ Unexpected token {

node_modules/cjson/test/fixtures/templates/conf12tmpl.json:2
  2:     "num": {{num}},
                 ^ Unexpected token {

I don't know if it's practical to ignore individual npm packages as they identify themselves as being problematic. I don't see any issue with ignoring the node_modules folder. I've done it and run flow and it works just fine.

In my projects I've had success ignoring problematic modules on a case-by-case basis. For example, fbjs, react-motion, etc. For those modules that do expose flow types and do not throw errors it is really nice to not have to write one's own type definitions for them. Simply use the modules as you would normally and everything will type check.

I'm not saying the current situation is ideal, but for larger projects I haven't had issues selectively ignoring node modules.

It's by no way ideal, but as an intermediate solution @iansinnott's approach has worked for me too. I'd also like to see this better resolved however. Hitting immediate hurdles like this one are not a great first impression, even if Flow is a really great tool.

I'm all for adding .*/node_modules/*. under the [ignore] directive, but I don't like seeing all the Required module not found errors. As @toddw and others have mentioned, this adds a lot of noise to Flow's output.

One solution would be to implement a syntax for ignoring certain classes of errors in the config file, like with eslint. For example, you could just mark Required module not found errors with a priority of 0, so that they don't get printed when running flow.

I take back what I said. Ignoring the whole node_modules folder actually did wind up causing an issue for me.

@samwgoldman

Regarding suppressing, I'm not sure how I feel about it. On one hand, we want to fix those issues, not suppress them. On the other hand, lots of new users run into issues and making their path easier is important.

I don't want to discourage you from trying, but I'm not confident that a PR adding this feature would be merged.

Who is the _we_ in this statement? Because the manifestation of this issue is that every consumer of a library with type errors is shouted at by flow, and it does not seem at all reasonable to suggest that all those users be expected to take responsibility for fixing upstream issues.

It's really just noise that distracts people from writing their own code - library maintainers will get their own errors if they use flow in their projects - it doesn't make sense to push those errors out to everyone.

I think I'm going to pull Flow from both of the projects I started to use it on. I'm just running into too many issues that wind up slowing me down rather than help me get my work done. I'll revisit later though, for sure.

IMO, I don't think there should be a separate project (flowtyped) to handle 3rd party libraries' typedefs. I think Flow should have some innate ability for library maintainers to export types and indicate to Flow (in a project using said libraries) how to find the types and incorporate them into the dev's project.

@ffxsam Libraries can include their own Flow types. It's not well documented but there's some information about how to do it here: https://github.com/facebook/flow/issues/1996#issuecomment-231619223. (The flowtyped project is for libraries that don't bundle their own types.)

Interesting, thanks! Maybe many library maintainers aren't aware of this. @ianobermiller - maybe Radium could add this so devs don't have to create declaration files?

What if rather than trying to come up with a way to skip type checking on these exernal files, there was an option to suppress the errors that are found in those files so that only errors you are "responsible for" are reported? I'm fine with flow scanning those directories and finding errors, but I don't really care to see it in the output

@joncursi I believe that's what the suggestion of a suppression mechanism was aimed at.

This is preventing me from using flow as well. I just ran flow on a preexisting react-native project and it returned ~160 errors, ~140 of which are from various modules I don't control. Would be awesome to see a suppression mechanism.

Hi there,

Any news on this? :)

Amusingly, I am mostly encountering third-party type errors on Facebook code, here is an excerpt of my [ignore] section ATM:

.*/node_modules/fbemitter/.*
.*/node_modules/fbjs/.*
.*/node_modules/flux/.*

I'm trying out Flow right now, and in all honesty I think I may not keep using it as it is because of this issue, although I was pretty excited to get started :(

Same problem here. It gets even worse if you try to use options.all=true to scan files that have no // @flow annotation. In this case flow process just uses 100% CPU and never finishes the validation (probably because it tries to scan every single file under node_modules)

Nearly all of the errors in dependencies I see any more come from purposefully malformed test json files that the dependencies didn't expect anyone outside of their test system to try to parse. I opened https://github.com/facebook/flow/issues/2364 as a smaller feature request to directly address this.

@AgentME This seems like a subset of all the errors. Why not address the root cause of the issue instead? How are JSON files different?

@Elijen seeing this too. For any serious project, I just want to check all of my files. I could never get flow check --all to complete.
This is definitely the kind of polish Flow needs to be an awesome tool.

Monitoring this thread closely.

@Elijen JSON files are special: Flow only parses javascript files that opt into it with the //@flow comment, and then it parses all JSON files. Flow used to try to parse all javascript files too, which often caused lots of issues with dependencies containing invalid javascript files. Flow was changed to only parse javascript files that opt into it, which addressed a large subset of the issue, but didn't change anything about how it deals with JSON files. If https://github.com/facebook/flow/issues/2364 were implemented, then the only issue left would be dependencies that specifically use Flow types but have real Flow errors (and this case is a little debatable about how it should be handled).

Flow is a non-starter if I'm going to get bombarded with errors before I've added a single annotation to any of my files.

Also, flow check --all hard-crashed my computer (frozen mouse).

I'm in the same boat as @mnpenner -- the all option actually took down my macOS system completely, which was quite a surprise!

You should never use --all property if you don't ignore node_modules. It's better to avoid it entirely

@vkurchatkin That's the point. You can't ignore node_modules because it produces import errors. You can't use --all because it crashes computer. So --all at the moment is useless.

However it makes sense to check all files (including files without // @flow annotation) in many cases.

As someone coming from a React Native wanting to integrate Flow into my own project, I'm pretty surprised about this. A very basic project with some dependancies (redux etc), running the flow check is giving me 316 errors, which are pretty much all in node_modules.

Is the flow team even acknowledging this as an issue? This is the only issue stopping me from using flow — maintaining a list of exceptions each time one of my node modules starts throwing up errors is cumbersome and I cant add flow check to my CI.

I think it's clear by now that users of flow do not expect errors from files they do not control.

Regarding suppressing, I'm not sure how I feel about it. On one hand, we want to fix those issues, not suppress them. On the other hand, lots of new users run into issues and making their path easier is important.
@samwgoldman commented on Mar 5

The issue is we _can't_ fix them. It seems to be a showstopper for many and I really think this issue should be addresses soon before flow starts to loose a lot of (new) users. The thread above proofs this is already the case.

I mean the fix should be relatively easy, right? Just a configuration option to suppress errors from node_modules. Would this solve all the issues?

@Elijen That has been mentioned, but apparently it's not "as simple as that".

The command flow check is pointless for me at the moment. The only saving grace of using Flow in our work flow right now is that our IDE (Webstorm EAP) supports Flow, so you get live Flow warnings which doesn't come up in node_modules, only _my_ code.

@Elijen
I'm assuming the problem with suppressing errors in node_modules is that the errors could (confusingly) show up in your code when you make use of the libraries with those broken flow definitions.

There must be more to it than that though, because you could still provide the option even with that caveat. It'd do more good than harm while a more elegant fix is worked on.

Running into same problem here. Wanted to get stared with flow because it looks better than typescript but after seeing this thread I might try typescript instead and see if that works for me.

../node_modules/conventional-changelog-core/test/fixtures/_malformation.json:1
  1:
     ^ Unexpected end of input

../node_modules/fbjs/lib/Deferred.js.flow:60
 60:     Promise.prototype.done.apply(this._promise, arguments);
                           ^^^^ property `done`. Property not found in
562: declare class Promise<+R> {
     ^ Promise. See lib: /private/tmp/flow/flowlib_32860485/core.js:562

../node_modules/fbjs/lib/shallowEqual.js.flow:29
 29:     return x !== 0 || 1 / (x: $FlowIssue) === 1 / (y: $FlowIssue);
                                   ^^^^^^^^^^ identifier `$FlowIssue`. Could not resolve name

../node_modules/findup/test/fixture/config.json:1
  1:
     ^ Unexpected end of input

../node_modules/findup/test/fixture/f/e/d/c/b/a/top.json:1
  1:
     ^ Unexpected end of input

../node_modules/findup/test/fixture/f/e/d/c/config.json:1
  1:
     ^ Unexpected end of input

Ok, my current workaround is to use nucleid and just ignore the node_module errors, seems to be pretty good.

Wouldn't a new configuration option along with changes to filter_suppressed_errors in types_js.ml be sufficient of a solution? All the files would still get scanned for dependency resolving etc, but all the errors from matching files could be silenced.
I tried making the related changes myself, but passing things around is tricky when you've never written OCaml before.

For anyone having trouble with this when using --all, there is a work around.
keep the node_modules ignored, and add a lib file:

[ignore]
<PROJECT_ROOT>/node_modules/.*
<PROJECT_ROOT>/libdefs.js
[include]
[libs]
./libdefs.js
[options]
all=true

Then in your lib file, add definitions for any required module you are getting errors with, for example I am using express so my file looks like:

declare module 'express' {  declare var exports: any;  }
declare module 'serve-favicon' {  declare var exports: any;  }
declare module 'morgan' {  declare var exports: any;  }
declare module 'cookie-parser' {  declare var exports: any;  }
declare module 'body-parser' {  declare var exports: any;  }

I tried flow. But there were simply too many error messages that had either nothing to do with my code or that I could not make any sense from (could not find any information on the matter). I will watch the project but for now it was too much diversion from everyday work.

I don't understand what's "impossible" about fixing this, at least with a temporary bandaid. It's fine with me if flow crawls node_modules, as long as it doesn't print the results from that directory. Pretty sure a special-case in this situation is justified.

Files under node_modules can contain flow types which are then used to check the usage of that module in the main project. It's not completely obvious that ignoring _all_ errors under node_modules is desirable (if I'm making use of types from a module, I'm really interested in knowing if I upgrade my version of Flow and that module's types are now somehow incompatible) and anyway that doesn't seem to be a new suggestion here.

Just to re-cap, I think the currently proposed alternatives to solve this are:

  1. Fix popular modules to not have issues Flow currently deems errors. This solution seems not workable with many popular modules: fbjs seems to add new Flow errors as often as they fix them, some modules insist on including broken json files for tests, and some are in code freeze.
  2. Ignore all errors under node_modules. This is problematic if you actually want to use the types from modules and a Flow update makes them incompatible somehow, because you could silently lose type safety that you were expecting to have. This could be opted into with a non-default flow config option, but if practically every single Flow-using projected is suggested to use this option then that's not much help.
  3. Ignore all errors under node_modules in files not directly or transitively required by files outside of node_modules. Addresses most of the issues above and possibly comes with load-speed benefits, but might be a bigger architectural change. This is what my original post in this issue specifically suggests.
  4. Ignore parse errors of json files (possibly just those under node_modules). This solution is described more in https://github.com/facebook/flow/issues/2364, and I feel it would be a good solution despite how special-cased it appears at first. (Flow only complains about unparseable .js files if they've opted in with the @flow comment. Before it did that, this was one of the main sources of Flow errors in node_modules. Now it ignores parse errors in non-Flow .js files, but it does _not_ ignore parse errors in non-Flow .json files, which seem to be the vast majority of Flow errors in node_modules now in my experience.)

Any news on this? Show stopper here as well. Was super excited, but I definitely wont be suggesting this to my team with an issue like this!

Ignore all errors under node_modules in files not directly or transitively required by files outside of node_modules. Addresses most of the issues above and possibly comes with load-speed benefits, but might be a bigger architectural change. This is what my original post in this issue specifically suggests.

any movement on this? seems like a good start

The libdefs.js workaround suggested by @RichyHBM is working pretty well and it allows for the granularity that we were looking for.

Why not solve this in the .flowconfig by [exclude] all node_modules and then [include] all modules which you need. This f.e. works with the .gitignore file.

I tried it but it doesnt work since [include] will not override [exclude].

@RichyHBM thanks for your hint! (https://github.com/facebook/flow/issues/869#issuecomment-256643823)
This is working exceptionally well!

Maybe this works, but it still a workaround, and tedious. Thank you for this, but our app is already fairly complex (reason we want flow or TS) and adding this additional trouble and boilerplate maintenance to my teams plate is not exactly appealing. Honestly its making me wonder if the whole approach (checking instead of compiling like TS) is inherently fragile. Not trying to be a debbie downer, but I feel like this deserves a real solution.

@cellvia would suppressing error output from errors found in the _node_modules_ folder work for you? That's what I really wanted. Just a simple filter of the array of errors before flow returns anything. I started down the route of adding this as seen in my comment above #issuecomment-192550474 but at the time it didn't sound like they were interested in such a solution and it wasn't likely to be merged.

I use a lot of modules from npm which contain Flow types. If there were an error with one of their types (because I'm using a newer version of Flow that had a breaking change for example), then it would be very important that I see the error instead of silently losing tons of type safety in my project where I deal with types imported from the module.

@AgentME Good point, but the tradeoff is lots of noise for lots of developers at once, it's made Flow hard to use. I am with @toddw and others on this thread that a good suppress option is the way to go. I don't feel that modules with Flow breakage present any immediate type-safety danger, they usually just surface as broken and that makes them temporarily less reliable. There's less of that lately but will always be cases that crop up.

The point is that we need a way to easily limit the noise, but still get full type-checking and an indication of which modules contain breakage. (I certainly don't like the idea of disabling type-checking, just filtering output.)

Here's a proposal for a suppress option that retains visibility:

  1. Leave [ignore] as is, continue to type-check everything else.
  2. Add a suppress block to the config, maybe [quiet]. Folks can either list specific problem modules here, or node_modules/* depending on the current noise level.
  3. Make [include] override quiet. So even if you use * in quiet you can whitelist the ones you contribute to or trust to stay current.
  4. And finally, a concise but informative status summary in the output like,

    No errors.

    34 errors hidden by [quiet]: @exponent/ex-navigation (31), other (3).

That last bit is key. Even if everyone was muting all, the right people will see the status and can whitelist the module and fix it, while others are tipped off to possible type-checking problems with a module. If a module breaks for a day or two everyone can easily mute it, keep working in an error-free state, and then restore it when it's fixed.

@samwgoldman Honestly if Flow had included this option from day 1, it would have been a lot easier for our team to adopt and work with it. We actually dropped it during its growing pains with React Native and have now come back, but it's a minus that we still have to write module-related workarounds. Happy to start a new issue thread for this if it's being considered!

I went ahead and made a new thread to extend the discussion of a suppression option:

Provide a suppression option (to mute noisy 3rd-party node_modules) #3208

I created a small helper package, that executes flow check but removes any errors reported inside node_modules from the default output.
Maybe this helps using flow in the meantime: flow-result-checker

@jbreckel I'm not thrilled that we need flow-result-checker, but I tried it out and I think it might just be the workaround that works for us. Thanks.

The problem with flow checking the whole of node_modules, in my experience, is two fold:

1) It is slow.
2) It surfaces a slew of errors, a lot of them in FB modules, that I don't care about.

The solution @RichyHBM is pretty great, but has the limitation that some of the dependencies that I import, say immutable.js, actually have good type definitions that I'd want to use.

So in lieu of a proper solution, I would like to configure flow like this:

  • ignore all of node_modules/*
  • except for the modules I know have type definitions.

This doesn't seem possible right now, because the ignore section has a higher priority than include, and the regular expressions in OCAML don't seem to support look-aheads.

We were struggling with this a lot in our app, and now have a pretty good .flowconfig setup that allows us to keep type defs from node_modules, but not be overwhelmed by errors. Rather than ignore all, then include selectively, we include all, but ignore selectively. Then we also use suppress_type=$FlowIssue to get rid of some of the rest of the noise, plus ./node_modules/fbjs/flow/lib under [libs] to make sure fbjs plays nice.

This works great overall. It means if we add a flow typed module, we will get type checking for our use of that module out of the gate. If we then see new errors that aren’t relevant, we can ignore that one module (like we had to do with radium). Once you have a nice quiet baseline, it works pretty well, but getting there was a real slog. Here’s our .flowconfig (minus module.name_mapper stuff) for reference:

[options]
module.system=haste
esproposal.class_instance_fields=enable
esproposal.class_static_fields=enable
# Ignore flow warning about not supporting decorator types fully yet
esproposal.decorators=ignore
suppress_type=$FlowIssue

[libs]
./node_modules/fbjs/flow/lib

[ignore]
.*/node_modules/radium/src/.*

@miracle2k Includes should override excludes if I recall correctly. So just add libs with defs to [include].

One way to avoid the Required module not found errors would be to run flow-typed install which will fetch existing libdefs for popular libraries from the flow-typed repository. It also generates stubs for the libraries that cannot be found in it. This works great for many projects but in some rare cases, the stubs for certain libraries like immutable were not generated because they already exist in node_modules/immutable (but we're ignoring it!)

My Solution

I eventually came up with a cli tool flow-scripts to automatically generate a lightweight libdef stub interfaces such that I could flowignore node_modules and not get the Required module not found errors.

Simply run

$ flow-scripts stub

in the root of the repository and the libdef stubs will be automatically generated in the flow-typed/ directory.

@vjpr The documentation states that the opposite is true: https://flowtype.org/docs/advanced-configuration.html#ignore

@miracle2k Oops, sorry for the mixup.

Any updates on this?

I went with the following approach for now in a new project:

[ignore]
# Performance hack: Ignore all node_modules except those starting with:
# - "a" (for axios)
# - "r" (for react)
# This needs to be tweaked as more dependencies are added.
# https://github.com/facebook/flow/issues/869
<PROJECT_ROOT>/node_modules/[^ar].*

I'll have to see how that scales going forward...

(This technique taken to the next level: https://gist.github.com/lydell/ee05b5acbc1f51eea649926af956dc98)

if in case anyone wants to use it online https://dsslimshaddy.github.io/flowignore/

I don't know in which world [ignore] is just for configuration aesthetics. Had such high hopes. 😫

@goncaloneves it should be in your root directory, named .flowconfig

It would be nice if flow used PCRE on the ignore regular expressions, this way we could work around it by using a negative lookahead.

To ignore all node_modules but immutable, .flowconfig would look like this:

[ignore]
.*\/node_modules\/(?!immutable\/).*

This might break the current regexes though...

This absolutely makes flow unusable. Ignoring is even worse. Adding tech debt like crazy to work around this makes the benefit of flow turn into a negative in the first place. All the sudden everyone needs to be handling all these little edge cases on every project.

Flow.js is, in my mind, not ready for public consumption / a beta product until it can produce results that don't require hundreds of extra steps. Those steps simply add more user-error and problem possibilities.

Flow.js is, in my mind, not ready for public consumption / a beta product

I tend to agree with this, because the number of times that Flow gets in the way of me getting stuff done is just too high. A developer should never have to fight with their tools. I've almost removed it from my project twice now, but then it redeems itself by saving me from doing something dumb like referencing a property on a potentially undefined value.

By contrast, Jest is perfection. It works great, it makes my life _easier_, and saves me time. Love it. I think Flow can get to that point too, but it's not there yet.

@iegik Come on, dude. What's the point of that, really? Feedback and criticism is fine, if the developers can take it and use it to improve the product. But a web page dedicated to airing complaints about Flow just feels kinda pitchforks-and-torches.

@ffxsam just-for-fun. I believe, that if developer write his code much more careful - he might not need Flow.

P.S. Content is needed. PR, please. :)

Proposal which might solve this issue: https://github.com/facebook/flow/issues/4298

@iegik I have PR for you, please review https://github.com/iegik/youmightnotneedflow/pull/1/commits

I'm testing Flow for the first time. This particular issue makes me hard to recommend the tool to my colleagues. The tool should just be plugged into your project and it should need to be configured for your own code only, and avoid taking your time to configure it for code that it's not authored by yourself.

I've tried @RichyHBM https://github.com/facebook/flow/issues/869#issuecomment-256643823 it just works good. But IMHO we shouldn't be doing this at all.

@jcready That's still just a workaround. You don't really want to list again all your dependencies.

FWIW, this is especially painful when your flow version doesn't match what a lib in node_modules is using for their project. Their types may depend on updated typedefs that ship with flow, causing errors for you but showing clean for them.

Similarly, when there are suppressed errors using suppress_comment in a lib in node_modules and it doesn't match YOUR suppress_comment values you will see errors where they will not.

Why not just add error suppression mechanism after all? :)

Every classy C-like compiler has such or similar feature! It's totally OK to ignore errors in third party files. And in this case we just need the ability to specify them explicitly and we don't need to automatically distinguish internal vs third party.

Another real-world example:

I updated to the latest version of flow, which fixed a flow bug. One of my dependencies has a $FlowFixMe to workaround the bug. I now get an Error suppressing comment. Unused suppression error because the bug has been fixed and the suppression is not necessary with my flow, but the dependency (https://github.com/callemall/material-ui) doesn't get an error with their flow version and can't remove that suppression line if they want to support older versions of flow that other users may have installed.

Another real-world example:

  1. Started with no flow errors or warnings.
  2. I updated material-ui to get new library features.
  3. The updated material-ui was using flow >= 0.53.0, so its React types didn't match the ones my version of flow (0.51.0) understood. I now had a bunch of type errors due to React type mismatches.
  4. I upgraded my version of flow to >= 0.53.0. The React errors went away as expected, but NEW errors in a subdep of some library I am using popped up due to changed flow behavior (example: https://github.com/cssinjs/jss/pull/580)

So now I have to either fork the whole dep chain to fix the issue, wait for the various libraries to fix, release, and update all their deps, or use yarn's overrides.


This is extremely bad and feels very kafkaesque. It essentially means that upgrading flow or any library could suddenly introduce errors without an easy way for me to fix them. It really makes working with typed dependencies painful and will only get worse the more libraries are converted to flow.


Suggestion

Have a way to suppress a specific error from _outside_ the file. Something like:

[suppress]
node_modules/jss/lib/renderers/DomRenderer.js.flow:128
node_modules/some/other/lib/Foo.js:42

This would essentially be inserting a virtual $FlowIgnore comment before the line. I would imagine the changes to support this would be fairly easy--code is probably around here: https://github.com/facebook/flow/blob/26dbdcbd35c82270f8b4b34e97844099dfb24769/src/typing/type_inference_js.ml#L48

This bug makes flow nearly unusable, will there be any updates on that?
Add: Maybe just give a option to suppress all warning and error from a folder is enough

@calebmer Are you aware of the aforementioned effects that this issue causes? This seems to be more critical than the “DX” label implies.

_Just an aside, what does "DX" even mean?_

It means "Developer eXperience", like UX for Devs

I put up a patch that I think should address this. It adds a [silence] section. Paths in [silence] are still typechecked (unlike [ignore]) but any problems found are not reported. Let me know if that would not be sufficient to fix this issue for you.

As an aside, this was my first time programming in OCaml...if I can do it, you can too! 🎉

@KingHenne To call this a simple experience issue is more than an understatement. Every team in our company has dropped using Flow because this issue has gone unsolved. The number of replies on this issue alone should be enough of a sign. In it's current state, Flow is simply not usable in any form of serious development.

@Etheryte are you willing to share more about the specific situation at your company? (Even if only privately, rather than in this thread. Email me if that would work better.)

It sounds like this is causing very painful problems. I can certainly empathize that getting tons of unrelated flow errors is frustrating. At the same time, I’ve worked in or have personal knowledge of many large codebases that use flow successfully. So I bet that if you’re willing to spend time on it we can find a way to solve your problem via configuration or other changes.

I've recently started to ignore the _entire_ node_modules/ directory, and instead use flow-typed install to install type definitions for all dependencies, or make stubs if no type definitions can be found. It works pretty well. One downside is that you lose type definitions for some dependencies that ship their Flow types in the npm packages, but are not available on flow-typed. Oh well. I guess I should start contributing more to flow-typed 😉

Just removed flow because of this. The number of issues you face during setup and configuring is just insane.

I added 1 package (react-apollo) and had to add 4 packages to ignore just because flow typings are broken in this package and its dependencies. 🤦‍♂️

I'm (was?) trying to sell Flow to my team and we get to this issue, even with a very small project we get thousand of errors coming from some of our dependencies (node_modules/), this is making almost impossible for me to justify the tool.

I really like the idea behind Flow, but for the moment is getting more on our ways than helping us.

@jlpiedrahita why not add

[ignore]
<PROJECT_ROOT>/node_modules/.*

To your .flowconfig?

@apsavin Doesn't that also prevent Flow from using the module code to check for errors in how _my own code_ uses it? Meaning I think ignoring everything goes too far. Just don't show errors that originate purely within the external modules.

@apsavin @lll000111 And because then another kind of errors arise: Required module not found, Required module not found, Required module not found.

@lll000111 @jlpiedrahita

Doesn't that also prevent Flow from using the module code to check for errors in how my own code uses it?

And because then another kind of errors arise: Required module not found, Required module not found, Required module not found.

No, if you will use flow-typed

@apsavin That would only work if flow-typed also had all the library definitions of libraries that ship flow types themselves.

@apsavin That breaks WebStorm's jump-to-definition. The editor will jump to flow-typed's generated stubs instead of the actual source code.

@dsilvasc Isn’t that a bug in WebStorm? It should allow you to jump to either?

@KingHenne that's true, but it's more convinient to include types in your repo anyway. And I think authors of libs should prefer flow-typed over shipping types themselves - because it comes with tests for types at least.

@dsilvasc Thats's true as well, but how often do you need to go to the code of vendor libs? It's much more often when you want to know what interface is. And it's exactly what you'll see in flow-typed dir.

@lydell I think WebStorm uses flow to decide where a symbol comes from. Flow tells it the symbol comes from the flow-typed definition, if one is present.

@dsilvasc

I think WebStorm uses flow to decide where a symbol comes from

WebStorm has an option for that.
2017-10-12 13 13 53

@apsavin There's no interface for generated stubs though. It's just a file full of anys. The source code is a more useful form of documentation when there isn't a definition in the flow-typed repo. You can, for example, look at the PropTypes for react components.

@KingHenne that's true, but it's more convinient to include types in your repo anyway. And I think authors of libs should prefer flow-typed over shipping types themselves - because it comes with tests for types at least.

That is an idealistic view and not very pragmatic. For example, I don't think Facebook will move their flow types for react-native to flow-typed. And maintaining them in two places makes no sense either.

@dsilvasc Thats's true as well, but how often do you need to go to the code of vendor libs? It's much more often when you want to know what interface is. And it's exactly what you'll see in flow-typed dir.

I agree. It's the same behaviour in vscode for TypeScript definition files.

@apsavin That's a good point. Maybe they could also have a feature to jump to source when the flow-typed definition is a generated stub (should be possible to detect). I'll send a feature request and link it here.

@apsavin There's no interface for generated stubs though. It's just a file full of anys. The source code is a more useful form of documentation when there isn't a definition in the flow-typed repo. You can, for example, look at the PropTypes for react components.

If you create those any stubs why even bother using flow in the first place?

@dsilvasc You should replace any with something sensible if you want to get help from flow anyway.

If you create those any stubs why even bother using flow in the first place?

@KingHenne You still benefit from everything else that is type-checked.

@KingHenne You still benefit from everything else that is type-checked.

That's not good enough for me.

@KingHenne @apsavin I don't mean types for my own code. Say for example that you use react-toggle. There are no types for it in the flow-typed repo, so flow-typed install generates a stub.

Those stubs aren't very useful, so currently I use flow-typed install --skip to avoid those stubs, and Flow looks at node_modules instead (inferring anys for untyped modules). The suggestion here is to stop using --skip so we can exclude all of node_modules, which brings back those generated stubs.

@apsavin flow-typed can't solve the issue when all packages are ignored, because not all npm packages are defined there.

@KingHenne

That is an idealistic view and not very pragmatic. For example, I don't think Facebook will move their flow types for react-native to flow-typed. And maintaining them in two places makes no sense either.

Partly agree, but:

https://github.com/flowtype/flow-typed/tree/master/definitions/npm/prop-types_v15.x.x
By Facebook as far as I can see

@apsavin Seems like this has been added to flow-typed because prop-types does not use Flow.

Sorry guys, but after some reflection I'm starting to remove all flow code from my project, I don't want to spend my time trying to find ways to hack things or being frustrated, I want to spend my time being productive.

When you're constantly fighting your tools, they are not tools any more.

In spite of its flaws, I still believe flow is worth using. Also, please note that flow 0.53 was released just this past August and many projects are still working on updating to satisfy the major changes brought by the release. If errors from third-party modules got you down, why not try out jbreckel's flow-result-checker or my flow-error-suppressor until silence support is merged?

I've been following this thread for a while, and while I share some of the disappointment/confusion here, particularly with libraries that use Flow and ship with hundreds of errors and warnings, I think we've been missing a point that incomplete types are indeed a feature, and not a bug. That feature has brought many of us to Flow after all.

Yes, flow-types is way behind compared to DefinitelyTyped, there is plenty of space for improvement, and yes, many of us approach Flow in some sort of a lazy way that results in warnings that we tend to ignore but isn't that freedom the beauty of it?

If we can't live with incomplete typings coming from third party modules, we can simply ignore them:

[ignore]
; Ignore libraries with broken flow
; See also corresponding lines in [options] below
.*/node_modules/react-native-fbsdk/.*
.*/node_modules/react-native-linear-gradient/.*
.*/node_modules/react-native-side-menu/.*

Then, to avoid any "Required module not found" errors we can add a few corresponding lines:

[options]
; Ignore libraries with broken flow
; See also corresponding lines in [ignore] above
module.name_mapper='^react-native-fbsdk$' -> 'emptyObject'
module.name_mapper='^react-native-linear-gradient$' -> 'emptyObject'
module.name_mapper='^react-native-side-menu$' -> 'emptyObject'

That's it.

There's also nothing that stops us from sending PRs to fix the problematic modules. Look at @Ashoat's PR to fix Flow errors in react-navigation. He didn't complain like many us like to do. Instead, he did all the hard work himself. Twice, in fact!

I might be somewhat naive but if that's not great then I don't know what is. Especially that there's always TypeScript as an alternative that seems to be much more suited for the hardcore purists that some of us seem to be.

Not pointing fingers, just sharing some love.

We have a library we use (validated) that provides its own types as part of its distribution. We've chosen to add packages in node_modules that Flow has an issue with. It's possible we'll have to add more as we add more packages, but I expect that to be slow and also obvious what to do when we see it.

Our project requires 100% type coverage, and while it's been a challenge, it's really paid off. We've had to write our own type defs for 3rd party libs and tweak existing ones from flow-typed. Unless you're talking about a complicated utility library like Immutable.js, it's really not that hard to add these libdefs.

I'm watching this thread because I want to know if I can take out those manual ignores at some point, or otherwise use a better means of avoiding the errors from node_modules. You're using an open source tool here. If it's not meeting your standards, you're a pull request away from fixing it. PRs get merged pretty quickly for flow-typed, and there's not even a requirement that the types be 100% complete.

Look forward to updated behavior for node_modules checking in Flow 0.57 🎉

In Flow 0.57 we will _only_ check files in node_modules if you have imported them in your flow-typed or product code 😊

Once Flow 0.57 is released, let us know if you have any issues 👍

https://github.com/facebook/flow/blob/v0.57/Changelog.md

Send your love to Gabe. (@evilgabe on Twitter.)

@calebmer I guess it will still give errors for direct imported packages if the types are wrong there.

@calebmer sounds great!
However, how to check? There's an issue: https://github.com/facebook/flow/issues/5085

While the 0.57 update is nice, and a step in the right direction, it still doesn't really resolve the main problem - the need to suppress third party code errors. If a library I'm using has flow errors but works fine, I don't care, I want to use flow only for the code we write.
The solution would be to get LegNeato's pull request merged asap. In it's current form, flow is simply not reasonably usable.

@Etheryte
Your point is definitely valid, the number of comments on this issue definitely shows it's a pain point.

If a library I'm using has flow errors but works fine, I don't care, I want to use flow only for the code we write.

About this, I think it's reasonable for flow to point out issues in interface definitions of files you are importing. It would be counter-intuitive for it to just ignore those errors and try to work out with what it has already inferred (which likely will be wrong).

FWIW here is a workaround we're using:

[ignore]
.*/node_modules/foo.*

[options]
module.name_mapper='^foo$' -> 'any'

(note that you will need a module any in your flow-typed with var exports: any)

Is there a way to combine flow libraries as such?
Something like:
for all the code in the app, it would check node_modules for flow definition. if none exists, check flow_typed for flow definition.
for all the code outside of the app, it would ignore/silence all errors (eg from node_modules)?

why would one want to list one-by-one all the modules that needs to be silenced?
although technically there are only like 20 so it wouldn't be too bad to add them in any case.

Flow 0.57 seems to work better with node_modules, but seems to break with relative node_modules imports. For example, in my flowconfig:

module.system.node.resolve_dirname=node_modules
module.system.node.resolve_dirname=./src

But an example of import x from "package-name/directory/file" will result in Required module not found

@AgentME Should this issue be closed (since the core problem has been fixed in https://github.com/facebook/flow/blob/v0.57/Changelog.md, implementing something close to your 3rd proposal from https://github.com/facebook/flow/issues/869#issuecomment-257201063)?

This is gathering a number of separate issues / proposals which maintainers will have a hard time finding.

@calebmer Flow 0.57's behavior is an improvement, but it seems like the change doesn't apply to .json files. Syntax errors in unimported .json files under node_modules are still treated as errors. Broken json files (usually included in dependencies as test files) are the primary issue I run into with Flow and dependencies. (I've got some examples of popular modules that trigger this in #2364.)

chris@canyon:/tmp/foo$ tree
.
├── foo.js
└── node_modules
    └── bad
        └── bad.json

2 directories, 2 files
chris@canyon:/tmp/foo$ cat foo.js 
/* @flow */

const x: number = 5;
chris@canyon:/tmp/foo$ cat node_modules/bad/bad.json 
{"
chris@canyon:/tmp/foo$ flow version
Flow, a static type checker for JavaScript, version 0.59.0
chris@canyon:/tmp/foo$ flow
Error: node_modules/bad/bad.json:1
  1: {"
       ^ Unexpected token ILLEGAL


Found 1 error

🎉

A year of watching this issue finally brings me to the point where I can finally try flow in my projects again. Amazing work guys, thank you for that.

Would be great to see a note about that in the official documentation though :)

This doesn’t really fix it, though, the way @LegNeato’s [silence] section did, does it? E.g., with [silence], Flow would check if my code comports with the types in imported modules (e.g., passing arrays, not strings, to lodash array functions), while suppressing any errors arising strictly within the libraries. With the accepted PR, it suppresses those errors, but also fails to check if my code is calling correctly into libraries.

I have found out how to sort-of fix this (plus https://github.com/facebook/flow/issues/4200 )

I have src in separate folder. I have .flowconfig there

I create a new directory there fake_flow_node_modules and there I symlink only the modules I want flow to load

And then I set module.system.node.resolve_dirname=fake_flow_node_modules

That way, flow actually loads the modules, but it doesn't load everything under the sun (so it's quicker and checks only what I want) and I can even update the modules, since the symlinks lead to the correct node_modules folders

so I have to do

cd src
mkdir fake_flow_node_modules
cd fake_flow_node_modules
ln -s ../../node_modules/some-module ./some-module

And then it works (with the resolve_dirname set)

note that you then have to ignore this new folder in your build system, otherwise it gets compiled

Wouldn't it make sense for the all option to allow for a way of defining it's own scope instead? Example:

[options]
all=true
all_include=<PROJECT_ROOT>/src/.*
all_ignore=<PROJECT_ROOT>/node_modules/.*

My hack stops working, when the dependent module is also a flow module, and it requires something (which then isn't correctly found in those fake modules). >:(

Any progress on that? Still have error issue with direct imports or "module not found" if i hack it

Sorry, but I'm really disappointed by this library. I spend a lot of time just to find, that I can't fix this ridiculous issue. Why should I care about type definition in any 3d party library? That is insane. This library should check only my code, nothing more

@optimatex

Why should I care about type definition in any 3d party library?

If you don't care, you can ignore it.

This library should check only my code, nothing more

This library should check the correctness of an application. So flow should check if you're using third-party libs correctly.

@apsavin Errors within 3rd party stuff are NOT of interest.

Please stop repeating your useless suggestion — you've already been downvoted previously above), we already had that OT discussion!

And your suggestion to "ignore" is way off — this prevents any checks from occurring!

Apparently you completely miss what this complaint is about. Of course people want to know about errors — in THEIR projects. That obviously includes errors in how one uses 3rd party code. However, errors within that 3rd party code are of no use. Especially when the report is about code you don't even use from that library.

@lll000111 sorry, the discussion is really long and I was not attentive enough.

I would like to add to "ignore" but, as mentioned before, it results to numerous other errors like "module not found"

How is Flow supposed to enforce that you are consuming the types of the lib properly if those libs themselves can't pass a type check?

FWIW I'm consuming multiple packages that have their own internal type definitions. Saying that those type defs then need to be exported to flow-typed is an increased non-trivial burden on maintainers. Things get even more complicated when you consider type defs that come from private packages.

When you use ignore for a particular library you're going to have to provide stubs for the packages you're consuming. This is something flow-typed can help (see flow-typed create-stub).

@LoganBarnett Please read the already existing discussion, it answers your question. All you have to read is the headline, for starters, and for the other issues there's already been plenty of explaining above. If you don't understand an issue other people report, why join the discussion at all anyway?

@lll000111, I don't think your response is appropriate, nor does it add to the discussion. you've already been warned. please abide by our Code of Conduct. thanks!

@lll000111 I've read it several times, and I've been watching the thread for months. This is really hard to track, and there's loads of vitriol in this thread.

I understand the ask of this thread will effect the way I consume libraries if everyone here gets what they want. So yeah, I'm weighing in on it.

If I'm clearly so confused (and apparently not the only one), maybe it's time to start a new thread with a clearly stated problem and proposed solution. Whatever it is you're trying to communicate has clearly been lost. Existing suggestions to work around the problem are met with what I can only take away from as "I don't like it". Were I a Flow maintainer, I wouldn't be giving any attention to this thread. This needs to be steered in a much more constructive direction if you hope to see improvements here.

@LoganBarnett As I said, if you don't see the issue, what's the point? The last responses all were of the same non-contributing kind. It's so simple: If this is not your issue, feel free to ignore other people who have one. Why do you feel you nee dto understand everybody and every issue anybody has?

One, Flow does not need to type-check the entire library, only what we actually use.

Two, Flow does not have to check the entire library, only the interface we use - errors that are within that library are something we cannot do anything about, nor does it matter, as long as we use the interface correctly. Well it may or may not point to a bug within that lib, but as long as we call it correctly _our_ code is doing what it is supposed to.

@LoganBarnett The original issue post has over 180 upvotes so clearly this is a pain point for a large number of people. In my opinion, all of your comments have already been addressed in the (albeit lengthy) discussion over the months, but you're free to disagree here. If you don't see the merit in this suggestion, that's completely fine — but please don't repeat points that have already been discussed, you're simply sending notifications to the 75 people who are all watching this thread.

@LoganBarnett

How is Flow supposed to enforce that you are consuming the types of the lib properly if those libs themselves can't pass a type check?

I can think of various ways:

  1. If the libraries have Flow annotations (or flow-typed specs), it can check if we’re matching the annotations rather than the internal code. If there’s a mismatch between 3rd party annotations and 3rd party internals, that’s an upstream bug that should break upstream builds—not mine.
  2. If we’re not using the failing parts of the library code, the failures are irrelevant to us and we don’t want to hear about them.
  3. There are certainly going to be cases like you suggest, where 3rd party errors makes it impossible to infer correctness. In this case, I’d love for Flow to say “There are potential issues in 3rd party code, re-run with -3rdparty for details” (or whatever), but not flag my build as a failure/exit with a non-zero status. Treat this as a warning, not an error.

It’s also worth keeping in mind that Flow can be rather anal-retentive about purely hypothetical issues, such as warning about “potentially undefined” references that cannot possibly be undefined even if the Flow parser isn’t able to infer that it’s impossible.¹ A library could be correct in absolutely every possible case, and yet fail a Flow check because it doesn’t do things the Flow way (which sometimes involves extra assertions of facts that I know but Flow doesn’t). By all means have Flow mention it, but don’t treat this the same way as an error in the code I’m working on.

I understand the ask of this thread will effect the way I consume libraries if everyone here gets what they want. So yeah, I'm weighing in on it.

Go back and look at @LegNeato’s PR, which proposes to add a [silence] section to .flowconfig. I can add all my node_modules to the [silence] section and get what I want without affecting you in the slightest. No one is forcing you to add a [silence] section.

More generally, giving us the ability to ignore reported issues we don’t care about does not change the way you consume libraries. You’re perfectly welcome to treat every single issue in every single dependency as an error, if that’s what you want.


¹ The thing I keep seeing: A non-const reference is checked for nullness and subsequently treated as non-null; but if it’s referenced from a callback, Flow assumes it may have changed, even if there are no assignments to it. I can create a const alias in my code to shut Flow up, but I can’t expect every 3rd party maintainer to do this.

This change was a terrible idea, breaking changes when alternatives exists is never a good idea.
[ignore] already existed and many people use it for broken code.

The point of typing is to increase productivity, if I am going to keep fighting and fixing flow, I am not sure if the pay off is worth it anymore.

@Etheryte

@LoganBarnett The original issue post has over 180 upvotes so clearly this is a pain point for a large number of people.

If anything, those up votes means that they agree that such problem exist, it says nothing about the breaking change solution that was pushed to address it, if you want to measure how that is accepted, search around the many many issues and upvotes against it.

@omeid It seems you've misunderstood the central point of the suggestion most of the parties here find reasonable — adding a configuration option for [silence]. It is not a breaking change and most of your comments really don't make sense in that regard.

@Etheryte I think you missed the solution that got pushed to fix this and broke many projects setups. My comments are in regards to changes in Flow 0.57:

we will only check files in node_modules if you have imported them in your flow-typed or product code.

This is meant to address the original issue raised here, but it broke how module.system.node.resolve_dirname worked.

Your suggestions and what you and others find reasonable means nothing to the current state of flow and constant churn.

See #5180, #1068, #5105 and many more.

@omeid Oh, sorry, it seems I misunderstood what you were addressing in that case.
However, it is ironically amusing that there is a PR for a solution that is not breaking and makes both sides of this argument happy, but what the Flow team actually implemented makes neither side of this happy and doesn't solve the actual problem.

_One side note, please group your messages together into a single one if you have multiple points to make, otherwise you're spamming everyone's inbox._

@omeid I believe the "breaking change" you're referencing was unfortunately the result of poor documentation and developers (like myself) using that field for something it was not designed for.

From what I can gather, module.system.node.resolve_dirname was always to dictate where the node_modules folder was—that is, where should Flow look for modules downloaded via npm.

Unfortunately, the usage of this property (like in the issues you linked to) turned into use cases like "I want to import from my src folder using an absolute path instead of a relative one." This use case should have been solved with:

module.name_mapper='^src\/\(.*\)$' -> '<PROJECT_ROOT>/src/\1'

From Flow's perspective, it's hard to label this as a "breaking change," since developers weren't using the property properly in the first place. But from a developer perspective, I also understand that the intended use of the field was not what it widely became.

@jjshammas

From what I can gather, module.system.node.resolve_dirname was always to dictate where the node_modules folder was—that is, where should Flow look for modules downloaded via npm.

The distinction of node_modules from the rest of code in the context of type-checking is somewhat arbitrary, from where I stand.

The official documentation says:

[options]
module.system.node.resolve_dirname=node_modules
module.system.node.resolve_dirname=custom_node_modules

Then Flow will look in directories named node_modules or custom_node_modules.
Note: you can specify module.system.node.resolve_dirname multiple times.

Considering that,

From Flow's perspective, it's hard to label this as a "breaking change," since developers weren't using the property properly in the first place.

If Flow decided to treat _node_modules_ differently, it is breaking changes, the use case is irrelevant. Besides, no one has made a sound argument as to why the type-safety of your dependencies isn't important to your application.

I think it is absurd to treat code type-safety differently based on who has written it.

For those who missed it, the [untyped] config option by @mroch addresses the most generic use-case well as far as I've tested it at the moment. It also allows you to silence only the libraries that don't play well with Flow.

[untyped]
.*/node_modules/offending_library

I don't know the earliest Flow version that shipped it, but it's live and working well with 0.71.

The problem with [untyped] is that it doesn't preserve types and simply treats the whole library as any.

haggholm puts it nicely:
"What I (and I think many others) really want is for Flow to verify our use of 3rd party APIs, but not to make us responsible for 3rd party internals. [untyped] and [ignore] fails to do the former. Doing nothing does the latter. [silence] hits the sweet spot, insofar as it is possible."

However, we don't have many other options right now.

The only reason I'm able to use flow in my project is because of @Palisand flow-result-checker package. This package doesn't work with recent version of flow due to changes in the error reporting format. I would be very nice if this option to suppress errors from node_modules would be available out of the box.

Does this issue has a resolution? Like wontfix or assigned?

I've been trying to use both typescript and flow in the past few days, and there are these dumb issues that come up here and there, that are NOT being addressed, showing the impossibility of corporate backed language features becoming ever production ready cause maintainer X and Y have no clue how their package is being used in production

Just give up polluting the ecosystem with your so called open source projects which are useful in production only for your own internal projects

@Diokuz, currently only @flow'd files are checked. Files can be ignored with [ignore]. If you want to still include node_modules in the type check but not see it's errors, you'll be able to use [declarations] once https://github.com/facebook/flow/pull/4916 lands. I'll attempt the merge tomorrow.

@mrkev Thanks for the followup. FYI, I have unexpected token , error in 'bower.json' of bcrypt, which is not a @flow'd file as far as I am concerned, and I do not even require it in a js file (it is required in a typescript file); nonetheless, I do not understand why this issue is closed if it is being addressed, even if partially, without at least proper label?

@HunderlineK Idk man, I just hopped into this issue14 hours ago ¯\_(ツ)_/¯. I'm sure somewhere in the thread there's discussion about why it was closed.

You can [ignore] that broken json file btw.

For those still following this issue, in the next release of Flow I added a new [declarations] section. Here are the docs about it:

_Often third-party libraries have broken type definitions or have type
definitions only compatible with a certain version of Flow. In those cases it
may be useful to use type information from the third-party libraries without
typechecking their contents._

_The [declarations] section in a .flowconfig file tells Flow to parse files
matching the specified regular expressions in _declaration mode_. In declaration
mode the code is not typechecked. However, the signatures of functions, classes,
etc are extracted and used by the typechecker when checking other code._

_Conceptually one can think of declaration mode as if Flow still typechecks the
files but acts as if there is a comment that matches
suppress_comment on every line._

I believe by leveraging [untyped], [ignore], and the new [declarations] section all problems mentioned in this issue can be resolved (once the release happens of course). 🎉

all problems mentioned in this issue can be resolved

@LegNeato except the fact that flow will still walk through not relevant files (even just to check if marked with// @flow) and for big repos with multiple packages this still a problem. Ignoring every single dependency and dependencies of dependencies is not an option. The question is still not answered - why to parse not imported/ not relevant files?

https://github.com/facebook/flow/issues/869#issuecomment-399817797

currently only @flow'd files are checked

This isn't quite true: .json files are always checked for syntax correctness, which is annoying because many dependencies intentionally publish tests that contain broken .json files.

why to parse not imported/ not relevant files?

@levenleven say you have 3 JS files: A, B and C. A has an import for B, and B has one for C.

  • If you open B, it's easy to see that it depends on C and you should check that file-- information about A is not relevant.
  • If you edit however B, you don't really care about C, but you do care about re-checking A, since whatever it's importing from B might have changed.

The only way to know what files should be re-checked is if you form a full dependency graph at the beginning of the type-checking process. Flow goes the safe route in including all files in this graph so you could change any file and all files that depend on it would be automatically re-checked.

Perhaps there's optimizations that could be made, sure, but I'd say that's a separate issue.

https://github.com/facebook/flow/issues/869#issuecomment-402366948

.json files are always checked for syntax correctness, which is annoying because many dependencies intentionally publish tests that contain broken .json files.

@AgentME so ignore all test json and/or treat json as declarations:

[ignore]
**/(__)?tests?(__)?/*.json

[declarations]
 **/*.json

https://github.com/facebook/flow/issues/869#issuecomment-402366189

except the fact that flow will still walk through not relevant files (even just to check if marked with// @flow) and for big repos with multiple packages this still a problem. Ignoring every single dependency and dependencies of dependencies is not an option

@levenleven I assure you Flow has been used on bigger codebases than yours in many large monorepos. Also, the things to check vs not check are usually _project dependent_...Flow's default behavior + config option customization is pretty much the only good method to make sure no files are missed in the general case as @mrkev demonstrates. FWIW you might carve huge chunks off for your use-case by doing creative things in your config like:

[ignore]
**
[include]
**/*.js(on)?

So with the next Flow version we have:

  • [ignore] - Have flow not parse
  • [untyped] - Have flow parse but throw away types
  • [declarations] - Have flow parse and use types but throw away errors

There are no known use-cases that can't be fixed with a combination of these.

The Flow defaults may not work out of the box for your project but that doesn't mean there is an issue here. Before [declarations] some fixes for use-cases were _impossible_. Please open other tasks with specific default behavior changes.

@LegNeato Nice suggestion, but once you [ignore] ** nothing can be included. Seems that ignore beats the include. Limited regex options (no way to ignore all but 'foo') makes "ignoring" really annoying.

[declarations], although it's now a supported config section, doesn't actually work.

I'm running flow 0.76.0. 16 files in node modules don't pass the flow check. One of these files is /Users/kylebebak/Code/Work/919/web/node_modules/npm/node_modules/config-chain/test/broken.json.

None of the following patterns silences flow errors in this file (or any of the other 15 files, for that matter):

  • /Users/kylebebak/Code/Work/919/web/node_modules/npm/node_modules/config-chain/test/broken\.json
  • <PROJECT_ROOT>/node_modules/.*
  • .*/node_modules/.*

So either the docs are wrong, or the feature was shipped but it's broken.

EDIT: So, as @LegNeato mentioned in #4916 , this isn't a problem with flow. I didn't understand the difference between ignore and declarations. Thank you!

I'll take a look and see what is up...in my testing it worked and it landed with tests. Maybe something is wonky. Thanks for trying it!

Thanks @LetNeato, I'm really looking forward to using this!

exactly as @levenleven noticed. [ignore] section has the highest precedence. Would be great if every sections [include], [ignore], [libs], [declarations] has also option for exclusive rule like .gitignore ! prefix.

I think there is more problems with Cannot resolve module error) https://github.com/facebook/flow/issues/5183

Most of my errors are gone by adding following to .flowconfig

[untyped]
.*/node_modules/**/.*

Use [declarations] as mentioned in https://github.com/facebook/flow/issues/869#issuecomment-402337645 and the document

[declarations]
<PROJECT_ROOT>/node_modules/.*

[untyped] treats extracts modules as any, instead of their actual types. This seems to negate the purpose of type checking in the first place.

Why is this issue closed? The problem has not been answered and has not been marked as won't fix.
Even though there is a new section to ignore errors from dependencies, flow is checking a lot of files which are not in the flow-typed, so it is unnecessary and takes some time.

Can we be given a simple option to just ignore all cannot-resolve-module errors? Like @joan-saum said, by using the new [untyped] and [declarations] sections it still causes potentially thousands of files to be checked unless you know precisely which .js files are needed, which is not always obvious.

cannot-resolve-module is not an especially useful check anyway since webpack will tell me that. So being able to just turn off those error messages completely is good enough work around for most.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bennoleslie picture bennoleslie  Âˇ  3Comments

marcelbeumer picture marcelbeumer  Âˇ  3Comments

damncabbage picture damncabbage  Âˇ  3Comments

ghost picture ghost  Âˇ  3Comments

philikon picture philikon  Âˇ  3Comments