Add option for watchers to allow you to share watchers between watchify instances

[jballant]

Thank you so much for this browserify plugin. In my project, we have an issue as the result of a large number of files sharing a large number of dependencies. These files are getting watched by each watchify instance. As a result of all these watchers being created a bunch of the same files, we are seeing this error everywhere:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at StatWatcher.EventEmitter.addListener (events.js:160:15)
    at Object.fs.watchFile (fs.js:1163:8)
    at EventEmitter.FSWatcher._watch (/usr/local/lib/node_modules/modulename/node_modules/watchify/node_modules/chokidar/index.js:244:15)
    at EventEmitter.FSWatcher._handleFile (/usr/local/lib/node_modules/modulename/node_modules/watchify/node_modules/chokidar/index.js:270:8)
    at /usr/local/lib/node_modules/modulename/node_modules/watchify/node_modules/chokidar/index.js:356:15
    at Object.oncomplete (fs.js:107:15)

This is because node maintains a private event emitter hash for stats for all files the process is watching, so every shared dependency between watchify instances could be binding a listener to the same internal node watcher instance, despite being different chokidar watchers. Since we can't access the objects in node's private hash, we can't do anything about these alerts.

I'm not sure my solution is the best way to handle it, but I think it might sense to allow users to share watchers between watchify instances if they need to, if for no other reason than to preserve a bit of memory, but also to combat this error. A shared hash of chokidar hashes would better reflect whats going on "under the hood". Additionally, I think it would be a good idea to pair this change with the addition of a 'watch' event on the instance with the watcher and the dependency object so any wrapper plugins can manage these watchers themselves, if desired.

In order to make this work however, I had to check the watcher for the number of listeners bound to it before it could be closed in the invalidate function. Basically, I had to change invalidate to remove it's own listeners and then check if any listeners remain before closing the watcher.

I'm interested to hear other thoughts on this issue.

[robmunro]

Nice work @jballant:) I believe this might solve the problem I am currently having also. I have 20 or so separate bundles that I want to watchify at the same time. However these share many of the same dependancies, and am seeing this error (not warning in my case).

In order to try out this fork, how do I share the watchers from one with another? Or does this happen automatically?

[ghost]

Can either of you describe your use-case in more detail? You're running watchify to produce dozens of bundles at the same time? Are you using the API or command-line to do that?

[robmunro]

That is correct, I am running watchify on 20 or so separate bundles - so > 20 watchify instances are being started. Each of these bundles has similar dependancies, hence the problem I am having above.

Using the API to do this. Here is the code:

cachingCoffeeify = require 'caching-coffeeify'
watchify = require 'watchify'
source = require 'vinyl-source-stream'
gulp = require 'gulp'

files = fs.readdirSync asset.dir
for file in files when /\.coffee$/.test file
  fileToWatchify = "#{asset.dir}/#{file}"
  buildDir = path.join './build', "#{asset.dir}" 
  console.log "Gulp setup for #{fileToWatchify} at #{buildDir}"
  exports.startWatchifyJob(fileToWatchify, buildDir)

exports.startWatchifyJob = startWatchifyJob = (file, saveToDirectory) ->
  entries = []
  entries.push(file)

  # Get the base name
  fileBaseName = path.basename(file)

  # Then take off the extension
  fileBaseName = path.basename fileBaseName, path.extname(fileBaseName)

  # Setup watchify bundler
  bundler = watchify
    entries: entries
    extensions: ['.coffee']
    cache: true
    minify: true
    gzip: true
    debug: false
    insertGlobals: true
    detectGlobals: false

  # Add coffeeify transform so that coffee files are accepted as files in bundle
  bundler.transform 'caching-coffeeify'

  console.log 'saveToDirectory', saveToDirectory
  # Bundle, chuck into a js into the same name, and reload page
  rebundle = ->
    return bundler.bundle()
      .pipe(source("#{fileBaseName}.js"))
      .pipe(duration("Building browserified #{fileBaseName}.js file"))
      .pipe(gulp.dest(saveToDirectory))
#      .pipe(livereload())

  # Listen for update so we can rebundle
  bundler.on 'update', rebundle

  # Bundle on startup
  return rebundle()

I have edited this slightly as it wouldn't have made sense as it was so hopefully it is not too confusing :)

[joemaller]

I might have a related issue: I'm trying to integrate watchify into a small set of gulp watch tasks, basically I want watchify to handle rebuilding my js, gulp to deal with css and a few other tasks. My problem is that watchify seems to be stomping on all the gulp watches. If watchify runs, none of gulp's watches fire. If I disable watchify, gulp's watches all work correctly.

[jballant]

@robmunro's example should illustrate the issue, but I'll try and provide a simple description of a use case as well.

I am using my own simple gulp extension that creates a watchify instance for each entry file using the watchify API. As an example, let's say I have around 15 entry files, each of which requires 'shared.js'. That mean's each one of these 15 watchify instances will create a chokidar instance on 'shared.js'. Because Node internally creates a private shared hash of files being watched, that means that although there are 15 chokidar watcher instances for 'shared.js', internally to Node there is only one. Creating these chokidar instances means that node adds 15 event listeners to this internal watcher (this is in Node v10.26). Because Node has a maxListeners property that is set to 10 by default, I get this warning.

[builtbylane]

@joemaller (+1) I'm encountering the same issue as you.

[iclanzan]

@joemaller and @builtbylane I am also encountering the same problem.

[tmtxt]

I encountered the same issue. Is this problem fixed now?

[rkit]

I have the same problem.

[wmayner]

👍

[minodisk]

Same problem. 👍

[elsbree]

👍

[zertosh]

I think #105 solved this. If the issue persists please open a new issue.

Side note: For resource constrained systems, having the ability to ignore paths (like node_modules) should go a long way. Hopefully something like #65 will see the light of day soon.