Show bundle download progress on iOS by janicduplessis · Pull Request #15066 · react/react-native

janicduplessis · 2017-07-17T23:12:10Z

This shows progress for the download of the JS bundle (different from the packager transform progress that we show already). This is useful especially when loading the JS bundle from a remote source or when developing on device (on simulator + localhost it pretty much just downloads instantly). This will be nice for the expo client since all bundles are loaded over the network and can take several seconds to load.

This depends on react/metro#28 to work but won't crash or anything without it, it just won't show the progress percentage.

Test plan
Tested that bundle download progress is shown properly in RNTester on both localhost + simulator and on real device with network conditionner to simulate a slow loading bundle.

Tested that it doesn't cause issues if the packager doesn't send the Content-Length header.

facebook-github-bot · 2017-07-19T00:03:36Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

shergin · 2017-07-20T01:20:41Z

+ RCTLoadingProgress *progress = [RCTLoadingProgress new];
+ progress.status = @"Downloading JavaScript bundle";
+ // Progress values are in bytes transform them to kilobytes for smaller numbers.
+ progress.done = done != nil ? @([done integerValue] / 1024) : nil;


Why do we convert to kilobytes here?

Mostly just to make the numbers smaller, we don't really have a way to show units with the current setup so it's no really clear what the numbers are anyway.

Why we don't make them smaller right before displaying them?

This is as close as we get to displaying them, this is also used for showing the packager module transform progress so it just takes a progress number and the total then formats it to something like "10% (10/100)".

facebook-github-bot · 2017-07-25T20:29:40Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

pull-bot · 2017-07-25T21:12:36Z

This PR has been submitted by a core contributor.

Attention: @shergin, @javache

Generated by 🚫 dangerJS

janicduplessis · 2017-08-02T22:34:46Z

@shergin Could you check why this didn't import

facebook-github-bot · 2017-08-03T04:14:20Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

shergin · 2017-08-06T02:11:12Z

@janicduplessis This strange but one of our internal builds failes with this error:

Libraries/FBReactKit/js/react-native-github/React/Base/RCTMultipartDataTask.h:14:81: error: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness]

Do you have any idea? Seems this diff even don't modifies this line.

janicduplessis · 2017-08-10T21:13:09Z

@shergin Fixed the error. I just removed the nullability specifier since we didn't use any in that file to begin with. Weird that the error was surfacing at the wrong place :o

Seems like this is new warning that can be enabled in xcode 9. Didn't update the projects to enable it for now but we might want to do that in the future when xcode 9 is released and update the projects to the new defaults.

facebook-github-bot · 2017-08-10T22:36:14Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2017-08-11T00:20:53Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

shergin

This awesome PR breaks RCTMultipartStreamReaderTests, unfortunately.

shergin · 2017-08-11T22:01:55Z

+ }
+}
+
+- (BOOL)readAllParts:(RCTMultipartCallback)callback progressCallback:(RCTMultipartProgressCallback)progressCallback


If we changed the signature of this method, may be we can name it a bit better including callback word into this? Something like readAllPartsWithCallback:progressCallback: or something even better?

Maybe "CompletionCallback" so it's clear how it's different from the progress callback?

Oups forgot to update the signature in tests, also updated it to readAllPartsWithCompletionCallback:progressCallback:. Should land this time!

facebook-github-bot · 2017-08-14T16:15:58Z

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: This shows progress for the download of the JS bundle (different from the packager transform progress that we show already). This is useful especially when loading the JS bundle from a remote source or when developing on device (on simulator + localhost it pretty much just downloads instantly). This will be nice for the expo client since all bundles are loaded over the network and can take several seconds to load. This depends on react/metro#28 to work but won't crash or anything without it, it just won't show the progress percentage. ![img_05070155d2cc-1](https://user-images.githubusercontent.com/2677334/28293828-2c08d974-6b24-11e7-9334-e106ef3326d9.jpeg) **Test plan** Tested that bundle download progress is shown properly in RNTester on both localhost + simulator and on real device with network conditionner to simulate a slow loading bundle. Tested that it doesn't cause issues if the packager doesn't send the Content-Length header. Closes#15066 Differential Revision: D5449073 Pulled By: shergin fbshipit-source-id: 43a8fb559393bbdc04f77916500e21898695bac5

Summary: This shows progress for the download of the JS bundle (different from the packager transform progress that we show already). This is useful especially when loading the JS bundle from a remote source or when developing on device (on simulator + localhost it pretty much just downloads instantly). This will be nice for the expo client since all bundles are loaded over the network and can take several seconds to load. This depends on react/metro#28 to work but won't crash or anything without it, it just won't show the progress percentage. ![img_05070155d2cc-1](https://user-images.githubusercontent.com/2677334/28293828-2c08d974-6b24-11e7-9334-e106ef3326d9.jpeg) **Test plan** Tested that bundle download progress is shown properly in RNTester on both localhost + simulator and on real device with network conditionner to simulate a slow loading bundle. Tested that it doesn't cause issues if the packager doesn't send the Content-Length header. Closesreact#15066 Differential Revision: D5449073 Pulled By: shergin fbshipit-source-id: 43a8fb559393bbdc04f77916500e21898695bac5

