RFC: Multi-bundle support (v2)

[zamotany]

Introduction

This RFC is a 2nd attempt finding the best way to add support for multiple bundles to the React Native. The original discussion can be found here: #127

The Core of It

Why we need multi-bundle support?

The main reason why want to introduce multi-bundle support, is that there are some cases in which the single-bundle approach, even with Hermes bytecode bundles won't work. For example, in apps where some logic has to be loaded dynamically or where the bundle has to be fetched from the remote location. A concrete example would be a word processing application, when based on some criteria a bundle with relevant logic, analytics, AI would be downloaded and used to provide better experience when the app detects that the content the user is writing matches those criteria.

Another example, can be preloading and warming up the environment ahead of time - in case of interconnected application like a application with a dashboard, which also includes shortcut to a messaging app, upon receiving a new message, we can assume that the user will switch to the messaging app, so we could be able to spun up the app in the background and load a bundle with common dependencies to improve TTR/TTI.

But what about Hermes and bytecode bundles? As great as Hermes is, it's not always feasible or straight up possible to use. As of right now, Hermes is not available for iOS and it might not be used in some out of tree platforms.

Required changes

Based on discussion in #127, in this proposal, we'll try to minimise the amount of the code that has to be touched in the React Native core and off-load as much of the logic as possible to a 3rd party native module - this means that the only changes required will be on the native side, no JavaScript API will be modified or added. Also the refactoring part (handling bundle types using polymorphism) will not be included here.

Essentially, what we need, is to be able to call loadScriptFromAssets/loadScriptFromFile and loadRAMBundleFromString/loadRAMBundleFromFile multiple times. This means that the initialisation part in the JSExecutor::loadApplicationScript (for instance in ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp) will need to be either done is a separate function, that will be called only once or executed in a conditional block based on a boolean flag like isInitialised.

The function registerSegment could be then removed in favour of loadScriptFromX functions.

Discussion points

The main question is, if the proposed changes are something that FB/React Native team would agree to merge and is there anything that needs to be tweaked/improved.

CC: @matthargett @joeblynch @TheSavior @cpojer @acoates-ms

Detailed plan

