add option in 'Dev Settings' for never use cached bundle file #4738

qbig · 2015-12-12T07:33:04Z

facebook-github-bot · 2015-12-12T07:33:26Z

By analyzing the blame information on this pull request, we identified @mkonicek, @astreet and @foghina to be potential reviewers.

mkonicek · 2015-12-14T15:40:15Z

ReactAndroid/src/main/res/devsupport/xml/preferences.xml

+    <CheckBoxPreference
+        android:key="use_cached_bundle"
+        android:title="Use Cached Bundle"
+        android:summary="Use prefetched JS bundle stored in local file."


stored in local file

"stored locally"

The way it's stored might change, making this message out of date.

mkonicek · 2015-12-14T15:43:12Z

I like this, it's really annoying we try to load the bundle; this always fails for OSS apps.

Could we specify this to be true by default for all OSS apps? For example, here: https://github.com/facebook/react-native/blob/master/local-cli/generator-android/templates/package/MainActivity.java#L24

That way people won't have to set this setting manually and fb apps will still have this set to false.

mkonicek · 2015-12-14T18:39:39Z

Alternatively, can we never read from file if the packager is running? I would expect the check here to do that but apparently it doesn't: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java#L291

qbig · 2015-12-15T04:17:03Z

Hi @mkonicek . Sorry but may I ask what is "OSS apps" ?

facebook-github-bot · 2015-12-15T04:54:13Z

@qbig updated the pull request.

qbig · 2015-12-15T04:56:49Z

@ide @mkonicek the current PR achieved 'never load from cache' by setting in "Dev Setting". But even after this setting, the redbox behaviour is still a bit inconsistent as mentioned here #4739.

The update would fix #4739 as well.

qbig · 2015-12-27T12:53:24Z

@skevy could you update the 'needs-revision' flag ?

facebook-github-bot · 2015-12-27T13:01:18Z

@qbig updated the pull request.

satya164 · 2015-12-29T22:51:27Z

Looks like this has conflicts now

facebook-github-bot · 2015-12-30T07:52:16Z

@qbig updated the pull request.

qbig · 2015-12-30T15:42:19Z

@satya164 resolved :)

satya164 · 2015-12-30T21:37:32Z

@qbig OSS -> Open Source Software

facebook-github-bot · 2016-01-05T08:42:23Z

@qbig updated the pull request.

astreet · 2016-01-05T18:12:44Z

