From 1f04ff580d243134f19129efdc9da315265a55f4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 24 Jun 2019 09:45:29 -0700 Subject: [PATCH] Make "Enable Hot Reloading" Instant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: As we saw in D15947985, and later traced down to D5623623, the `hot` option isn't used by Metro anymore. The relevant transforms _always_ run in DEV regardless of the option. Given that, it doesn't make sense that enabling or disabling Hot Reloading forces a full refresh. This significantly raises the usage barrier because **currently, you might have to wait ~20 seconds (on a large app) to just start using Hot Reloading when you're already in the middle of some screen.** So you just end up not using it. This diff changes enabling/disabling Hot Reloading to be _instant_. Here's how it works: 1. Now we always send the necessary info to the client via the new `HMRClient.setup()` function. It creates a Metro HMR client instance, but only actually sets up the socket if Hot Reloading is on. 2. The "Enable Hot Reloading" menu no longer forces a reload. Instead, it calls `HMRClient.enable()` which lazily sets up a socket (at most once). 3. The "Disable Hot Reloading" menu also doesn't trigger a refresh now. Instead, it calls `HMRClient.disable()`. We don't actually tear down the socket here because it's a pain to deal with race conditions and such. Instead, we keep the connection — but we _ignore the updates_ that come in while we're disabled. 4. As a result, it is possible to enable and disable it many times during a single session. (Updates while disabled would be ignored — which has a risk of making your running app inconsistent — but I'd argue it's expected and is worth it. You can always save a particular file to force it to update once the mode is on.) 5. In order to support "ignoring" updates, Metro's `HMRClient` (not to be confused with RN's module) now supports a `shouldApplyUpdates` field. The RN module uses it to disable handling updates when the mode is off. 6. In case there is an error that makes hot reloading unavailable (such as the server disconnecting), we surface the error only if the mode is on. If the mode is off, we stash the error message in the `_hmrUnavailableReason` variable, and display it next time you try to enable Hot Reloading. Reviewed By: rickhanlonii Differential Revision: D15958160 fbshipit-source-id: 8256fc4d5c2c3f653a78edf13b8515a5671953e4 --- Libraries/Utilities/HMRClient.js | 136 +++++++++++++++--- React/CxxBridge/RCTCxxBridge.mm | 10 +- React/Modules/RCTDevSettings.mm | 17 ++- .../devsupport/DevSupportManagerImpl.java | 32 +++-- .../facebook/react/devsupport/HMRClient.java | 14 +- 5 files changed, 172 insertions(+), 37 deletions(-) diff --git a/Libraries/Utilities/HMRClient.js b/Libraries/Utilities/HMRClient.js index 4c275c98b40..fbe236402df 100644 --- a/Libraries/Utilities/HMRClient.js +++ b/Libraries/Utilities/HMRClient.js @@ -16,22 +16,82 @@ const MetroHMRClient = require('metro/src/lib/bundle-modules/HMRClient'); import NativeRedBox from '../NativeModules/specs/NativeRedBox'; +let _didSetupSocket = false; +let _hmrClient = null; +let _hmrUnavailableReason: string | null = null; + /** * HMR Client that receives from the server HMR updates and propagates them * runtime to reflects those changes. */ const HMRClient = { - enable(platform: string, bundleEntry: string, host: string, port: number) { + enable() { + if (_hmrUnavailableReason !== null) { + // If HMR became unavailable while you weren't using it, + // explain why when you try to turn it on. + throw new Error(_hmrUnavailableReason); + } + + invariant(_hmrClient, 'Expected HMRClient.setup() call at startup.'); + _hmrClient.shouldApplyUpdates = true; + + // We connect lazily. This only ever must run once. + if (!_didSetupSocket) { + _didSetupSocket = true; + _hmrClient.enable(); + } + + // Intentionally reading it outside the condition + // so that it's less likely we'd break it later. + const modules = (require: any).getModules(); + if (_hmrClient.outdatedModules.size > 0) { + let message = + "You've changed these files before turning on Hot Reloading: "; + message += + Array.from(_hmrClient.outdatedModules) + .map(id => { + const mod = modules[id]; + return getShortModuleName(mod.verboseName); + }) + .join(', ') + '.'; + message += + "\n\nThese pending changes won't be reflected unless you save them again " + + 'or perform a full reload.'; + console.warn(message); + // Don't warn about the same modules twice. + _hmrClient.outdatedModules.clear(); + } + }, + + disable() { + invariant(_hmrClient, 'Expected HMRClient.setup() call at startup.'); + // Note: we don't actually tear down the connection. + // We just tell the client to ignore updates. + // This lets us avoid reasonining about complex race conditions + // if the user toggles the setting on and off. + _hmrClient.shouldApplyUpdates = false; + }, + + // Called once by the bridge on startup, even if hot reloading is off. + // It creates the HMR client but doesn't actually set up the socket yet. + setup( + platform: string, + bundleEntry: string, + host: string, + port: number | string, + isEnabled: boolean, + ) { invariant(platform, 'Missing required parameter `platform`'); invariant(bundleEntry, 'Missing required paramenter `bundleEntry`'); invariant(host, 'Missing required paramenter `host`'); - + invariant(!_hmrClient, 'Cannot initialize hmrClient twice'); // Moving to top gives errors due to NativeModules not being initialized const HMRLoadingView = require('./HMRLoadingView'); /* $FlowFixMe(>=0.84.0 site=react_native_fb) This comment suppresses an * error found when Flow v0.84 was deployed. To see the error, delete this * comment and run Flow. */ + const wsHostPort = port !== null && port !== '' ? `${host}:${port}` : host; bundleEntry = bundleEntry.replace(/\.(bundle|delta)/, '.js'); @@ -43,6 +103,7 @@ const HMRClient = { `bundleEntry=${bundleEntry}`; const hmrClient = new MetroHMRClient(wsUrl); + _hmrClient = hmrClient; hmrClient.on('connection-error', e => { let error = `Hot reloading isn't working because it cannot connect to the development server. @@ -76,29 +137,31 @@ Error: ${e.message}`; }); hmrClient.on('update-start', () => { - if (enableLoadingView) { + if (hmrClient.shouldApplyUpdates && enableLoadingView) { HMRLoadingView.showMessage('Hot Reloading...'); } }); hmrClient.on('update', () => { - if ( - Platform.OS === 'ios' && - NativeRedBox != null && - NativeRedBox.dismiss != null - ) { - NativeRedBox.dismiss(); - } else { - const NativeExceptionsManager = require('../Core/NativeExceptionsManager') - .default; - NativeExceptionsManager && - NativeExceptionsManager.dismissRedbox && - NativeExceptionsManager.dismissRedbox(); + if (hmrClient.shouldApplyUpdates) { + if ( + Platform.OS === 'ios' && + NativeRedBox != null && + NativeRedBox.dismiss != null + ) { + NativeRedBox.dismiss(); + } else { + const NativeExceptionsManager = require('../Core/NativeExceptionsManager') + .default; + NativeExceptionsManager && + NativeExceptionsManager.dismissRedbox && + NativeExceptionsManager.dismissRedbox(); + } } }); hmrClient.on('update-done', () => { - if (enableLoadingView) { + if (hmrClient.shouldApplyUpdates && enableLoadingView) { HMRLoadingView.hide(); } }); @@ -108,12 +171,12 @@ Error: ${e.message}`; if (data.type === 'GraphNotFoundError') { hmrClient.disable(); - throw new Error( + setHMRUnavailableReason( 'The packager server has restarted since the last Hot update. Hot Reloading will be disabled until you reload the application.', ); } else if (data.type === 'RevisionNotFoundError') { hmrClient.disable(); - throw new Error( + setHMRUnavailableReason( 'The packager server and the client are out of sync. Hot Reloading will be disabled until you reload the application.', ); } else { @@ -123,13 +186,46 @@ Error: ${e.message}`; hmrClient.on('close', data => { HMRLoadingView.hide(); - throw new Error( + setHMRUnavailableReason( 'Disconnected from the packager server. Hot Reloading will be disabled until you reload the application.', ); }); - hmrClient.enable(); + if (isEnabled) { + HMRClient.enable(); + } else { + HMRClient.disable(); + } }, }; +function setHMRUnavailableReason(reason) { + invariant(_hmrClient, 'Expected HMRClient.setup() call at startup.'); + + _hmrUnavailableReason = reason; + if (_hmrClient.shouldApplyUpdates) { + // If HMR is currently enabled, show the error right away. + // Otherwise, it will be shown when you try to enable it. + throw new Error(reason); + } +} + +// Returns the filename without the folder path. +// If file is called index.js, it does include the parent folder though. +function getShortModuleName(fullName) { + const BEFORE_SLASH_RE = /^(.*)[\\\/]/; + let shortName = fullName.replace(BEFORE_SLASH_RE, ''); + if (/^index\./.test(shortName)) { + const match = fullName.match(BEFORE_SLASH_RE); + if (match) { + const pathBeforeSlash = match[1]; + if (pathBeforeSlash) { + const folderName = pathBeforeSlash.replace(BEFORE_SLASH_RE, ''); + return folderName + '/' + shortName; + } + } + } + return shortName; +} + module.exports = HMRClient; diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index 42b05dfa678..72a8a7eb94c 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -914,14 +914,16 @@ struct RCTInstanceCallback : public InstanceCallback { } #if RCT_DEV - if (self.devSettings.isHotLoadingAvailable && self.devSettings.isHotLoadingEnabled) { + if (self.devSettings.isHotLoadingAvailable) { NSString *path = [self.bundleURL.path substringFromIndex:1]; // strip initial slash NSString *host = self.bundleURL.host; NSNumber *port = self.bundleURL.port; + BOOL isHotLoadingEnabled = self.devSettings.isHotLoadingEnabled; [self enqueueJSCall:@"HMRClient" - method:@"enable" - args:@[@"ios", path, host, RCTNullIfNil(port)] - completion:NULL]; } + method:@"setup" + args:@[@"ios", path, host, RCTNullIfNil(port), @(isHotLoadingEnabled)] + completion:NULL]; + } #endif } diff --git a/React/Modules/RCTDevSettings.mm b/React/Modules/RCTDevSettings.mm index 3a2652c6e24..ef3cb7df488 100644 --- a/React/Modules/RCTDevSettings.mm +++ b/React/Modules/RCTDevSettings.mm @@ -327,7 +327,22 @@ RCT_EXPORT_METHOD(setHotLoadingEnabled:(BOOL)enabled) { if (self.isHotLoadingEnabled != enabled) { [self _updateSettingWithValue:@(enabled) forKey:kRCTDevSettingHotLoadingEnabled]; - [_bridge reload]; + if (_isJSLoaded) { + #pragma clang diagnostic push + #pragma clang diagnostic ignored "-Wdeprecated-declarations" + if (enabled) { + [_bridge enqueueJSCall:@"HMRClient" + method:@"enable" + args:@[] + completion:NULL]; + } else { + [_bridge enqueueJSCall:@"HMRClient" + method:@"disable" + args:@[] + completion:NULL]; + } + #pragma clang diagnostic pop + } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java index dfc241c5109..60e24714d33 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java @@ -488,15 +488,23 @@ public class DevSupportManagerImpl implements new DevOptionHandler() { @Override public void onOptionSelected() { - if (!mDevSettings.isHotModuleReplacementEnabled() && !mDevSettings.isJSDevModeEnabled()) { - Toast.makeText( - mApplicationContext, - mApplicationContext.getString(R.string.catalyst_hot_reloading_auto_enable), - Toast.LENGTH_LONG).show(); - mDevSettings.setJSDevModeEnabled(true); - } - mDevSettings.setHotModuleReplacementEnabled(!mDevSettings.isHotModuleReplacementEnabled()); - handleReloadJS(); + boolean nextEnabled = !mDevSettings.isHotModuleReplacementEnabled(); + mDevSettings.setHotModuleReplacementEnabled(nextEnabled); + if (mCurrentContext != null) { + if (nextEnabled) { + mCurrentContext.getJSModule(HMRClient.class).enable(); + } else { + mCurrentContext.getJSModule(HMRClient.class).disable(); + } + } + if (nextEnabled && !mDevSettings.isJSDevModeEnabled()) { + Toast.makeText( + mApplicationContext, + mApplicationContext.getString(R.string.catalyst_hot_reloading_auto_enable), + Toast.LENGTH_LONG).show(); + mDevSettings.setJSDevModeEnabled(true); + handleReloadJS(); + } } }); options.put( @@ -692,13 +700,15 @@ public class DevSupportManagerImpl implements mDebugOverlayController = new DebugOverlayController(reactContext); } - if (mDevSettings.isHotModuleReplacementEnabled() && mCurrentContext != null) { + if (mCurrentContext != null) { try { URL sourceUrl = new URL(getSourceUrl()); String path = sourceUrl.getPath().substring(1); // strip initial slash in path String host = sourceUrl.getHost(); int port = sourceUrl.getPort(); - mCurrentContext.getJSModule(HMRClient.class).enable("android", path, host, port); + mCurrentContext + .getJSModule(HMRClient.class) + .setup("android", path, host, port, mDevSettings.isHotModuleReplacementEnabled()); } catch (MalformedURLException e) { showNewJavaError(e.getMessage(), e); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/HMRClient.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/HMRClient.java index 56d838124b6..c221dbee89c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/HMRClient.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/HMRClient.java @@ -25,6 +25,18 @@ public interface HMRClient extends JavaScriptModule { * @param bundleEntry The path to the bundle entry file (e.g. index.ios.bundle). * @param host The host that the HMRClient should communicate with. * @param port The port that the HMRClient should communicate with on the host. + * @param isEnabled Whether HMR is enabled initially. */ - void enable(String platform, String bundleEntry, String host, int port); + void setup(String platform, String bundleEntry, String host, int port, boolean isEnabled); + + /** + * Sets up a connection to the packager when called the first time. + * Ensures code updates received from the packager are applied. + */ + void enable(); + + /** + * Turns off the HMR client so it doesn't process updates from the packager. + */ + void disable(); }