Macil / browserify-hmr

Hot Module Replacement plugin for Browserify
MIT License
373 stars 26 forks source link

Find free port if 3123 is in use #25

Closed Widdershin closed 8 years ago

Widdershin commented 8 years ago

Heya!

I've been making extensive use of this library recently, and it's been super useful.

I've been using it so much that I often find myself wanting to run two instance of budo + browserify-hmr at once. Currently this fails with a port in use error.

I've changed it to use find-port to find the next available port.

The only downside I can think of is if you explicitly specify a port that is in use, it will find the next free port, so it might end up running in a different place than you expect.

I would love to write a test for this but I'm not quite sure how to go about it with coroutines and promises. If you have any thoughts I would love to hear them.

Macil commented 8 years ago

This doesn't work. (The parameter to .then must be a function, findPort expects two parameters, and it doesn't write the new port number into the bundle.) The example/ directory depends on the NPM-published version of browserify-hmr; are you sure you weren't testing with that? (I usually replace example/node_modules/browserify-hmr with a symlink to ../.. when manually testing.) Yeah, the websocket stuff is a blind spot to the tests right now.

It is a good feature idea though. I was wondering if anyone was going to request this.

I looked at the find-port module and noticed there's a race condition with its design: another process can start listening on the port between the time find-port tells you a free port number, and you start listening on it. This could easily happen when someone starts up a build script that starts multiple browserify-hmr instances. I've just made the listen-on-free-port module which addresses this and I'm going to implement this in the next version of Browserify-HMR along with some other options changes.

Widdershin commented 8 years ago

Excellent! Thanks for implementing this. I must have indeed testing against the wrong version.