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

Passing options to chokidar #162

Closed
zertosh opened this issue Mar 23, 2015 · 11 comments
Closed

Passing options to chokidar #162

zertosh opened this issue Mar 23, 2015 · 11 comments

Comments

@zertosh
Copy link
Member

zertosh commented Mar 23, 2015

There seem to be two ways to do this: (1) via something like a opts.chokidar, or (2) piecemeal options like usePolling, interval and ignored.

  1. via something like a opts.chokidar:
    • Pros:
      • no maintenance on the part of watchify since chokidar just takes a whole object blindly.
      • it's mostly isolated from browserify options since opts.chokidar is unique.
    • Cons:
      • it's a mouthful via the command line (e.g. --chokidar [ --usePolling --ignored '**/node_modules/**' ] vs --usePolling --ignoreWatch '**/node_modules/**').
  2. piecemeal options:
    • Pros:
      • concise.
      • implementation agnostic (there are alternatives to chokidar like sane which can piggyback on watchman).
    • Cons:
      • clashes with existing browserify options (the entire opts object is passed to module-deps, browser-pack, etc.).
      • no options autonomy since options would have to be picked off the opts object.
@zertosh
Copy link
Member Author

zertosh commented Mar 23, 2015

Regardless of what way is decided, it is clear that having a way to specify usePolling is very important for NFS watching to work (see #120)

@tarsolya
Copy link

@zertosh I like 1.), but using the --chokidar flag is not really intuitive for someone quickly glancing for an option for watchify to ignore globs.

I think something along the lines of the following would be optimal:

  • Translate a simple --ignoreWatch like option to a default chokidar opts hash.
  • For fine-grained control one can look into documentation and use --chokidar excplicitly.

How's that sound?

@jonbretman
Copy link

Has anyone raised the issue of wanting to pass other options to chokidar other than usePolling? If not then maybe that makes the most sense as it is easy to understand and as @zertosh says implementation agnostic.

@jareware
Copy link

I came here to open an issue on this; seems I don't have to. :)

My (h|n)asty workaround is a contrib/polling-watchify.js with:

var chokidar = require('watchify/node_modules/chokidar');
var origWatch = chokidar.watch;

// Monkey-patch chokidar.watch() to use polling:
// @see https://github.com/substack/watchify/blob/2.4.0/index.js#L83
// @see https://github.com/substack/watchify/blob/2.4.0/index.js#L98
chokidar.watch = function(file, opts) {
    // @see https://github.com/paulmillr/chokidar#performance
    opts.usePolling = true;
    opts.interval = 500;
    return origWatch.call(chokidar, file, opts);
};

// Proxy to the actual watchify "binary":
require('watchify/bin/cmd');

@jareware
Copy link

Also in the case of having to use polling I would assume it'd be hugely beneficial to be able to exclude certain paths, say, node_modules from being actively watched. At least in my setup node_modules content never changes unless I also restart the watchify process.

@zertosh
Copy link
Member Author

zertosh commented Mar 25, 2015

Translate a simple --ignoreWatch like option to a default chokidar opts hash.
For fine-grained control one can look into documentation and use --chokidar excplicitly.

@tarsolya That sounds good, but the issue then becomes deciding which flag overrides which, or how to merge them. That's not a technical challenge but a UX one - one whose outcome will make someone unhappy for sure.

I'm leading more towards mapping:

  • ignoreWatch -> ignored and,
  • usePolling: true to {usePolling: true, interval: [defaultValue]}, and
  • usePolling: integer to {usePolling: true, interval: integer}

And take the remaining options on case-by-case basis. I'm generally apprehensive about expanding API surface area - anywhere.

Has anyone raised the issue of wanting to pass other options to chokidar other than usePolling?

@jonbretman: Nope just that and only because it's a solution to NFS watching - which is kinda common since everyone wants to run and develop their apps in containers. Passing ignored to chokidar is a cheap solution to a more common request than NFS watching - which is to ignore files.

My (h|n)asty workaround is a contrib/polling-watchify.js with:

@jareware O_o wow heh. Thank you for sharing that snippet - it's clear that we need to support passing the polling option. Quick tip: since you're monkey-patching chokidar already, you can also pass in ignored to ignore paths - that'll still create chokidar instances but not watchers (that's the expensive bit).

@zertosh
Copy link
Member Author

zertosh commented Mar 25, 2015

I'll come up with something in the next few days. I want to cut another 2.x release that resolves (1) listeners leak in cmd.js and (2) EPERM on windows, before introducing a 3.x release that bumps chokidar to 1.0.0-rc4 and allows passing (some) options (in some form).

@tarsolya
Copy link

@zertosh Fair enough and I'm with you on keeping the API surface as tight as possible. I think what you proposed covers the majority of ignore use cases very well. And the custom ones ... well, they will have to be made custom anyway.

👍

@jareware
Copy link

@zertosh Ha, thanks for the tip! Also really looking forward to dropping the monkey-patching nonsense with an upcoming release. ;)

@zertosh
Copy link
Member Author

zertosh commented Mar 30, 2015

Just published 3.0.0 (see #170). It has the options as I described them above.

@zertosh zertosh closed this as completed Mar 30, 2015
@jareware
Copy link

Wicked, thanks!

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

No branches or pull requests

4 participants