[HMR] Improve Hot Loading gating

[martinbigio]

Hot Loading is currently disabled by default until we're confident enough it's good to go. On iOS, users have to enable it by tweaking the method hotLoadingEnabled on RCTDevMenu.m. @satya164 might know about how to enable this on Android.

Ideally the feature should be enabled way easier (i.e.: by implementing a method on AppDelegate.m for instance). I don't know much about Android to make a proposal.

[facebook-github-bot]

Hey martinbigio, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

• If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
• If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
• We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

[satya164]

I'll look into how to do this on Android.

[martinbigio]

@javache any pointer on how to enable this on AppDelegate given the current gating code on RCTDevMenu/RCTBridgeDelegate? :)

[javache]

Our default app delegate template (https://github.com/facebook/react-native/blob/master/local-cli/generator-ios/templates/app/AppDelegate.m) doesn't really make use of the bridge's delegate methods. We could change this to create a RCTBridge first, but you'd lose the advantage of the simple RCTRootView API.

Anyone who has a slightly more complicated setup should be able to simply element the new delegate method.

[satya164]

I was thinking, on Android, we can have the following to enable experimental features,

protected List<Integer> getEnabledExperiments() {
    return Array<Integer>.asList(
        Experiments.HOT_MODULE_REPLACEMENT,
        Experiments.SOME_OTHER_CRAZY_EXPERIMENT);
}

Or simpler, call Experiments.setEnabledExperiments anywhere,

Experiments.setEnabledExperiments(
    Array<Integer>.asList(
        Experiments.HOT_MODULE_REPLACEMENT,
        Experiments.SOME_OTHER_CRAZY_EXPERIMENT));

Then in the code which implements the experiment, we can do the following,

if (Experiments.isExperimentEnabled(Experiments.HOT_MODULE_REPLACEMENT)) {
    // Do stuff
}

What do you think about the approach before I implement it?

[satya164]

cc @mkonicek

[satya164]

So another approach would be to allow people enable experiments from dev settings UI as @mkonicek suggested.

[satya164]

I'm not sure about having it in the UI, because it's very easy to find and enable, and then we can get bug reports even if it's not ready. Adding it only in code means that only people who know what they are doing enable experimental things, and we get better quality bug reports.

[martinbigio]

Yeah, that makes sense

[mkonicek]

@martinbigio When I start the packager in a new RN app I see it listens on "/hot" by default. Is that intentional?

[satya164]

@mkonicek It'll only work when the url has hot=true. By default it's hot=false in Android. In iOS it seems to be turned on by default.

[mkonicek]

Cool, that makes sense. Don't think it should be enabled by default on iOS though, should it?

[satya164]

@mkonicek Yes, it shouldn't. @martinbigio can confirm :)

[martinbigio]

The feature is disabled but if a user would modify RCTDevMenu to return YES on hotLoadingAvailable, the feature will not only become visible on the menu but enabled by default. Will fix that today

[JohnyDays]

Hey, is there any guidelines on testing the feature for android? I'd love to give some feedback on it, but can't seem to figure out how to enable it.
People in e018aa3 seem to be asking for it as well. Cheers 👍

[satya164]

@JohnyDays For now you can install from master. There will be an option to enable it in the dev menu.

[jsierles]

Now HMR is enabled in 0.22.0, is this issue still relevant?

[satya164]

No, let's close it.