Summary: This shows progress for the download of the JS bundle (different from the packager transform progress that we show already). This is useful especially when loading the JS bundle from a remote source or when developing on device (on simulator + localhost it pretty much just downloads instantly). This will be nice for the expo client since all bundles are loaded over the network and can take several seconds to load. This depends on react/metro#28 to work but won't crash or anything without it, it just won't show the progress percentage. ![img_05070155d2cc-1](https://user-images.githubusercontent.com/2677334/28293828-2c08d974-6b24-11e7-9334-e106ef3326d9.jpeg) **Test plan** Tested that bundle download progress is shown properly in RNTester on both localhost + simulator and on real device with network conditionner to simulate a slow loading bundle. Tested that it doesn't cause issues if the packager doesn't send the Content-Length header. Closesreact/react-native#15066 Differential Revision: D5449073 Pulled By: shergin fbshipit-source-id: 43a8fb559393bbdc04f77916500e21898695bac5

Summary: This shows progress for the download of the JS bundle (different from the packager transform progress that we show already). This is useful especially when loading the JS bundle from a remote source or when developing on device (on simulator + localhost it pretty much just downloads instantly). This will be nice for the expo client since all bundles are loaded over the network and can take several seconds to load. This depends on react/metro#28 to work but won't crash or anything without it, it just won't show the progress percentage. ![img_05070155d2cc-1](https://user-images.githubusercontent.com/2677334/28293828-2c08d974-6b24-11e7-9334-e106ef3326d9.jpeg) **Test plan** Tested that bundle download progress is shown properly in RNTester on both localhost + simulator and on real device with network conditionner to simulate a slow loading bundle. Tested that it doesn't cause issues if the packager doesn't send the Content-Length header. Closesreact#15066 Differential Revision: D5449073 Pulled By: shergin fbshipit-source-id: 43a8fb559393bbdc04f77916500e21898695bac5

Summary: Android equivalent of #15066 Tested that download progress shows up properly when reloading the app. [ANDROID] [FEATURE] [DevSupport] - Show bundle download progress on Android Closes#17809 Differential Revision: D6982823 Pulled By: hramos fbshipit-source-id: da01e42b8ebb1c603f4407f6bafd68e0b6b3ecba

Show bundle download progress on iOS
0a0f115

janicduplessis added the Platform: iOS iOS applications. label Jul 17, 2017

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jul 17, 2017

shergin reviewed Jul 22, 2017
View reviewed changes

Merge branch 'master' into bundle-download-progress
16975fd

shergin requested a review from javache as a code owner July 25, 2017 20:29

facebook-github-bot added GH Review: accepted and removed GH Review: review-needed labels Jul 25, 2017

facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jul 26, 2017

Merge branch 'master' into bundle-download-progress
c3be763

Merge branch 'master' into bundle-download-progress
afdc406

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 3, 2017

Fix nullability
a6ee75b

Merge branch 'master' into bundle-download-progress
4fbec2c

Merge branch 'master' into bundle-download-progress
8096502

shergin suggested changes Aug 11, 2017
View reviewed changes

janicduplessis added 2 commits August 14, 2017 04:00

Update signature, fix tests
7d02bb2

Properly update signature
f3d3e47

Merge branch 'master' into bundle-download-progress
dc9f138

facebook-github-bot closed this in ef23d2bAug 14, 2017

janicduplessis mentioned this pull request Feb 1, 2018
Bundle download progress on Android #17809
Closed

Uh oh!

Conversation

janicduplessis commented Jul 17, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jul 19, 2017

Uh oh!

sherginJul 20, 2017

Choose a reason for hiding this comment

Uh oh!

janicduplessisJul 22, 2017

Choose a reason for hiding this comment

Uh oh!

sherginJul 25, 2017

Choose a reason for hiding this comment

Uh oh!

janicduplessisJul 25, 2017

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 25, 2017

Uh oh!

pull-bot commented Jul 25, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janicduplessis commented Aug 2, 2017

Uh oh!

facebook-github-bot commented Aug 3, 2017

Uh oh!

shergin commented Aug 6, 2017

Uh oh!

janicduplessis commented Aug 10, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Aug 10, 2017

Uh oh!

facebook-github-bot commented Aug 11, 2017

Uh oh!

shergin left a comment

Choose a reason for hiding this comment

Uh oh!

sherginAug 11, 2017

Choose a reason for hiding this comment

Uh oh!

ideAug 11, 2017

Choose a reason for hiding this comment

Uh oh!

janicduplessisAug 14, 2017

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

janicduplessis commented Jul 17, 2017•
edited
Loading

pull-bot commented Jul 25, 2017•
edited
Loading

janicduplessis commented Aug 10, 2017•
edited
Loading