Skip to content

Commit

Permalink
Deprecate bridge reload API [1/n]
Browse files Browse the repository at this point in the history
Summary:
Testing the waters with an idea for unifying RN lifecycle with/without bridge.

### Motivation/Background

RN bridge is being reloaded from [several different places](https://fburl.com/codesearch/ae3zeatt). When this happens, the bridge does a bunch of things:
1. Post `RCTBridgeWillReloadNotification` (RCTSurfacePresenter listens to this and reloads)
2. Invalidate batched bridge, which does:
    a. invalidate display link
    b. post `RCTBridgeWillInvalidateModules/RCTBridgeDidInvalidateModules` (TurboModuleManager listens to this and reloads modules)
    c. clear js thread state
    d. clear some local caches
3. Set up (reload bundle and batched bridge)

In a bridgeless world, there isn't one thing which owns all these peices of infra, and can reload them.

### Plan

Use `RCTReloadCommand` to handle reloads. This light class previously only handled CMD+R. The new workflow looks like this:
1. Anything which cares about reloads can register itself as a listener. (RCTBridge, RCTSurfacePresenter, TurboModuleManager...)
2. Anything that previously called `bridge reload` now calls `RCTTriggerReloadCommandListeners`.
3. Delete old notifications

### Alternate Plan

Use yet another NSNotification. I like `RCTReloadCommand` better.

### Unknowns

Looks like we log the reason for bridge reloading [here](https://fburl.com/diffusion/lc9jj8la), do we want to save this behaviour? Does anyone look at why bridge is being reloaded cc/Rick? If so, I can add back that functionality to `RCTReloadCommand`.

It should be possible to customize the order/priority of what gets reloaded first. There may be some racy behaviour that I haven't thought about yet.

Changelog: [iOS][Deprecated] Deprecate [bridge reload] API - prefer RCTReloadCommand

Reviewed By: shergin

Differential Revision: D17869635

fbshipit-source-id: 81f39eaa2c3ce08ea1bc6f2193684c2630d81a2d
  • Loading branch information
Peter Argany authored and facebook-github-bot committed Oct 30, 2019
1 parent 35b8b06 commit ffe2306
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
6 changes: 3 additions & 3 deletions React/Base/RCTBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,12 @@ RCT_EXTERN void RCTEnableTurboModule(BOOL enabled);
/**
* Reload the bundle and reset executor & modules. Safe to call from any thread.
*/
- (void)reload __deprecated_msg("Call reloadWithReason instead");
- (void)reload __deprecated_msg("Use RCTReloadCommand instead");

/**
* Reload the bundle and reset executor & modules. Safe to call from any thread.
*/
- (void)reloadWithReason:(NSString *)reason;
- (void)reloadWithReason:(NSString *)reason __deprecated_msg("Use RCTReloadCommand instead");

/**
* Handle notifications for a fast refresh. Safe to call from any thread.
Expand All @@ -286,7 +286,7 @@ RCT_EXTERN void RCTEnableTurboModule(BOOL enabled);
/**
* Inform the bridge, and anything subscribing to it, that it should reload.
*/
- (void)requestReload __deprecated_msg("Call reloadWithReason instead");
- (void)requestReload __deprecated_msg("Use RCTReloadCommand instead");

/**
* Says whether bridge has started receiving calls from javascript.
Expand Down
8 changes: 7 additions & 1 deletion React/Base/RCTBridge.m
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,16 @@ - (BOOL)moduleIsInitialized:(Class)moduleClass
}

/**
* Legacy reload, please use reloadWithReason and provide a reason for stats.
* DEPRECATED - please use RCTReloadCommand.
*/
- (void)reload
{
[self reloadWithReason:@"Unknown from bridge"];
}

/**
* DEPRECATED - please use RCTReloadCommand.
*/
- (void)reloadWithReason:(NSString *)reason
{
#if RCT_ENABLE_INSPECTOR && !TARGET_OS_UIKITFORMAC
Expand Down Expand Up @@ -328,6 +331,9 @@ - (void)onFastRefresh
[[NSNotificationCenter defaultCenter] postNotificationName:RCTBridgeFastRefreshNotification object:self];
}

/**
* DEPRECATED - please use RCTReloadCommand.
*/
- (void)requestReload
{
[self reloadWithReason:@"Requested from bridge"];
Expand Down
13 changes: 11 additions & 2 deletions React/Base/RCTReloadCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,21 @@

#import <React/RCTDefines.h>

/**
* A protocol which should be conformed to in order to be notified of RN reload events. These events can be
* created by CMD+R or dev menu during development, or anywhere the trigger is exposed to JS.
* The listener must also register itself using the method below.
*/
@protocol RCTReloadListener
- (void)didReceiveReloadCommand;
@end

/** Registers a weakly-held observer of the Command+R reload key command. */
/**
* Registers a weakly-held observer of RN reload events.
*/
RCT_EXTERN void RCTRegisterReloadCommandListener(id<RCTReloadListener> listener);

/** Triggers a reload for all current listeners. You shouldn't need to use this directly in most cases. */
/**
* Triggers a reload for all current listeners.
*/
RCT_EXTERN void RCTTriggerReloadCommandListeners(void);

0 comments on commit ffe2306

Please sign in to comment.