I think we want to get rid of the cached bundle completely instead (let's wait to make sure Krzysztof didn't add it for some reason I'm not thinking of though).

In that case, I'd ask that we update this PR to remove that logic instead of adding a checkbox.

astreet · 2016-01-05T18:48:21Z

ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java

@@ -289,7 +289,7 @@ public void onPackagerStatusFetched(final boolean packagerIsRunning) {
                    new Runnable() {
                      @Override
                      public void run() {
-                        if (packagerIsRunning) {
+                        if (packagerIsRunning || !mDevSupportManager.hasUpToDateJSBundleInCache()) {


Why this change? We check it above on L278

@astreet I did this for #4739 so that, when the packager is not running and there is no update-to-date bundle, it would try to load a existent bundle and throw a confusing redbox message( the else part)

maybe I will just not touch it and fix the redbox message in a different PR

facebook-github-bot · 2016-01-09T05:48:17Z

@qbig updated the pull request.

qbig · 2016-01-09T05:48:29Z

@astreet updated accordingly. Would love to hear your thought on #4739

astreet · 2016-01-11T13:19:35Z

ReactAndroid/src/main/java/com/facebook/react/devsupport/DevInternalSettings.java

@@ -72,7 +78,7 @@ public boolean isJSDevModeEnabled() {
  public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key) {
    if (PREFS_FPS_DEBUG_KEY.equals(key) ||
        PREFS_RELOAD_ON_JS_CHANGE_KEY.equals(key) ||
-        PREFS_JS_DEV_MODE_DEBUG_KEY.equals(key)) {
+        PREFS_JS_DEV_MODE_DEBUG_KEY.equals(key) || PREFS_USE_CACHED_BUNDLE.equals(key)) {


nit: can you move this to a new line like all the other checks?

astreet · 2016-01-11T13:19:52Z

Seems reasonable, see nit inline and I'll merge

facebook-github-bot · 2016-01-12T03:48:15Z

@qbig updated the pull request.

qbig · 2016-01-12T03:49:18Z

@astreet fixed

astreet · 2016-01-12T15:23:11Z

Thank you!

astreet · 2016-01-12T15:23:17Z

@facebook-github-bot shipit

facebook-github-bot · 2016-01-12T15:23:34Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/492049884308119/int_phab to review.

qbig · 2016-01-18T02:00:23Z

@astreet Anything I could improve here ?

mkonicek · 2016-01-18T21:41:49Z

There's a compilation error, scroll down in the CircleCI results:
https://circleci.com/gh/facebook/react-native/1287

astreet · 2016-01-19T21:59:23Z

Yay automated testing! Thanks Martin :)

mkonicek · 2016-01-20T17:58:03Z

🏆

mkonicek · 2016-03-20T16:35:19Z

@qbig Can you rebase please and resolve any failing tests?

ghost · 2016-03-25T17:00:22Z

@qbig do you have any updates for this pull request? It's been a while so we wanted to check in and see if you've looked at the requested changes.

mkonicek · 2016-04-13T16:43:53Z

Let's close this since there hasn't been a response from the author. If you want to continue working on this please send a new pull request based on top of master.

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Dec 12, 2015

skevy added Android labels Dec 13, 2015

mkonicek reviewed Dec 14, 2015
View reviewed changes

mkonicek mentioned this pull request Dec 14, 2015

[Workflow][Android] Provide an option to never load JS from cache #2676

Closed

skevy added GH Review: needs-revision and removed GH Review: review-needed labels Dec 15, 2015

qbig force-pushed the option-no-cache branch from beaa9f4 to 310dcbf Compare December 15, 2015 04:53

satya164 added GH Review: review-needed and removed GH Review: needs-revision labels Dec 27, 2015

qbig mentioned this pull request Dec 27, 2015

[Android][Redbox Message] Misleading redbox message when starting app for the first time without packager running. #4739

Closed

satya164 added GH Review: needs-revision and removed GH Review: review-needed labels Dec 29, 2015

qbig force-pushed the option-no-cache branch from f6b8fa0 to d6a4159 Compare December 30, 2015 07:52

astreet reviewed Jan 5, 2016
View reviewed changes

qbig force-pushed the option-no-cache branch from d6a4159 to ff129e1 Compare January 9, 2016 05:47

astreet reviewed Jan 11, 2016
View reviewed changes

add option in 'Dev Settings' for never use cached bundle file

c1fd550

qbig force-pushed the option-no-cache branch from ff129e1 to c1fd550 Compare January 12, 2016 03:48

astreet added GH Review: accepted and removed GH Review: needs-revision labels Jan 13, 2016

astreet added GH Review: needs-revision and removed GH Review: accepted labels Jan 19, 2016

mkonicek added the Triage - In Progress label Mar 20, 2016

mkonicek closed this Apr 13, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add option in 'Dev Settings' for never use cached bundle file #4738

add option in 'Dev Settings' for never use cached bundle file #4738

qbig commented Dec 12, 2015

facebook-github-bot commented Dec 12, 2015

mkonicek Dec 14, 2015

mkonicek commented Dec 14, 2015

mkonicek commented Dec 14, 2015

qbig commented Dec 15, 2015

facebook-github-bot commented Dec 15, 2015

qbig commented Dec 15, 2015

qbig commented Dec 27, 2015

facebook-github-bot commented Dec 27, 2015

satya164 commented Dec 29, 2015

facebook-github-bot commented Dec 30, 2015

qbig commented Dec 30, 2015

satya164 commented Dec 30, 2015

facebook-github-bot commented Jan 5, 2016

astreet commented Jan 5, 2016

astreet Jan 5, 2016

qbig Jan 9, 2016

qbig Jan 9, 2016

facebook-github-bot commented Jan 9, 2016

qbig commented Jan 9, 2016

astreet Jan 11, 2016

astreet commented Jan 11, 2016

facebook-github-bot commented Jan 12, 2016

qbig commented Jan 12, 2016

astreet commented Jan 12, 2016

astreet commented Jan 12, 2016

facebook-github-bot commented Jan 12, 2016

qbig commented Jan 18, 2016

mkonicek commented Jan 18, 2016

astreet commented Jan 19, 2016

mkonicek commented Jan 20, 2016

mkonicek commented Mar 20, 2016

ghost commented Mar 25, 2016

mkonicek commented Apr 13, 2016

add option in 'Dev Settings' for never use cached bundle file #4738

add option in 'Dev Settings' for never use cached bundle file #4738

Conversation

qbig commented Dec 12, 2015

facebook-github-bot commented Dec 12, 2015

mkonicek Dec 14, 2015

Choose a reason for hiding this comment

mkonicek commented Dec 14, 2015

mkonicek commented Dec 14, 2015

qbig commented Dec 15, 2015

facebook-github-bot commented Dec 15, 2015

qbig commented Dec 15, 2015

qbig commented Dec 27, 2015

facebook-github-bot commented Dec 27, 2015

satya164 commented Dec 29, 2015

facebook-github-bot commented Dec 30, 2015

qbig commented Dec 30, 2015

satya164 commented Dec 30, 2015

facebook-github-bot commented Jan 5, 2016

astreet commented Jan 5, 2016

astreet Jan 5, 2016

Choose a reason for hiding this comment

qbig Jan 9, 2016

Choose a reason for hiding this comment

qbig Jan 9, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Jan 9, 2016

qbig commented Jan 9, 2016

astreet Jan 11, 2016

Choose a reason for hiding this comment

astreet commented Jan 11, 2016

facebook-github-bot commented Jan 12, 2016

qbig commented Jan 12, 2016

astreet commented Jan 12, 2016

astreet commented Jan 12, 2016

facebook-github-bot commented Jan 12, 2016

qbig commented Jan 18, 2016

mkonicek commented Jan 18, 2016

astreet commented Jan 19, 2016

mkonicek commented Jan 20, 2016

mkonicek commented Mar 20, 2016

ghost commented Mar 25, 2016

mkonicek commented Apr 13, 2016