Prevent removing ReactNativeDevBundle in LiveReload #1332
Conversation
| public void clearDebugCacheIfNeeded(ReactInstanceManager instanceManager) { | ||
| boolean isLiveReloadEnabled = false; | ||
|
|
||
| if(instanceManager != null) { |
|
|
||
| if(instanceManager != null) { | ||
| DevSupportManager devSupportManager = instanceManager.getDevSupportManager(); | ||
| if(devSupportManager != null) { |
| @@ -118,7 +118,12 @@ private void setJSBundle(ReactInstanceManager instanceManager, String latestJSBu | |||
|
|
|||
| private void loadBundle() { | |||
| clearLifecycleEventListener(); | |||
| mCodePush.clearDebugCacheIfNeeded(); | |||
| try { | |||
| mCodePush.clearDebugCacheIfNeeded(resolveInstanceManager()); | |||
There was a problem hiding this comment.
Can we reuse final ReactInstanceManager instanceManager = resolveInstanceManager(); variable from line 125 (from below) in clearDebugCacheIfNeeded to avoid unnecessary using reflection twice and additional try-catch block?
There was a problem hiding this comment.
Yeah, we can, but we have some cases when resolveInstanceManager() has errors. In this case we should use clearDebugCacheIfNeeded with null argument. I think that adding this logic into next try-catch block is unnecessary as it make it more difficult for understanding.
Could you please share your thoughts about it?
There was a problem hiding this comment.
No worries, I believe current solution fits well. The only thing that I'd probably add is a comment with e.g. reference to the issue or some explanation of the intent because it could be not evident why do we need this logic at all.
There was a problem hiding this comment.
Yeah, good suggestion. Thanks. I added comments.
alexandergoncharov-zz
deleted the
Goncharov/fix_issue_with_update_during_liveReload
branch
Fix for #1272
Prevent removing
ReactNativeDevBundle.jsfile in case if LiveReload mode is enabled.In other cases all behaviour is same.