Show bundle download progress on iOS #15066

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 facebook/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

React/Base/RCTJavaScriptLoader.mm

+  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

React/Base/RCTMultipartStreamReader.m

+  }
+}
+
+- (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 facebook/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 facebook/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 facebook#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 facebook/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 facebook/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 facebook/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 facebook#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

Summary: Android equivalent of facebook#15066 Tested that download progress shows up properly when reloading the app. [ANDROID] [FEATURE] [DevSupport] - Show bundle download progress on Android Closes facebook#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 ef23d2b Aug 14, 2017

janicduplessis mentioned this pull request Feb 1, 2018

Bundle download progress on Android #17809

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show bundle download progress on iOS #15066

Show bundle download progress on iOS #15066

janicduplessis commented Jul 17, 2017 •

edited

Loading

facebook-github-bot commented Jul 19, 2017

shergin Jul 20, 2017

janicduplessis Jul 22, 2017

shergin Jul 25, 2017

janicduplessis Jul 25, 2017

facebook-github-bot commented Jul 25, 2017

pull-bot commented Jul 25, 2017 •

edited

Loading

janicduplessis commented Aug 2, 2017

facebook-github-bot commented Aug 3, 2017

shergin commented Aug 6, 2017

janicduplessis commented Aug 10, 2017 •

edited

Loading

facebook-github-bot commented Aug 10, 2017

facebook-github-bot commented Aug 11, 2017

shergin left a comment

shergin Aug 11, 2017

ide Aug 11, 2017

janicduplessis Aug 14, 2017

facebook-github-bot commented Aug 14, 2017

Show bundle download progress on iOS #15066

Show bundle download progress on iOS #15066

Conversation

janicduplessis commented Jul 17, 2017 • edited Loading

facebook-github-bot commented Jul 19, 2017

shergin Jul 20, 2017

Choose a reason for hiding this comment

janicduplessis Jul 22, 2017

Choose a reason for hiding this comment

shergin Jul 25, 2017

Choose a reason for hiding this comment

janicduplessis Jul 25, 2017

Choose a reason for hiding this comment

facebook-github-bot commented Jul 25, 2017

pull-bot commented Jul 25, 2017 • edited Loading

janicduplessis commented Aug 2, 2017

facebook-github-bot commented Aug 3, 2017

shergin commented Aug 6, 2017

janicduplessis commented Aug 10, 2017 • edited Loading

facebook-github-bot commented Aug 10, 2017

facebook-github-bot commented Aug 11, 2017

shergin left a comment

Choose a reason for hiding this comment

shergin Aug 11, 2017

Choose a reason for hiding this comment

ide Aug 11, 2017

Choose a reason for hiding this comment

janicduplessis Aug 14, 2017

Choose a reason for hiding this comment

facebook-github-bot commented Aug 14, 2017

janicduplessis commented Jul 17, 2017 •

edited

Loading

pull-bot commented Jul 25, 2017 •

edited

Loading

janicduplessis commented Aug 10, 2017 •

edited

Loading