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

No way to Ignore devDependencies? #719

Closed
dchambers opened this issue Aug 14, 2015 · 20 comments
Closed

No way to Ignore devDependencies? #719

dchambers opened this issue Aug 14, 2015 · 20 comments

Comments

@dchambers
Copy link

This is a question as much as anything, and I'm hoping you're just going to tell me that I'm doing it wrong.

If I try to use flow check on even a very simple NPM library with just one dependency it's unusably slow (~13secs) because of all the devDependencies I'm using. If I add .*/node_modules* to the ignore section of .flowconfig than it becomes much faster (~1sec), but fails because it can't find the single non dev-dependency, showing this message:

/home/dominicc/git-dev/fell/src/Log.js:4:15,30: emitr
Required module not found

Adding entries to the include section to counter this doesn't seem to help, regardless of how I do it. Given that this isn't very sustainable either (you'd essentially have to maintain two dependency manifests), is there a better way of ignoring devDependencies to improve performance?

@samwgoldman
Copy link
Member

I have seen this as well, and hopefully someone else can chime in with their successful method of ignoring node_modules without hitting the missing package.json issue.

However, since performance is your problem, have you tried using the flow server instead of running flow check every time? If you just run flow, Flow will start a server, load and parse the files, and will then watch the file system, updating itself automatically. If you run flow again, it should be super fast.

@dchambers
Copy link
Author

However, since performance is your problem, have you tried using the flow server instead of running flow check every time?

I have, but that makes no difference for me at all (still ~13secs). I've tried running the server in both the background and foreground too, neither with any apparent effect.

@samwgoldman
Copy link
Member

That's really surprising. You start the server then just run flow (not flow check—that doesn't use the server)?

@dchambers
Copy link
Author

Ignore the last message. I'd misunderstood that you have to run flow the second time, and not flow check! Now it's really fast even with the devDependencies in the mix. Would still be good to have an option to ignore these though for use on a CI machine, etc.

dchambers added a commit to BladeRunnerJS/fell that referenced this issue Aug 14, 2015
I've since found out that the `flow` command can be used to first start
a server, and then to perform the equivalent of `flow check` against the
background server.

Additionally, I've raised an issue that should hopefully address making
`flow check` faster in CI:

  * facebook/flow#719
@GGAlanSmithee
Copy link

I am also having issues with this. I wish to ignore mode_modules for performance, but still be able to require a npm package.

Would be nice to be able to have flow not cover node_module/* but still recognize the existence of modules that are explicitly required or imported in code.

*EDIT for clarify: I understand the solution of starting the flow server with flow and then checking only updated files with flow again, but there are situations ,such as having a flow check step be a part of a build (even if this might not be the recommended way to use flow), that would require flow to recognize these modules.

@dchambers
Copy link
Author

Glad it's not just me.

To provide some further justification for why this would be helpful in my case, consider that our developers will run the same npm test command as used on CI, so that developers never need to debug via CI, which would be hugely time consuming. Since running flow check doesn't benefit from a server running in the background, an npm test command that involves flow is slow for both developers and CI, yet needn't be.

@GGAlanSmithee
Copy link

Some additional info.

I just remembered why I ignored node_modules in the first place. I use an in-browser IDE (cloud9) and my workspace has limited CPU/memory. I also use a lot of node packages and this combo makes my environment run out of resources trying to run flow without ignoring node_modules first.

Maybe flow isn't suited for my use-case, but if you want to support these kind of scenarios, there needs to be a way to ignore node_modules but still be able to require a package.

@GGAlanSmithee
Copy link

Update for anyone having this problem and finding this issue.

It turns out flow handles these situations in a very nice way using Declarations (at least as far as my use case is concerned)

By ignoring node_modules and then adding a declaration of the required module, flow will OK it (as long as the declaration is consistent with the usage.) This is also nice in that it allows you to code to an abstraction.

An example would be:

src/test.js

/* @flow */

import { Foo } from 'foo';

export default class Bar {
    foo : Foo,
    constructor() {
        this.foo = new Foo();
    }
}

lib/foo.js

declare module foo {
    declare class Foo {
    }
}

.flowconfig

[ignore]
.*/node_modules/*.

[include]
.*/src/.*

[libs]
lib/

@chrisdevereux
Copy link

Another workaround:

Use a project structure like:

- package.json
- node_modules
+ libs
  + module1
    - package.json
    - node_modules
    - index.js
  - module2
  - .flowconfig

Manage your dev dependencies with the outer package.json and runtime deps with the inner package.json. Flow will ignore the dev dependencies since they are outside its scope.

