Skip to content

Commit

Permalink
De-jank DevLoadingView
Browse files Browse the repository at this point in the history
Summary:
## Problems
Repro steps:
1. Disable Fabric (because CMD + R doesn't work with Fabric right now).
2. Open up Marketplace and hit `CMD + OPT + R`
3. **Observe:** The progress bar doesn't show up right away. It also doesn't actually show progress.
https://pxl.cl/140g1

RN Support post: https://fb.workplace.com/groups/rn.support/permalink/3437652016283389/

## Fixes
The first problem is that progress bar doesn't actually show progress.

**Fix:** Bundle load progress is updated in `RCTCxxBridge`, where we first require `RCTDevLoadingView`, and then call its `updateProgress` method. Previously, we wouldn't  lazily load `RCTDevLoadingView`, it already didn't exist. Lazily loading `RCTDevLoadingView` causes the progress view to show up. Here: https://pxl.cl/140gt

If you look at the above video, you'll notice there are two stages to the progress bar: stage 1 displays the actual progress. Stage 2 prompts that we're downloading the JS bundle. As you can see, stage 1 and stage 2 have different background colors, even though both of them are green.

**Fix:** I adjusted the JS to match the Native color. Here: https://pxl.cl/140gT

We're almost there, but the progress bar is dismissed twice?

**Fix:** I dug into the code, and the reason why was because when we hit `CMD + R`, we invalidate the bridge, and immediately re-initialize it. This means that we asynchronously invalidate the old TurboModuleManager, and immediately create a brand new one. Therefore, two `RCTDevLoadingView` modules can (and do) exist at once. So, I moved `RCTDevLoadingView` to be an instance member of `FBReactModule`, to ensure that it doesn't get cleaned up and re-created when TurboModuleManager is deleted and re-created. This finally fixed the progress bar jank:
https://pxl.cl/140hn

Changelog:
[iOS][Fixed] - Remove RCTDevLoadingView jank

Reviewed By: rickhanlonii

Differential Revision: D20607815

fbshipit-source-id: 05825c67adaf3cfda70be0fa2dc92d413dc8921b
  • Loading branch information
RSNara authored and facebook-github-bot committed Mar 25, 2020
1 parent 421bc5f commit faff19a
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Libraries/Utilities/LoadingView.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import NativeDevLoadingView from './NativeDevLoadingView';
module.exports = {
showMessage(message: string, type: 'load' | 'refresh') {
if (NativeDevLoadingView) {
const green = processColor('#275714');
const green = processColor('#005a00');
const blue = processColor('#2584e8');
const white = processColor('#ffffff');

Expand Down
4 changes: 2 additions & 2 deletions React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ - (void)start
}
onProgress:^(RCTLoadingProgress *progressData) {
#if (RCT_DEV | RCT_ENABLE_LOADING_VIEW) && __has_include(<React/RCTDevLoadingViewProtocol.h>)
// Note: RCTDevLoadingView should have been loaded at this point, so no need to allow lazy loading.
id<RCTDevLoadingViewProtocol> loadingView = [weakSelf moduleForName:@"DevLoadingView" lazilyLoadIfNecessary:NO];
id<RCTDevLoadingViewProtocol> loadingView = [weakSelf moduleForName:@"DevLoadingView"
lazilyLoadIfNecessary:YES];
[loadingView updateProgress:progressData];
#endif
}];
Expand Down

0 comments on commit faff19a

Please sign in to comment.