1. Instance from CxxReact exposes loadScriptFromString method - it would create an instance of Bundle using Bundle::fromString and call loadBundle.
2. Instance from CxxReact exposes loadScriptFromFile method - it would create an instance of Bundle using Bundle::fromFile and call loadBundle.
3. CatalystInstance from ReactAndroid exposes jniLoadScriptFromAssets method - it would create an instance of Bundle using android::Bundle::fromAssets and call loadBundle.
4. Instance exposes loadBundle(bundle, bundleId, loadSynchronously), which would store bundle in RAMBundleRegistry if it's a RAM bundle, call loadApplication or loadApplicationSync for initial bundle; for additional bundles it would call initializeBundle on nativeToJsBridge
5. initializeBundle on nativeToJsBridge calls initializeBundle on JSExecutor, which should take startupScript from the bundle and evaluate it (RAM bundle is already stored in RAMBundleRegistry in Instance::loadBundle for nativeRequire).
6. RAMBundleRegistry will be stored in Instance and passed down to JSExecutor as shared pointer.
7. Bundle is a abstract class for StringBundle, IndexedRAMBundle and FileRAMBundle.
8. All RAM bundles are stored in RAMBundleRegistry (it's always a multibundle registry)
9. setSourceURL should be renamed to loadScriptFromRemoteDebugger (optional)
10. Bundles are identified with integers. When calling loadScriptFromString/loadScriptFromFile/jniLoadScriptFromAssets the last argument is a bundleId with default value 0 - 0 for initial bundle.
11. When loading additional bundles Instance::loadBundle raises error if trying to overwrite already existing bundle.

Files that would be touched:

• RAMBundleRegistry (h, cpp)
• Instance (h, cpp)
• CatalystInstance (h, cpp, java)
• NativeToJsBridge (h, cpp)
• JSExecutor (h, cpp)
• JSIExecutor (h, cpp)
• ProxyExecutor (h, cpp)
• RCTCxxBridge
• Bundle, StringBundle, IndexedRAMBundle, FileRAMBundle (h, cpp, added)

We plan to open 2 PRs together:

1. Adds new files necessary for 2.
2. Modifies already existing code in RN to use files from 1.

[brentvatne]

also cc @ide

[ide]

The loadScript approach sounds directionally correct to me. One question for the Facebook team that comes to mind is whether to deprecate RAM bundles and focus just on loading strings for simplicity.

[elicwhite]

cc @acoates-ms based on the conversation from RN.EU (ah, already in OP’s cc list).

Thanks for trimming this down based on our conversation in Poland. Im on vacation this week but will take a look when I get back. Hopefully some of the people you cc’d have thoughts. Especially Microsoft on whether this is the same change they needed to make to make this possible.

[zcgit]

So we can use require("androidFilesPath/X.js") or require("iOSDocumentsPath/X.js") if the loadScript function is done?

[acoates-ms]

So I was looking through our code, and it doesn't actually look like we've made this change currently. We are calling loadApplicationScript multiple times as is. It looks like the logic in that function is setting up some globals etc in a relatively stateless way, so I'm curious what issues you hit with calling it multiple times.

Having said that, it shouldn't redo that work when called multiple times, so we agree the with the proposed change.

[acoates-ms]

@zcgit those require statements would still be evaluated at bundle time. The change to core being done wont provide out of the box bundle splitting / loading for RN. But it makes changes to core that allow libraries on top to potentially provide that capability.

[zamotany]

@acoates-ms The idea to make this globals initialisation a one-time logic was added here to (like you said) prevent it from redoing the work and prevent any potential edge-cases, that we haven't spotted yet.

[zamotany]

@TheSavior @cpojer
Does the proposed changes look good from the Facebook standpoint, are we good to go with making the PR?

[elicwhite]

@zamotany, you and I followed up over DM a couple of days ago but forgot to follow up here as well. It seems like if this is really a small change and Microsoft agrees with that change, then going to a PR seems like a reasonable next step for us to take a closer look. 👍

[zamotany]

Updated the description with Detailed plan
cc: @TheSavior

[ejanzer]

@zamotany

Thanks for all the work you’ve put into this! I’ve taken a look and I think the changes to RN core make sense to me, and I think we're good to go with making the PR(s) - I just have a couple questions/suggestions:

Questions:

• I might be missing some context from prior discussions here, but what are the specific things that are missing from RN core that are needed for your use case? It seems like most of what you need to do can be accomplished today using registerSegment, with the exception of being able to load a segment from a string instead of a file - it seems like the rest of the changes are just cleaning up / unifying the APIs we use to load JS bundles. Is that right? (If so, that's great! I’m just trying to make sure I understand what the missing pieces are.)
• If the main thing that’s missing is being able to load a segment from a string instead of a file, what’s the reason for wanting to do that? Is there a reason why it’s undesirable to save it to a file first?

Again, I don't mean to suggest that we shouldn't do this - just trying to understand the motivations.

Suggestions

I only really have one change I’d propose, for #4 in your proposal:

4. Instance exposes loadBundle(bundle, bundleId, loadSynchronously), which would store bundle in RAMBundleRegistry if it's a RAM bundle, call loadApplication or loadApplicationSync for initial bundle; for additional bundles it would call initializeBundle on nativeToJsBridge

In this proposal, both loadApplication and initializeBundle would call evaluateJavaScript for a given JS bundle - the only difference is, one is called on the initial bundle (bundleId 0) and installs a bunch of global variables in the JS runtime, and one is called on each subsequent bundle (and doesn’t do that). It seems to be that a cleaner way to organize this is to instead pull out all of the logic that should be done once per JS VM instance (installing globals) into a separate function, and then every JS bundle gets loaded by the same loadBundle function. So we’d have something like:

• Instance::initializeBridge calls JSIExecutor::initializeRuntime (or something like that), which installs global variables (currently done in JSIExecutor::loadApplicationScript)
• Instance::loadBundle calls JSIExecutor::loadBundle, which calls evaluateJavaScript

What do you think?

Splitting it up

It’d be great to split this up even further, if possible - obviously you’re more familiar with the code, so you’ll know better if this is feasible, but here’s how I'd go about splitting this up into separate commits/PRs:

1. Make the changes I proposed above: split up loadApplicationScript into two functions, one to install globals and one to call evaluateJavaScript. Modify existing code in Instance and NativeToJsBridge to call those functions in the right places (you’ll probably also have to modify JSExecutor and ProxyExecutor to match)
2. Add Bundle and StringBundle, implement Bundle::fromString, and modify loadScriptFromString to use them
3. Switch to using a shared_ptr for RAMBundleRegistry everywhere, and modify Instance to store it
4. Add FileRAMBundle, implement Bundle::fromFile, and modify loadScriptFromFile to use it (and also IndexedRAMBundle? It’s not clear to me where that one will be used)
5. Add android::Bundle::fromAssets and use that in CatalystInstance::jniLoadScriptFromAssets
6. Add the bundleId param and remove registerSegment (or modify it to call into new code, if that’s preferable?)
7. Rename setSourceURL (unrelated to other changes)

Testing

I know most of these classes probably don’t have any tests right now :( but if you’re up for it it would be really awesome to add them here - it would help make the PRs easier to review (so we can see how each of these is intended to be used) and it would also make it harder for us to break in the future (which is especially important for APIs that we don’t use at FB).

[zamotany]

@ejanzer Thanks for the feedback 👍

Like you said, one of the differences between proposed changes and what's currently in RN core is the ability to load from string, which itself is not a main motivation. We would like to be able to mix multiple bundle types together, for example it's currently not possible to have initial bundle as plan JS and additional as RAM bundles. Moreover we want to improve the API for bundle handling and make the API consumers concerned only about where to load the bundle from, instead of what the bundle actually is.

As for the rest, I really like the suggestion you've proposed and we'll incorporate it. The reason why we haven't proposed it, is that we wanted to keep the amount of changes smaller. Given the much better way of splitting up the work you've proposed, I assume it won't be much of an issue now.

We'll try to add tests for the untested code. If you have any suggestions how to test it, what's the best approach, I'll be more than happy to hear it.

[zamotany]

@ejanzer
While making the changes, we realised that we can get desired behaviour and features with less changes than originally planned and just gone with necessary changes. This also means that we had to modify the splitting the work, however we think this still shouldn't be a problem. You can find the split PRs with changes here (in order):

1. https://github.com/callstack/react-native/pull/22/files
2. https://github.com/callstack/react-native/pull/24/files
3. [Feat][Multibundling] - add bundleId callstack/react-native#25

We would greatly appreciate your feedback on them.

Another think I've realised and wanted to discuss is that with current setup, we're not supporting a use case we have, which is to synchronously call native side from JS, to load required bundle, before continuing evolution of the current bundle. In out case we have a host bundle, which depends on base bundle. The base bundle contains dependencies like react-native, which host uses. So in the host bundle need to call a native function to synchronously load and evaluate base bundle.

We figured out there are 2 viable ways to achieve that:

1. Our native module would use ReactCommon/jsi and JNI on Android to export function in JSContext, which means we would need to use NDK build pipeline.
2. Allow nativeRequire to be called with single argument - bundleId and allow to provide interceptor for nativeRequrie calls, so that we could detect when nativeRequire was called with bundleId only and load + evaluate bundle based on received bundleId, otherwise use regular nativeRequire logic. This would mean nativeRequire would need to be always set on the global JS context.

WDYT? Maybe you have some other idea? We would like to avoid having to deal with NDK in native module that will be open source, due to greater maintenance cost.

[ejanzer]

@zamotany

So am I understanding correctly that you only need the first 2 PRs to support your use case? If that's true, then I don't think there's any reason to go ahead with the third one - splitting up and renaming loadApplicationScript was just a suggestion for to reorganize things if you needed to call loadApplicationScript multiple times, but if you're using registerBundle instead then I don't think we need those changes.

I believe it should be possible to make a native module method synchronous today: (see: https://medium.com/@some_day_man/synchronous-returns-in-react-native-native-modules-453af33d5999). What you described in 1 is similar to TurboModules, which is coming soon, but not available yet.

I'm not as familiar with nativeRequire - when is it not set? You're talking about overriding it in JS, right?

[zamotany]

@ejanzer Well, the problem is I cannot use NativeModules directly, by the time I need to load base bundle, the bridge from RN is not yet initialized, only global properties like nativeModuleProxy or __fbBatchedBridge. Basically, I need to call a native code without RN's infrastructure (like bridge, native modules etc).

Right now nativeRequire is only set when the initial bundle is a RAM bundle, without out changes it's set when RAM bundle is loaded (can be initial or not).

So to better illustrate the problem:

1. JS context is created, global JS properties set
2. Initial bundle is loaded, in boostart (startup) code it call loadBundle('base') (or sth similar)
3. loadBundle (global property created in native side, created similar to https://github.com/callstack/react-native/blob/master/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp#L85-L103) executes native code to load base bundle and evaluate it (all of this needs to be synchronous)
4. The base is evaluated, the normal execution in initial bundle continues.
5. RN JS code is evaluated, InitializeCore.js is called etc

[fredikey]

Thanks for all the work you’ve put into this! It's Awesome! So, I researched a lot about "multi-bundle support" in the last time and I didn't find any solution for this. In my case, I need to do "Super App" which contains mini-apps, bundles of mini-apps will store on the server-side. I thought about rendering mini-apps in WebView but I like native solution more
Is it possible to create an experimental version of the "Super App" using your fork https://github.com/callstack/react-native/tree/feat/apennine-0.60?

[zamotany]

@fredikey Sorry for late response. Yes, you can use that together with Haul. However, I would suggest to do it only for prototyping, since it's using old version and is not really maintained.

[simka]

First PR is here, the other two are on their way.

[simka]

And here are the second and the last, third PR.

[dlebedynskyi]

@simka Is there any progress on PRs? Multi-bundle support looks like a great feature.

[enahum]

Would this approach work for something like plugins ? Say you deploy you main application that has a plugin architecture and then plugins can be downloaded from a remote server and "installed" into the app as a bundle ? that would be incredible for a use case we have at Mattermost.

[zamotany]

@enahum As long as the bundles will be run under a single app that should work. Alternatively you could run multiple React instances with some native code as a brownfield app.

[grabbou]

@enahum, yes, this would be possible. In fact, we have already prototyped a similar use-case for one of our existing clients. Feel free to reach out in a DM/email to follow up.

[enahum]

@grabbou thanks.. I'll contact you by email within the rest of the week, I'm a bit caught up with something else

[rorofino]

@grabbou I will also contact you by email, it sounds really interesting.

[usamaehsan]

@grabbou can you please create a medium post about it? So that we can all read it

[kiennguyentiki]

First of all, thanks @zamotany and Callstack for making the proposal for loading multi bundle in React Native.

We also faced the same challenge when we develop a solution to load multiple "mini-apps" in the same hosted app.
In our user cases, we have a hosted app, this hosted app is written in native (Objective C/Java), and have some features already written in React Native. The navigation between Native part and RN part is very complex.

Let ' say, we have these screens:

• On the home tab is 4 tab: Home, Profile, and Catalogs is written in native. Feeds is written in RN
• Product Detail tab is written in Native
• Seller tab is written in RN
  Any Native screens could go to any RN screens and vise versa.
  E.g:
  Home (Native) -> Product Detail (Native) -> Feeds (RN) -> Seller (RN) -> Product Detail (Native) -> Seller (RN)

Each RN screens have a lot of features inside (e.g: Seller tab shows all products of sellers, their categories, their videos, their feeds, ...). So it is fair to call they are RN app instead of RN screens.
Each RN app is implemented by different teams. These teams bundle their app in a different bundle. Because of the complex navigation between Native and RN, every time we go to a new RN app, we start a new RCTBridge, and load a bundle for the coresponding app to the new bridge. This approach makes TTI of RN app is high, and increase memory consume because we have many bridges.

So, we wanted to take another approach to solve this issue, and the multi bundle is the right answer for us.
NOTE that we are using React Native 0.61.5
Our solution is like that:

• We use haul to build a locked dll to contains all common libraries we shared between our RN apps. These are ReactNative, React, some common UI libraries
• For each app, we generate the new bundle which does not contain code from dll any more. So the code for a RN app is now very small (less than 500KB each app)
• We write native modules to manage the bundle for each app. When our hosted app is open, the hosted app will start the RCTBridge to load the dll bundle
• Every time we want to open a new app, we will check does the bundle for that app is loaded or not. If it is not loaded, we loaded it and create a new RCTRootView which register for the app and its bundle

The interesting part is how we load a new bundle?
We figure our 2 ways to do that

• At the beginning we use the API for RCTBridge registerSegmentWithId:path

This API is worked for both Android and iOS. Although this API is marked as an experiment, we still try to use it, and it turns out it works quite well.
But this API has some minor side effect, event after the RN app is unloaded, the bundle still there.
And all RN apps share the same JSContext. This side effect helps us to figure the second solutions

• With the above issue, we want each RN app loaded only its bundle, and does not load the dll bundle. And each RN app runs in a separated JSContext, so that each RN is isolated from each other.
  Our idea is based on the fact that inside one JSVirtualMachine, different JSContext could share JSValue.
  When our app start, RN load the dll bundle, and its create a new JSContext. Let 's call it is dll context.

So when we load a new RN app, we will create a new JSContext, and copy some global variables from dll context to new JSContext. Let's call the new JSContext is app context. Inside the app context, we will load the app bundle.
Since app context share global variables with dll context, app context could access all React Native functionality.
When the app is destroyed, we release the JSContext.

This approach is very easy to implement, and we could implement it without modifying RN Core
Here is how we do it in iOS

- (JSGlobalContextRef)cloneJSContext:(JSGlobalContextRef)ctx name:(NSString *)name {
    // creat a new context
    JSContextGroupRef vm = JSContextGetGroup(ctx);
    JSGlobalContextRef newCtx = JSGlobalContextCreateInGroup(vm, nullptr);
    JSGlobalContextSetName(newCtx, JSStringCreateWithUTF8CString([name UTF8String]));
    
    // copy global object
    JSObjectRef global = JSContextGetGlobalObject(ctx);
    JSObjectRef newGlobal = JSContextGetGlobalObject(newCtx);
    JSPropertyNameArrayRef names = JSObjectCopyPropertyNames(ctx, global);
    for (size_t i = 0; i < JSPropertyNameArrayGetCount(names); i++) {
        JSStringRef name = JSPropertyNameArrayGetNameAtIndex(names, i);
        JSValueRef value = JSObjectGetProperty(ctx, global, name, nil);
        JSObjectSetProperty(newCtx, newGlobal, name, value, kJSPropertyAttributeNone, nil);
    }
    JSPropertyNameArrayRelease(names);
    return newCtx;
}

- (void)evaluateJavascriptWithContent:(NSString *)content sourceName:(NSString *)name {
    JSStringRef sourceRef = JSStringCreateWithUTF8CString([content UTF8String]);
    JSStringRef sourceURLRef = JSStringCreateWithUTF8CString([name UTF8String]);
    JSEvaluateScript(_ctx, sourceRef, nil, sourceURLRef, 0, nil);
    JSStringRelease(sourceRef);
    if (sourceURLRef) {
      JSStringRelease(sourceURLRef);
    }
}

- (void)evalulateJavascriptWithPath:(NSString *)path sourceName:(NSString *)name {
    NSString* content = [NSString stringWithContentsOfFile:path
                                                  encoding:NSUTF8StringEncoding
                                                     error:NULL];
    [self evaluateJavascriptWithContent:content sourceName:name];
}

For JavascriptCore and V8 runtime, we figure out the solution to do it. But for Hermes, we still looking digging into Hermes to figure out how to do it. So if you guys know how to do it, please let me know.

Thanks