@nevir
Copy link

nevir commented Dec 29, 2015

@GGAlanSmithee note that the [include] .*/src/.*in your configuration has no effect. [ignore] takes precedence :( (also, you wouln't want it matching node_modules w/ a src directory inside them)

@GGAlanSmithee
Copy link

@nevir I'm not sure I follow. How would [ignore] take precendence over [include] in my example? (As long as you don't have a node_modules folder in your src folder.)

I'm not sure how the config behaves as far as what takes precendence over what, but your second statement could be troublesome if [include] takes precendence. This could be solved by using the absolute path src/.* though, right?

Thinking about it, it might be best to just use absolute paths?

[ignore]
node_modules/.*

[include]
src/.*

[libs]
lib/.*

EDIT:

If using an absolute path to [include], I guess we don't need to explicitly [ignore] anything, but what I've found happen then is that running flow will make it check all files in all folders not explicitly ignored, causing a major increase in need for CPU and startup times, so I think the way to go here is to be explicit on what to ignore.

@nevir
Copy link

nevir commented Dec 29, 2015

It's my understanding that every js file in your project is included by
default (I.e. in the same directory as, or a sub directory of,
.flowconfig). Adding a path to the [include] also doesn't appear to
switch it into a white listing mode.

AFAIK the main use of [include] is to let flow pick up files that are
outside of your project root

On Tue, Dec 29, 2015, 02:40 Alan Smithee notifications@github.com wrote:

@nevir https://github.com/nevir I'm not sure I follow. How would
[ignore] take precendence over [include] in my example? (As long as you
don't have a node_modules folder in your src folder.)

I'm not sure how the config behaves as far as what takes precendence over
what, but your second statement could be troublesome if [include] takes
precendence. This could be solved by using the absolute path src/.*
though, right?

Thinking about it, it might be best to just use absolute paths?

[ignore]
node_modules/.*

[include]
src/.*

[libs]
lib/.*


Reply to this email directly or view it on GitHub
#719 (comment).

@Kim-Andersen
Copy link

Ignoring node_modules and declaring required modules works fine for me. However, using gulp-flowtype which basically runs flow on changed files automatically, flow always reports errors of missing package.json files for each declared module.

Do you know if there's a workaround for this?

@kumar303
Copy link

kumar303 commented Feb 5, 2016

Just piling on here with an example of a hacky workaround. I am auto-generating ignore rules based on my package.json. I added it to my project in this patch: https://github.com/mozilla/web-ext/pull/23/files

Because of npm module flattening these auto-generated ignore rules made the standalone flow check run twice as fast, so it seemed worth it. That's 3.5 seconds instead of 7 seconds which is a big deal for continuous integration.

I decided not to ignore all modules (and define interfaces) because I wanted to let Flow tell me when I'm missing an external module or using it wrong.

@kumar303
Copy link

I think lazy-loading (#1461) would address this better because if you opted out your test files (i.e. no @flow) then the dev-dependencies wouldn't be analyzed.

@samwgoldman
Copy link
Member

Version 0.22 should greatly improve this problem. As of that version, Flow does not parse non-@flow files, which should greatly improve cold start times for projects with large node_modules directories.

I'm going to try on my end to see, but I'd encourage everyone here to upgrade and try it out.

@nevir
Copy link

nevir commented Feb 26, 2016

That sounds like it'll be a huge improvement! I'm assuming it's still scanning all JS files under node_modules, though?

@samwgoldman
Copy link
Member

@nevir Yeah, we still check, because dependencies might be @flow—increasingly, I hope!

@Epskampie
Copy link

Epskampie commented Apr 26, 2017

This is still a huge pain if you have a big node_modules directory, and not using an SSD. It is caused by the combination of these design decisions:

  • Flow includes node_modules dir by default
  • Ignores are processed AFTER includes. If you both include and ignore a file it will be ignored.

This leaves you with only ineffective solutions, since you have to explicitly blacklist modules you don't want, and cannot whitelist modules you do want.

I'm resorting to just generating a list of all node_modules using the linux ls -1 command, and including this whole list in the .flowconfig, while deleting the entries that I want:

[ignore]
.*/node_modules/config-chain/.*
.*/node_modules/npmconf/.*
# ... etc

Another, slightly more sophisticated solution to generate the ignore section with regexes is here: https://gist.github.com/lydell/ee05b5acbc1f51eea649926af956dc98

@vkurchatkin
Copy link
Contributor

Closing in favor of #869

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

9 participants