Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use RCTBridgeDelegate in the new project template #23031

Closed

Conversation

@janicduplessis
Copy link
Contributor

janicduplessis commented Jan 17, 2019

The main goal of this change is to fix an issue that happens on iOS when the native app starts before the packager server is available which causes app reloads to fail continually until the app is restarted.

What happens is that with the current setup we call RCTBundleURLProvider#jsBundleURLForBundleRoot once in AppDelegate#didFinishLaunchingWithOptions. If at that point the packager server is not running yet it will return nil (expected behaviour) and that will be passed to the bridge constructor. Subsequent reloads will keep trying to load this nil bundle url since it has no way to ask RCTBundleURLProvider for a new url now that the packager is actually running.

We can fix this by using RCTBridgeDelegate instead which is a lot more flexible. Instead of passing the bundle url at construction time it will call the sourceURLForBridge method each time the bridge is reloaded. This means that even if the packager is not running yet and that RCTBundleURLProvider returns nil for the first invocation, subsequent reloads will call RCTBundleURLProvider again for a new value.

Changelog:

[iOS] [Added] - Use RCTBridgeDelegate in the new project template

Test Plan:

Tested that using RCTBridgeDelegate fixed the issue by building an app first without starting the packager and waiting for the no bundle preset redscreen. Then I started the packager and hit reload and made sure the app was able to load properly. Before this fix the app would keep failing to load the bundle even if the packager was started.

I wasn't able to test the template directly (no sure how to do this now that local-cli is in another repo) but I applied the same changes that I did to my app to the template file carefully.

Copy link

facebook-github-bot left a comment

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@cpojer

This comment has been minimized.

Copy link
Contributor

cpojer commented Jan 17, 2019

Looks good, thanks for the PR :)

@react-native-bot

This comment has been minimized.

Copy link
Collaborator

react-native-bot commented Jan 17, 2019

@janicduplessis merged commit 68b0d4d into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Jan 17, 2019
matt-oakes pushed a commit to matt-oakes/react-native that referenced this pull request Feb 7, 2019
Summary:
The main goal of this change is to fix an issue that happens on iOS when the native app starts before the packager server is available which causes app reloads to fail continually until the app is restarted.

What happens is that with the current setup we call `RCTBundleURLProvider#jsBundleURLForBundleRoot` once in `AppDelegate#didFinishLaunchingWithOptions`. If at that point the packager server is not running yet it will return nil (expected behaviour) and that will be passed to the bridge constructor. Subsequent reloads will keep trying to load this nil bundle url since it has no way to ask `RCTBundleURLProvider` for a new url now that the packager is actually running.

We can fix this by using `RCTBridgeDelegate` instead which is a lot more flexible. Instead of passing the bundle url at construction time it will call the `sourceURLForBridge` method each time the bridge is reloaded. This means that even if the packager is not running yet and that `RCTBundleURLProvider` returns nil for the first invocation, subsequent reloads will call `RCTBundleURLProvider` again for a new value.

Changelog:
----------

[iOS] [Added] - Use RCTBridgeDelegate in the new project template
Pull Request resolved: facebook#23031

Differential Revision: D13710048

Pulled By: cpojer

fbshipit-source-id: 0059c5c962d508737ae410a82315c11ad305efe8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.