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

[Packager] Add "dev" option to CLI #112

Closed
wants to merge 1 commit into from
Closed

[Packager] Add "dev" option to CLI #112

wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Feb 28, 2015

Exposes the dev option that is already there to the CLI so that you can turn off invariant checks, etc. I also made it omit the inlined source map when dev=false which made it a lot faster to run on a phone, both due to smaller download size and fewer bytes to copy from Obj-C to JS and evaluate.

Exposes the dev option that is already there to the CLI so that you can turn off invariant checks, etc. I also made it omit the inlined source map when dev=false which made it a lot faster to run on a phone, both due to smaller download size and fewer bytes to copy from Obj-C to JS and evaluate.
@amasad
Copy link
Contributor

amasad commented Mar 2, 2015

I thought we already omit source maps in the production build. Maybe we have an internal script to do that, if that's not the case and we do ship source maps, then that's pretty clowny.

Anyways, thanks, will pull it shortly.

@ide
Copy link
Contributor Author

ide commented Mar 2, 2015

Thanks @amasad. From what I could tell, the code that generates the source string always concatenated the source map so your production build script (maybe it is a person?) might just truncate the end of the file.

I noticed the source map (and file size -- minification helps too) really affect performance on the phone. This is just a rough idea but longer-term it would improve on-device development to have the packager always minify and have the phone be able to load source maps separately. Or if minification compromises debuggability, then it could make sense to expose a option in the package URL to toggle minification so that the simulator can load a non-minified bundle.

@caridy
Copy link

caridy commented Mar 2, 2015

@ide this is somehow related to #116, which we have discussed briefly with @amasad and co last week.

@amasad
Copy link
Contributor

amasad commented Mar 2, 2015

From what I could tell, the code that generates the source string always concatenated the source map so your production build script (maybe it is a person?) might just truncate the end of the file.

You're right, I just checked the script strips out the source map.

I noticed the source map (and file size -- minification helps too) really affect performance on the phone. This is just a rough idea but longer-term it would improve on-device development to have the packager always minify and have the phone be able to load source maps separately. Or if minification compromises debuggability, then it could make sense to expose a option in the package URL to toggle minification so that the simulator can load a non-minified bundle.

Yes, we talked about this. We want to have dev be overridable from the url, so you can toggle it from the device as you're developing. With this patch, this means that they don't get the source maps as well, which is great.

As for minification, with sourcemaps it should be the same debugging experience. That could be an option in the URL. It's worth noting that it could be slow to minify the bundle on each request, especially if we want to preserve sourcemaps.

I wonder how much minification really affect performance, unless we're talking about Closure Compiler -- which does inlining and dead code elimination -- I'm doubtful that there is much change on runtime perf.

@ide
Copy link
Contributor Author

ide commented Mar 3, 2015

I don't have perf numbers but anecdotally the minified JS is faster for a bunch of little reasons that add up. For boot time it helps to read fewer bytes off of disk / from the packager server, and to run the code the app has to copy the string from Obj-C to JS.

@amasad
Copy link
Contributor

amasad commented Mar 3, 2015

closed via caf21ae

@amasad amasad closed this Mar 3, 2015
@ide ide deleted the packager-dev branch March 18, 2015 20:52
vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 13, 2015
Summary:
Exposes the dev option that is already there to the CLI so that you can turn off invariant checks, etc. I also made it omit the inlined source map when dev=false which made it a lot faster to run on a phone, both due to smaller download size and fewer bytes to copy from Obj-C to JS and evaluate.
Closes facebook#112
Github Author: James Ide <[email protected]>

Test Plan:
* ./runJestTests.sh
* test bundle creation with `bundle.sh`
* test `load_dependencies.js` script
* start the server and click around shell app
vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 14, 2015
Summary:
Exposes the dev option that is already there to the CLI so that you can turn off invariant checks, etc. I also made it omit the inlined source map when dev=false which made it a lot faster to run on a phone, both due to smaller download size and fewer bytes to copy from Obj-C to JS and evaluate.
Closes facebook#112
Github Author: James Ide <[email protected]>

Test Plan:
* ./runJestTests.sh
* test bundle creation with `bundle.sh`
* test `load_dependencies.js` script
* start the server and click around shell app
vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 15, 2015
Summary:
Exposes the dev option that is already there to the CLI so that you can turn off invariant checks, etc. I also made it omit the inlined source map when dev=false which made it a lot faster to run on a phone, both due to smaller download size and fewer bytes to copy from Obj-C to JS and evaluate.
Closes facebook#112
Github Author: James Ide <[email protected]>

Test Plan:
* ./runJestTests.sh
* test bundle creation with `bundle.sh`
* test `load_dependencies.js` script
* start the server and click around shell app
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
Exposes the dev option that is already there to the CLI so that you can turn off invariant checks, etc. I also made it omit the inlined source map when dev=false which made it a lot faster to run on a phone, both due to smaller download size and fewer bytes to copy from Obj-C to JS and evaluate.
Closes facebook/react-native#112
Github Author: James Ide <[email protected]>

Test Plan:
* ./runJestTests.sh
* test bundle creation with `bundle.sh`
* test `load_dependencies.js` script
* start the server and click around shell app
acoates-ms added a commit to acoates-ms/react-native that referenced this pull request Jul 25, 2019
jfrolich pushed a commit to jfrolich/react-native that referenced this pull request Apr 22, 2020
…i-fix-pct-margin-padding

Support % for margin
react-one pushed a commit to react-one/react-native that referenced this pull request Sep 24, 2021
Closes facebook#16

When the statusbar is not translucent, the view resizes when the keyboard is shown on Android. The tab bar stays above the keyboard. This PR makes the tab bar hide automatically when the keyboard is shown.

The behaviour is enabled by default and can be disabled with `keyboardHidesTabBar: false` in `tabBarOptions`
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.

3 participants