Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Fixes Multi-Socket Reload Bug #82

Merged
merged 1 commit into from
Feb 2, 2017
Merged

Fixes Multi-Socket Reload Bug #82

merged 1 commit into from
Feb 2, 2017

Conversation

Anaphase
Copy link
Contributor

@Anaphase Anaphase commented Feb 1, 2017

The socket close event handler appears to have a bug in it. As far as I know, the only acceptable first parameter to Array::splice is a Number (as opposed to an object as seen in line 65 of index.js.)

Specifying an object as the first parameter appears to always splice the first element off. This pull request fixes this issue by using Array::indexOf to look up the index of the socket first.

Some steps to illustrate this bug:

  1. Start your brunch server.
  2. Load up the page in Chrome.
  3. Load up the page in Firefox.
  4. Make a change and see that both browsers refresh.
  5. Close Firefox. This will cause Chrome's socket to to be popped off of the conns array instead of Firefox's.
  6. Make a change and see that Chrome is no longer responding.

The socket close event handler appears to have a bug in it. As far as I know, the only acceptable first parameter to `Array::splice` is a Number (as opposed to an object as seen in [line 65 of index.js(https://github.com/brunch/auto-reload-brunch/blob/673350f86ea4c28efd2a9379e03ca6e24c25a5f1/index.js#L65)]).

Specifying an object as the first parameter appears to always splice the first element off. This pull request fixes this issue by using `Array::indexOf` to look up the index of the socket first.

Some steps to illustrate this bug:

1. Start your brunch server.
2. Load up the page in Chrome.
3. Load up the page in Firefox.
4. Make a change and see that both browsers refresh.
5. Close Firefox. This will cause Chrome's socket to to be popped off of the `conns` array instead of Firefox's.
6. Make a change and see that Chrome is no longer responding.
@paulmillr paulmillr merged commit 65148cd into brunch:master Feb 2, 2017
@Anaphase Anaphase deleted the patch-1 branch February 2, 2017 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants