Allow both HMR clients and live reloading clients

[cpunion]

In current implementation, if a HMR client conntcted, live reloading will be disabled, all other clients don't update. Sometimes we want multi clients both work, maybe some clients are live reloading and others are hot reloading, this PR allows them to work on same server.

Test plan (required)
In current implementation, packager server just allow one HMR client, so before every test begins, must set every clients to enable live reloading and disable live reloading and reload them. (because the last HMR client will overwrite other HMR clients notification, see:

react-native/local-cli/server/util/attachHMRServer.js

Line 132 in a461d25

packagerServer.setHMRFileChangeListener((filename, stat) => {

)

• Test 1: Connect multi clients, set an iOS client to hot reloading, update code, expect just this iOS client do hot reloading, and other clients reloaded.
• Test 2: Connect multi clients, set an Android client to hot reloading, update code, expect just this Android client do hot reloading, and other clients reloaded.
• Test 3: If also apply [HMR] Multiple HMR clients #6179, allow many clients work with hot reloading, and others work with live reloading.

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @nicklockwood, @mkonicek and @martinbigio to be potential reviewers.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

[martinbigio]

I think this is the single required change to make this work but I'm not sure what would be the best way to support this feature without affecting HMR performance.

The problem is that inside of _hmrFileChangeListener lots of things happen asynchronously so if you remove the return those async operations will compete for CPU and IO with the Live Reload changes. For Live Reload it doesn't matter if the change takes a couple more of milliseconds but for HMR it does.

Even if we turn _hmrFileChangeListener into a function that returns a promise and wait to execute the other listeners, we'll end up slowing the HMR codepath because the code is not injected on the client until the sourcemaps for the new file are requested and returned (see

react-native/Libraries/Utilities/HMRClient.js

Lines 112 to 135 in 235b16d

	require('SourceMapsCache').fetch({
	text: code,
	url: `http://${serverHost}${sourceURLs[i]}`,
	sourceMappingURL: sourceMappingURLs[i],
	});
	// on JSC we need to inject from native for sourcemaps to work
	// (Safari doesn't support `sourceMappingURL` nor any variant when
	// evaluating code) but on Chrome we can simply use eval
	const injectFunction = typeof global.nativeInjectHMRUpdate === 'function'
	? global.nativeInjectHMRUpdate
	: eval;
	code = [
	`__accept(`,
	`${id},`,
	`function(global,require,module,exports){`,
	`${code}`,
	'\n},',
	`${JSON.stringify(inverseDependencies)}`,
	`);`,
	].join('');
	injectFunction(code, sourceURLs[i]);

)

For this change to not affect performance we'll need to also include the sourcemaps on the original request response.

cc @satya164

[cpunion]

I think this is the single required change to make this work but I'm not sure what would be the best way to support this feature without affecting HMR performance.

The changes of other two files(RCTDevMenu.m and DevServerHelper.java) avoid the clients process both onchange and HMR, because clients can enable live reloading and hot reloading.

Thanks for your point, I tested competition, HMR first and Live Reload first, every are not good, just worked.

Is it possible to bundle once for Live Reload and HMR? or more common works to reduce bundle time? I can't handle this topic, I don't know details of how packager/bundler works yet.

[martinbigio]

@cpunion thanks for answering so quickly and for contributing!

Is it possible to bundle once for Live Reload and HMR? or more common works to reduce bundle time? I can't handle this topic, I don't know details of how packager/bundler works yet.

Unfortunately no. We reuse much of the intermediate stuff though, but the bundle itself cannot be share as the Live Reload one is a full bundle whereas the HMR one contains only the the code of the modules that have changed + the new modules if any.

I'm not sure if it worth supporting this use case, feel like it's an edge case but I might be wrong. If it's an edge case we need to make sure it doesn't make the HMR experience worse and that the additional complexity that it adds worth it.

I think it would be very useful to create a product pains and ask around if this is something the community would like the support for :). If there's broad interest we could work on improving this pull request to avoid a hit on performance. What do you think?

[cpunion]

@martinbigio Thanks for your explain clearly. I think if HMR works fine and can replace Live Reload, we no longer need Live Reload. Yes, it seems an edge case.

Thanks again.

[cpunion]

So may I close this PR?

[martinbigio]

Yup, go ahead. Thanks again for contributing!.