Skip to content

Commit

Permalink
Don't minify JS bundle by default when using hermes (#30496)
Browse files Browse the repository at this point in the history
Summary:
Minification is not needed for hermes as it does all required optimisations on the bytecode. This is what facebook does internally for hermes bundles and I also validated by comparing the bytecode bundle size on a minified and non-minified bundle.

## Changelog

[General] [Changed] - Don't minify JS bundle by default when using hermes

Pull Request resolved: #30496

Test Plan: Verified that the JS bundled generated on Android and iOS when using hermes is not minified by checking the generated JS file manually.

Reviewed By: rickhanlonii

Differential Revision: D25235195

Pulled By: cpojer

fbshipit-source-id: ad2131aab4dfd17ab53b6a5720ed0e2f1b09cca4
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Dec 2, 2020
1 parent d435d26 commit 1a67dda
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
17 changes: 13 additions & 4 deletions react.gradle
Expand Up @@ -159,12 +159,21 @@ afterEvaluate {
def devEnabled = !(config."devDisabledIn${targetName}"
|| targetName.toLowerCase().contains("release"))

def extraArgs = config.extraPackagerArgs ?: [];
def extraArgs = []

if (bundleConfig) {
extraArgs = extraArgs.clone()
extraArgs.add("--config");
extraArgs.add(bundleConfig);
extraArgs.add("--config")
extraArgs.add(bundleConfig)
}

// Hermes doesn't require JS minification.
if (enableHermes && !devEnabled) {

This comment has been minimized.

Copy link
@heroic

heroic Dec 3, 2020

@janicduplessis why do we need to check for devEnabled for Hermes?

This comment has been minimized.

Copy link
@janicduplessis

janicduplessis Dec 3, 2020

Author Contributor

It won’t minify if devEnabled is true so there is no need to add the flag

extraArgs.add("--minify")
extraArgs.add("false")
}

if (config.extraPackagerArgs) {
extraArgs.addAll(config.extraPackagerArgs)
}

commandLine(*execCommand, bundleCommand, "--platform", "android", "--dev", "${devEnabled}",
Expand Down
7 changes: 6 additions & 1 deletion scripts/react-native-xcode.sh
Expand Up @@ -163,6 +163,11 @@ if [[ $EMIT_SOURCEMAP == true ]]; then
EXTRA_ARGS="$EXTRA_ARGS --sourcemap-output $PACKAGER_SOURCEMAP_FILE"
fi

# Hermes doesn't require JS minification.
if [[ $USE_HERMES == true && $DEV == false ]]; then
EXTRA_ARGS="$EXTRA_ARGS --minify false"
fi

"$NODE_BINARY" $NODE_ARGS "$CLI_PATH" $BUNDLE_COMMAND \
$CONFIG_ARG \
--entry-file "$ENTRY_FILE" \
Expand All @@ -187,7 +192,7 @@ else
if [[ $EMIT_SOURCEMAP == true ]]; then
EXTRA_COMPILER_ARGS="$EXTRA_COMPILER_ARGS -output-source-map"
fi
"$HERMES_CLI_PATH" -emit-binary $EXTRA_COMPILER_ARGS -out "$DEST/main.jsbundle" "$BUNDLE_FILE"
"$HERMES_CLI_PATH" -emit-binary $EXTRA_COMPILER_ARGS -out "$DEST/main.jsbundle" "$BUNDLE_FILE"
if [[ $EMIT_SOURCEMAP == true ]]; then
HBC_SOURCEMAP_FILE="$BUNDLE_FILE.map"
"$NODE_BINARY" "$COMPOSE_SOURCEMAP_PATH" "$PACKAGER_SOURCEMAP_FILE" "$HBC_SOURCEMAP_FILE" -o "$SOURCEMAP_FILE"
Expand Down

0 comments on commit 1a67dda

Please sign in to comment.