Skip to content
New issue

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

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

Already on GitHub? # to your account

Debounce/Throttle option? #134

Closed
CWSpear opened this issue Apr 11, 2014 · 9 comments
Closed

Debounce/Throttle option? #134

CWSpear opened this issue Apr 11, 2014 · 9 comments

Comments

@CWSpear
Copy link
Contributor

CWSpear commented Apr 11, 2014

I'm working on a larger project with a lot of CoffeeScript files. When I change one of the files, it currently reprocesses all of those files, and browser-sync is watching the compiles JS.

When I save a CoffeeScript file, I get the following output:

# these two lines for every file that "changed" (a few dozen)
[BS] File Changed: contacts.js
[BS] Reloading all connected browsers...
# ... (omitting all of the files, but you get the idea)

child_process.js:935
    throw errnoException(process._errno, 'spawn');
          ^
Error: spawn EMFILE
  at errnoException (child_process.js:988:11)
  at ChildProcess.spawn (child_process.js:935:11)
  at Object.exports.spawn (child_process.js:723:9)
  at module.exports (/Users/cameron/Sites/proximic/planner_ui/src/node_modules/terminal-notifier/terminal-notifier.js:27:17)
  at EventEmitter.<anonymous> (/Users/cameron/Sites/proximic/planner_ui/src/gulpfile.js:235:9)
  at EventEmitter.emit (events.js:117:20)
  at [object Object]._onTimeout (/Users/cameron/Sites/proximic/planner_ui/src/node_modules/browser-sync/lib/file-watcher.js:78:29)
  at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

It seems like a throttle and/or debounce option might help here. We only need to reload the browsers one time for this batch of files. If I had a debounce of even 100ms, I don't think it'd crash like this.

@CWSpear
Copy link
Contributor Author

CWSpear commented Apr 11, 2014

Forgot to mention I tried both the reloadDelay and fileTimeout options, but they didn't solve this particular issue.

CWSpear added a commit to CWSpear/browser-sync that referenced this issue Apr 11, 2014
…sarily. Solves BrowserSync#134

I added a `debounce` option that when used turns the change callback function into a debounced function a la Lodash that will wait `options.debounce` milliseconds since the last call to actually execute. If the `debounce` option isn't provided or is 0, then it behave as it did before this change.
@CWSpear
Copy link
Contributor Author

CWSpear commented Apr 11, 2014

In testing, #135 does solve my issue. With a debounce of 100ms, it does trigger 2 change events, but it appears to be because of 2 different steps in my build, so it's likely because there is a > 100ms gap between the two steps.

But regardless, I can either increase the debounce, or live with 2 change events (which I can definitely live with). 2 is much better than a couple dozen and browser-sync crashing Gulp!

@shakyShane
Copy link
Contributor

fileTimeout was removed as it was still very unreliable. When you think about it, adding a debounce/throttle is always going to be prone to errors because the actual compilation time is unknown. (although I'm still happy to have a debounce option - I just don't think it's the best solution)

There's already a way to emit file change events to Browser Sync (see the last example here) & that way you can remove the file watching altogether and handle it manually. (although it's still not Ideal if you don't know the file names).

Ideally going forward, I want something like this to work:

gulp.task('coffee', function () {
    gulp.src('coffee/*.coffee')
        .pipe(task1())
        .pipe(task2())
        .pipe(task3())
        .pipe(gulp.dest('js'))
        .pipe(browserSync.reload())
});

That's one of the big goals of next release.

@CWSpear
Copy link
Contributor Author

CWSpear commented Apr 14, 2014

That's a better approach! Good call.

@shakyShane
Copy link
Contributor

^ hoping to have the improvement to gulp workflow above released tonight/tomorrow :)

@shakyShane
Copy link
Contributor

on npm at version 0.8.0

@CWSpear
Copy link
Contributor Author

CWSpear commented May 15, 2014

@shakyShane by the way, the new way of putting it at the end of the pipe works really nice. It was a good call. =)

@shakyShane
Copy link
Contributor

@CWSpear Thanks! just make sure you're using at as described here & not as I mentioned in this thread. :)

@CWSpear
Copy link
Contributor Author

CWSpear commented May 15, 2014

Yeah, I did look up the official usage, first.

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

No branches or pull requests

2 participants