Skip to content
This repository has been archived by the owner on Jul 13, 2021. It is now read-only.

fix(socket-server): send stats to new connections #102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(socket-server): send stats to new connections #102

wants to merge 2 commits into from

Conversation

ryanclark
Copy link
Collaborator

@ryanclark ryanclark commented Aug 31, 2018 •
edited by michael-ciniawsky

This sends the compilation stats directly to new connections, instead of broadcasting them globally when there is only one client connected. This fixes the issue where pages would not hot reload until a file was saved twice, as it ensures every new client receives the "initial" ok event. Should fix #92 and #93.

This worked fine with 3.0.0 because of a bug - it used to check connections.length, but because it's a Set, this was always undefined, meaning the check always passed.

By broadcasting to new connections only, this should fix #61 (as the parent window will not receive an ok event when the child connects, forcing the page to refresh). In turn, it addresses the concerns that #96 was made to fix.

Update:

I've created a bare bones setup based off of #61 and tested it on these changes. No page refreshing and both the parent window and iframe reload correctly when their respective entry files are changed.

Type

  • Fix

Issues

SemVer

  • Patch

@ryanclark
fix(socket-server): send stats to new connections
44ce688 
directly send stats to new connections instead of only broadcasting them globally when there is one client connected
@ryanclark
Copy link
Collaborator Author

Also, if we remove these lines of code -

webpack-hot-client/lib/client/index.js

Lines 7 to 9 in 276d950

if (window.__webpackHotClient__) {
return;
}

This will also fix #95 - thoughts on including that?

I've ran a test against the code in #61 as well as the same code but without the iframe (both entries in index.html) and removing that line as well as the changes in this PR gives us the expected correct behaviour.

@michael-ciniawsky michael-ciniawsky changed the title fix(socket-server): send stats to new connections fix(socket-server): send stats to new connections Sep 15, 2018
@michael-ciniawsky
Copy link
Member

@rynclark Would you like to include the fix mentioned in #102 (comment) in this PR or open or would you like to open a separate PR for it as a follow up?

@rudi23
Copy link

rudi23 commented Sep 21, 2018

@michael-ciniawsky could you merge it, please? :)

@ryanclark
Copy link
Collaborator Author

webpack-serve has been deprecated and as far as I'm aware, the author also has exclusive publish rights to this package on npm, effectively making webpack-hot-client useless from this point onwards

@rudi23
Copy link

rudi23 commented Sep 21, 2018

Thanks for the info 👍 it's new for me that webpack-serve has been deprecated

@ryanelian
Copy link

ryanelian commented Sep 24, 2018 •
edited

webpack-serve has been deprecated and as far as I'm aware, the author also has exclusive publish rights to this package on npm, effectively making webpack-hot-client useless from this point onwards

Ouch. Please merge the fix and republish under new name? :)

(https://www.npmjs.com/package/webpack-hot is available)

I'd hate to see this amazing package go away.

@JasonBoy
Copy link

so when this will be merged?

@hedgepigdaniel hedgepigdaniel mentioned this pull request Sep 20, 2019
Merged
@hedgepigdaniel
Copy link

I've forked this repository at https://github.com/hedgepigdaniel/webpack-hmr-client and merged this, along with some other fixes.

Its available on NPM as webpack-hmr-client

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@michael-ciniawsky michael-ciniawsky michael-ciniawsky approved these changes

Assignees
No one assigned
Labels
semver: patch status: pending type: bug
Projects
None yet
Milestone
No milestone
6 participants
@ryanclark @michael-ciniawsky @rudi23 @ryanelian @JasonBoy @hedgepigdaniel