rnx-kit icon indicating copy to clipboard operation
rnx-kit copied to clipboard

feat(cli): add support for "Random Access Module" bundle

Open tido64 opened this issue 2 years ago • 1 comments

Description

Add support for "Random Access Module" bundle.

Test plan

Enable ramBundle for iOS only:

diff --git a/packages/test-app/package.json b/packages/test-app/package.json
index fa7a9e76..994291e3 100644
--- a/packages/test-app/package.json
+++ b/packages/test-app/package.json
@@ -99,7 +99,8 @@
           },
           "ios": {
             "bundleOutput": "dist/main.ios.jsbundle",
-            "sourcemapOutput": "dist/main.ios.jsbundle.map"
+            "sourcemapOutput": "dist/main.ios.jsbundle.map",
+            "ramBundle": true
           },
           "windows": {
             "bundleOutput": "dist/main.windows.bundle",

Bundle all platforms and verify that only the iOS bundle is binary:

% yarn bundle --dev false

% file dist/main.android.bundle
dist/main.windows.bundle: ASCII text, with very long lines (36299)

% file dist/main.ios.jsbundle
dist/main.ios.jsbundle: data

% file dist/main.windows.jsbundle
dist/main.windows.bundle: ASCII text, with very long lines (37843)

Enable ramBundle for all platforms:

diff --git a/packages/test-app/package.json b/packages/test-app/package.json
index fa7a9e76..87e9aba9 100644
--- a/packages/test-app/package.json
+++ b/packages/test-app/package.json
@@ -84,6 +84,7 @@
             "react-is"
           ]
         },
+        "ramBundle": true,
         "typescriptValidation": true,
         "treeShake": false,
         "targets": [

Bundle all platforms and verify that only the Android bundle is still text:

% yarn bundle --dev false

% file dist/main.android.bundle
dist/main.android.bundle: ASCII text, with very long lines (5559)

% file dist/main.ios.jsbundle
dist/main.ios.jsbundle: data

% file dist/main.windows.jsbundle
dist/main.windows.bundle: data

ram-bundle isn't really supported on Android since it's been superseded by Hermes byte-code.

tido64 avatar Aug 25 '22 14:08 tido64

fyi @crazyfraggle

tido64 avatar Aug 25 '22 14:08 tido64

Can you reorganize this to use its own CLI command rnx-ram-bundle so that it matches existing CLI convention? Right now, rnx-bundle and rnx-start both go the great lengths to mirror and be drop-in replacements for bundle and start. If we do RAM bundling, we should have a similar drop-in command for that as well.

Would it make sense to keep this as-is, then add the rnx-ram-bundle? If/when we're getting a third bundle format, e.g. Hermes bytecode, we would have to add a third command if we're following this pattern. It would be a lot simpler if we introduced the bundleFormat option. I'm thinking this is something we can upstream as well.

tido64 avatar Aug 29 '22 08:08 tido64

Would it make sense to keep this as-is, then add the rnx-ram-bundle? If/when we're getting a third bundle format, e.g. Hermes bytecode, we would have to add a third command if we're following this pattern. It would be a lot simpler if we introduced the bundleFormat option. I'm thinking this is something we can upstream as well.

Hermes bundling is currently tied to the native build (iOS configuration, Android variant). In the future, this may evolve to a CLI bundle variant, but there's no indication how they'll model it yet.

For now, keeping parity with the CLI is important for developer adoption, and to keep in sync with code in the CLI project.

A good example of this is mirroring the way the CLI does bundling vs. ram bundling. In cli-plugin-metro, bundleWithOutput accepts a parameter which controls serialization and is the decision point between a CJS bundle or a RAM bundle. Since we have almost identical functions in our CLI, we should mirror what they have to make future alignment easier.

I recently spent a lot of time undoing variations we made in our config/CLI-params, for these reasons. I'm trying to avoid going back in that direction, even if it makes a bit more complexity on our end.

I'll leave specific comments in the PR to better express what I'm talking about in concrete terms.

afoxman avatar Sep 08 '22 17:09 afoxman