Repository navigation
Prevent removing ReactNativeDevBundle in LiveReload - #1332
Alexander Goncharov (alexandergoncharov-zz) merged 4 commits into
Conversation
| public void clearDebugCacheIfNeeded(ReactInstanceManager instanceManager) { | ||
| boolean isLiveReloadEnabled = false; | ||
|
|
||
| if(instanceManager != null) { |
There was a problem hiding this comment.
nit: space missing
|
|
||
| if(instanceManager != null) { | ||
| DevSupportManager devSupportManager = instanceManager.getDevSupportManager(); | ||
| if(devSupportManager != null) { |
There was a problem hiding this comment.
nit: space missing
| 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.
2 participants
Fix for #1272
Prevent removing
ReactNativeDevBundle.jsfile in case if LiveReload mode is enabled.In other cases all behaviour is same.