Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not checking node_modules #1548

Closed
oreqizer opened this issue Mar 18, 2016 · 31 comments
Closed

Not checking node_modules #1548

oreqizer opened this issue Mar 18, 2016 · 31 comments

Comments

@oreqizer
Copy link

My .flowconfig file looks like this:

[ignore]
.*node_modules.*
.*data.*
.*dist.*
.*\.tmp.*

I want to ignore everything that's located in node_modules, because it throws like 20 different errors when I turn it on.
Problem is, when I let's say import _ from "lodash", Flow shouts at me that ^^^^^^^ lodash. Required module not found.

How do I make Flow to not check node_modules but recognize it's modules?

@oreqizer
Copy link
Author

screen shot 2016-03-18 at 16 40 07
the files checked aren't even // @flow annotated.

some have dependency problems, or aren't annotated:
screen shot 2016-03-18 at 16 41 39

ignoring them one by one would be a lot of unnecessary work - is it possible to just not check node_modules, but resolve imports from it?

@samwgoldman
Copy link
Member

is it possible to just not check node_modules, but resolve imports from it?

No, it is not possible. We are working to improve the current situation with these errors, but for now you have two options: 1) use more granular ignores or 2) ignore all of node modules and declare your dependencies.

For (1), something like this might work (I haven't verified the regex syntax, but something along these lines):

[ignore]
.*/node_modules/.*/test/.*
.*/node_modules/fbjs/.*
.*/node_modules/react-motion/.*

The fbjs errors are resolved in the latest alpha, 0.8.0-alpha.3, although that doesn't help you now. This is the first report I've seen for react-motion, so thanks for the heads up.

For (2), you can continue to ignore node_modules, but you need to tell Flow about the existence of your modules with libdefs. For example:

[libs]
flow/
# flow/lodash.js
declare module lodash {
  declare var exports: any;
}

You could of also be more specific about the types. Hope this helps.

@makarov-roman
Copy link

@samwgoldman I have the same error. I tried to use immutable.js (which have .js.flow with declarations) with flow. But it is inside node_modules directory, which is ignored.
[ignore] .*/node_modules/.*

[libs] ./node_modules/immutable/dist/immutable.js.flow
Is it reason why flow can't find module? If it is, how to use flow with any packet manager?
I have also tried to move js.flow from node_modules. It's look like:
[libs] ./flow/interfaces/
But nothing is happened. Did I miss something important?

@despairblue
Copy link

I seem to have the same issue: #1816

@arasmussen
Copy link

Zaven just posted on Product Pains about this: https://productpains.com/post/flow/support-including-files-without-type-checking-them

@sheerun
Copy link

sheerun commented Aug 1, 2016

Bit me as well. Ideal behavior of flow would be just as all files in project's directory had // @flow header (except ones in node_modules). Currently I need to opt-out of check --all and prepend // @flow too all files in the project to achieve desired behavior.

@StreetStrider
Copy link

Crazy to see this issue still opened. I've used third-party type definitions successfully for a while, but today I'd depended on package with actual types built in already. So I'd faced this issue too. I believe Flow must to not typecheck node_modules/ as code under it is in packages' vendors responsibility. The most crucial thing in current behavior is Flow, when analyzing dependencies does not respect their own flowconfigs. Which means it doesn't respect their resolution rules, doesn't know type definition locations, suppress comment format and so on. ¯\_(ツ)_/¯

@schovi
Copy link

schovi commented Feb 10, 2017

Hi, any progress or solution with this?
I am dont like to copy original .js.flow file from library (immutable js for example)

@eric-burel
Copy link

eric-burel commented Mar 28, 2017

@schovi How did you quickfixed this for immutable ? I don't get where I should copy the flow type file or include, so that I get at least something that works.

@schovi
Copy link

schovi commented Mar 28, 2017

@eric-burel I took this file https://github.com/facebook/immutable-js/blob/master/type-definitions/immutable.js.flow and copy it to root of flow libs. it should be in you .flowconfig file like this:

[libs]
./flow-typed

kadoyau added a commit to kadoyau/learning-js-stack-from-scratch that referenced this issue May 29, 2017
@eric-burel
Copy link

eric-burel commented Jun 6, 2017

@schovi I had to wrap the whole file content into declare module 'immutable" { ...} to get this (eventually) working.
Those required module not found errors seems often to be caused by bad "declare modules". For example redux-saga autogenerated flow types do not declare redux-saga/effects correctly.

@calebmer
Copy link
Contributor

Try installing library definitions from flow-typed with flow-typed install. This should give you the type definitions without checking code in node_modules. We will continue to think about this, please reopen if you still have issues 😊

@jquense
Copy link

jquense commented Oct 12, 2017

This situation is terrible and constantly keeps me from adapting flow on projects. Flowtype is constantly touting how easy it is to incrementally start using flow, while at the same time requiring any existing project to immediately either

  • write typedefs for every dependency or hope the flow-typed ones aren't broken
  • wade through 1000s of irrelevant errors from node_modules and try and understand the shadow dependency tree of which lib versions support the version of flow you are using.

Why is flow even trying to type check a third order dependency of of one of my eslint plugins? IT seems like the reasonable heuristic here is to not touch files that aren't even in the my apps dependency graph...

@eric-burel
Copy link

Hi @calebmer, those typings where actuallly installed using flow-typed,. As @jquense is pointing out, there some underlying issues when working with npm packages, I am not sure this should be closed (or maybe things have improved in new versions ?).

@loyd
Copy link

loyd commented Jan 29, 2018

It's a pain in the ass to distribute libraries with .js.flow files instead of monstrous flow-typed now due to the subject.

Please, reopen the issue.

@jdalegonzalez
Copy link

The issue is particularly acute when flow chooses to change the type of an already established return - say setInterval no longer returning a number (when the docs clearly state that it does). Now, every single module that happens to have use that function throws errors and countless libraries have to be updated by the maintainers before the project flows clean. Seeing all these errors and having to "just ignore them" causes signal attenuation and makes it hard to get any real value out of flow.

@stevemk14ebr
Copy link

stevemk14ebr commented Mar 3, 2018

How am i supposed to actually fix this? I just installed flow and i'm using Radium as a dependency. There's around 30 errors thrown from radium inside node_modules, and i did run flow-typed install already.

@StreetStrider
Copy link

StreetStrider commented Mar 3, 2018

@stevemk14ebr the only adequate way to use Flow right now is to:

  1. Completely ignore node_modules.
  2. Install (and commit) flow-typed definitions.
  3. Append them with manually writed missing definitions (and commit them) or at least stubs (flow-typed create-stub).

@TrySound
Copy link
Contributor

TrySound commented Mar 3, 2018

@StreetStrider @stevemk14ebr Completely ignore is stupid since a lot of packages have correct types. You may easily treat broken modules as any with [untyped] config section. Just pass similar to [ignore] regexp

[ignore]

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

@StreetStrider
Copy link

@TrySound thank you, I'll look onto your solution. I was hoping for such one. My solution is bulletproof for a couple of reasons:

  1. Packages may have their own flowconfig options and even Flow versions, while flow-typed/ always conforms my Flow version. Flow cannot dynamically switch it's mode/version when passing through package's boundaries.
  2. When working on backend or Node utility project you can't use package's builtin types directly in many cases, since such files not only have types but also tends to have special syntaxes (basicly import/export or even esnext features non-compatible with Node). So file which is consumed by Flow and file which is actually exectuted by Node will be different files. You'll need more complex setup (like custom module resolver or special build pipeline) in that cases.
    For that reasons I still recommend completely ignore node_modules/.

(3. I also remembering there was a performance issues scanning large node_modules/, but I expect that this issue is already resolved in modern Flow. I can't check it since I always ignore node_modules/ in my projects).

@TrySound
Copy link
Contributor

TrySound commented Mar 3, 2018

@StreetStrider

  1. I don't follow adding versions to flowconfig. Send PRs with removing them.
  2. Can't talk about cjs. I always use esm via babel or webpack/rollup.
  3. Performance issues was fixed. Currently only direct and transitive dependencies are checked

@StreetStrider
Copy link

@TrySound

  1. Even if no explicit versions lock, builtin defs can be non-compatible, that's what I'm talking about in that point. flow-defs always shipped for current Flow version.
  2. To be clear, are you speaking about backend setup or frontend setup? I was speaking about Node project (backend/util) where CJS is still rule. I can't see how you use build pipeline in that case for your deps (vendor code, not your code). Frontend is OK (deps are merged into bundle and processed). I'm thinking about using @std/esm+hooks as a good solution for Node setup.

@TrySound
Copy link
Contributor

TrySound commented Mar 4, 2018

Flow works fine with esm. It doesn't matter node or browser. I provide cjs dist via babel, but flow with esm. And interop works fine.

@StreetStrider
Copy link

@TrySound what your imports look like? I mean, import foo from 'package/foo' (es6 + flow), or import foo from 'package/dist/foo' (cjs node). If you use first one, after translation it can't be consumed by Node, since it points to non-transpiled versions of files in vendor code, if second one, Flow will lack builtin defs (since they are missing in already transpiled files) and warn.

@TrySound
Copy link
Contributor

TrySound commented Mar 4, 2018

  1. use flow-copy-source to copy all files with .flow extension and allow flow to see original source in destination path.
  2. use echo "// @flow\n\nexport * from '../src/index..js';" > dist/index.js.flow to have the same effect for bundled code.

@StreetStrider
Copy link

@TrySound great, thanks for the tips. I remember using .flow-files like half a year ago, but then this kind of annotations disappear from this section. I thought that gone deprec. Flow docs got bit gaps. For some time I also have issues when using .flow in vendor code, it was looked like Flow sees .flow-«doppelganger» files in my code, but ignore them in node_modules/'s.

Sometimes it's hard to say is it my issues using Flow or Flow issues itself. Docs are vague.

@TrySound
Copy link
Contributor

TrySound commented Mar 4, 2018

Often flow issues are related to editor. I use vim+ale which works great with flow. And I never had issues with .js.flow.

@StreetStrider
Copy link

Btw, neither [untyped] nor .js.flow decls features are documented.

@benbieler
Copy link

benbieler commented Aug 16, 2018

[untyped]
.*/node_modules/

@TrySound
Copy link
Contributor

@benbieler This will treat all out of the box type definitions as any and will hide a lot of errors from you.

@benbieler
Copy link

@TrySound Okay thanks :) What do you mean with box type definitions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests