Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass minify CLI option to metro-serializer-esbuild #2722

Merged
merged 5 commits into from
Oct 10, 2023

Conversation

AAHAbbas
Copy link
Contributor

@AAHAbbas AAHAbbas commented Oct 9, 2023

Description

Running yarn react-native rnx-bundle --dev false --minify false doesn't generate a prod bundle that's not minified. Passing the minify CLI option to the metro-serializer-esbuild so the outputted bundle will be minified or not depending on the passed minify flag.

Test plan

Prepare:

cd packages/test-app
yarn build --dependencies

Generating a prod bundle that's minified:

yarn bundle+esbuild --dev false

Generating a prod bundle that's not minified (this would still generate a minified bundle before):

yarn bundle+esbuild --dev false --minify false

@AAHAbbas AAHAbbas changed the title Passing minify CLI option to metro-serializer-esbuild Pass minify CLI option to metro-serializer-esbuild Oct 9, 2023
Copy link
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this. I think we want to avoid having two minify options inside BundleParameters. See the comment below.

packages/cli/src/bundle/metro.ts Outdated Show resolved Hide resolved
@AAHAbbas AAHAbbas force-pushed the abbas/PassMinifyOption branch from 6843675 to 650ed34 Compare October 10, 2023 07:36
Copy link
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me now. I left a couple of nits.

packages/cli/src/bundle/metro.ts Outdated Show resolved Hide resolved
packages/cli/src/bundle/metro.ts Outdated Show resolved Hide resolved
AAHAbbas and others added 2 commits October 10, 2023 09:58
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
@tido64 tido64 merged commit f1668f4 into microsoft:main Oct 10, 2023
12 checks passed
@tido64
Copy link
Member

tido64 commented Oct 10, 2023

Thanks again 👍

@AAHAbbas AAHAbbas deleted the abbas/PassMinifyOption branch October 10, 2023 08:21
@tido64
Copy link
Member

tido64 commented Oct 10, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants