improve dev mode and HMR interop #24377
Conversation
facebook-github-bot
added
the
CLA Signed
ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java
Outdated
Show resolved
Hide resolved
ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java
Outdated
Show resolved
Hide resolved
| @@ -509,6 +513,10 @@ public void onOptionSelected() { | |||
| new DevOptionHandler() { | |||
| @Override | |||
| public void onOptionSelected() { | |||
| if (!mDevSettings.isHotModuleReplacementEnabled() && !mDevSettings.isJSDevModeEnabled()) { | |||
| Toast.makeText(mApplicationContext, "Dev mode needs to be turned on to enable HMR.", Toast.LENGTH_LONG).show(); | |||
There was a problem hiding this comment.
I think the message isn't sufficiently actionable (as opposed to the one above) because of the return afterwards. Could we either make it enable dev mode or change the message to include "Enable Dev mode first to enable HMR"? I think I'd probably prefer it just enabling dev mode and HMR both, and showing the message you have here. What do you think?
There was a problem hiding this comment.
yes, I was gonna suggest this as well.
There was a problem hiding this comment.
@cpojer please let me know what you think, thanks
There was a problem hiding this comment.
also (that would be something for another PR) - I find that for me, the only reason I go to the "Dev Settings" is to turn off / on the dev mode. I don't do that all too often, but still, perhaps it would make sense to move that option to the dialog, same as with turning on / off the HMR
There was a problem hiding this comment.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request was successfully merged by @vonovak in 3b1760d. When will my fix make it into a release? | Upcoming Releases |
Summary: Motivation is following - I'm sure many people encountered this because it has been like this for a long time. 1 . you're developing something on android, HMR and dev mode is enabled 2 . you go to dev settings, you disable dev mode because you want to see how something behaves 3 . you reload the app because that's what is required for the change to take effect 4 . you wait for the bundle to be compiled and served, and when that is done, you get an error message about HMR not being a registered callable module - because HMR is not available when `__DEV__ === false` (todo screenshot) this fixes the described case by checking if HMR is enabled and dev mode disabled when reloading (step 3) and disables HMR in that case. this also fixes the case when dev mode is disabled and without knowing it, you try to enable HRM (will enable both dev hmr and dev mode). [Android] [Changed] - improve developer experience around Dev mode and HMR interop Pull Request resolved: facebook#24377 Differential Revision: D14890695 Pulled By: cpojer fbshipit-source-id: 95b6ff4131c6d05a32aadd09a9d5ed11f602122c
Summary
Motivation is following - I'm sure many people encountered this because it has been like this for a long time.
1 . you're developing something on android, HMR and dev mode is enabled
2 . you go to dev settings, you disable dev mode because you want to see how something behaves
3 . you reload the app because that's what is required for the change to take effect
4 . you wait for the bundle to be compiled and served, and when that is done, you get an error message about HMR not being a registered callable module - because HMR is not available when
__DEV__ === false(todo screenshot)this fixes the described case by checking if HMR is enabled and dev mode disabled when reloading (step 3) and disables HMR in that case.
this also fixes the case when dev mode is disabled and without knowing it, you try to enable HRM (will enable both dev hmr and dev mode).
Changelog
[Android] [Changed] - improve developer experience around Dev mode and HMR interop
Test Plan
I used the RN tester app and manually verified it works as intended. See gifs where I proceeded as in the steps given above:
current master - you need to be patient to get to the error screen
this branch - HMR gets disabled