HMR support multi clients

[cpunion]

Explain the motivation for making this change. What existing problem does the pull request solve?

Likes #11875 , I also need multi devices can update in HMR mode, currently it supports only one.
This PR can works with #11875, your devices can work with Live Reload or HMR mode.

Prefer small pull requests. These are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.

As small as possible.

Test plan (required)

Test steps:

1. Open multi devices in HMR mode.
2. Update your code.

Expected result:

1. Every device can update with HMR experience.

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @cpojer and @yungsters to be potential reviewers.

[cpojer]

@jeanlauliac can you take this one?

[hramos]

Waiting on review from @jeanlauliac

[cpunion]

Found a mistake, need fix, clients should be group by platform and bundleEntry. The problem is if clients are both iOS or both Android, it's fine, but not correct for iOS and Android.

[cpunion]

Fix a case

• Some Android and iOS clients connected, create/change/delete a component but only one platform

Steps:

1. Create an empty project
2. Connect some Android and iOS clients (Android first), and switch to HMR mode.
3. Add TestComponent.android.js and write a simple component.
4. Use TestComponent in index.android.js
5. Update TestComponent.android.js
6. Add TestComponent.ios.js and write a simple component
7. Use TestComponnent in index.ios.js
8. Update Test.Component.ios.js

Expect:

• All clients receive update events, and update correctly.

Bug in previous commit:

• iOS clients update not correctly

That's fixed. The last commit group clients by bundleEntry.

[cpunion]

Can someone help me to test?

@jeanlauliac Can you review now?

[cpunion]

Found a problem but not this PR's: On Android, If you enable HMR and then disable it, the WebSocket connection doesn't disconnect, must close it to let it be disconnected.

Not a big problem, but need to know if you test.

[jeanlauliac]

Yes, I'll try moving forward with this in the next few days. I'm not familiar with the existing HMR module so I'll have to comprehend that first. Thank you for the updates!

[facebook-github-bot]

@jeanlauliac has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[cpunion]

Should I do something to fix?

[mkonicek]

Hey @cpunion, please ignore the message from the bot above. This is because the pull request was imported to Phabricator but not accepted yet and the bot doesn't take that into account :/ You'll just need to wait for @jeanlauliac to review it.

[jeanlauliac]

Hi there, I just wanted to apply the patch locally and do a few tests. No action is required :) I'll seek to land this before the end of this week.

[jeanlauliac]

Hi there! I've been dragging feet on that PR—that I'm sad about—because I have low confidence that it's not going to break so edge cases; especially on a part of the code I'm not too familiar with. I realised that this file could use some automated testing, that is what you give me this missing confidence. I'll look into adding at least a unit test that verifies the basic behaviour, and I'll look into adding an integration test for testing HMR packager-wide. (On the bright side, we already have an e2e test covering HMR in CircleCI, but it might be flaky and covers only a tiny range of HMR behaviour.) After doing so I'll be a lot more confident into merging this changeset. I'd be grateful for automated testing contributions if this is of interest to you, otherwise I myself wish to look into it next week or so.

[cpunion]

I'll try to add e2e tests.

[jeanlauliac]

I'd like to strengthen the integration testing coverage before merging these kind of changes, something we're slowing improving. We also want to extract the packager on a separate repo eventually so that it's easier to contribute and review. I'm closing this PR for the time being, but I'd love to reopen once we get good integration testing.

[sballew-insteon]

@jeanlauliac Any update on the feasibility of merging a change like this? Multiple HMR clients would be great.