Skip to content
This repository was archived by the owner on Nov 22, 2024. It is now read-only.

Commit fa67c21

Browse files
mweststratefacebook-github-bot
authored andcommitted
Fix bug where client plugins weren't always started
Summary: Changelog: Fixed issue where occasionally a plugin wouldn't open after starting Flipper This fixes a long standing issue where rarely Flipper wouldn't show the selected plugin. This turned out to be a raise condition, that was easy to reproduce in the Flipper browser version; if a client register before all the plugins are loaded, the plugins that are enabled for that client, but not loaded yet, will not instantiate and hence not show up. This diff fixes that Reviewed By: timur-valiev, aigoncharov Differential Revision: D32987162 fbshipit-source-id: f3179cd9b6f2e4e79d05be1f2236f63acdf50495
1 parent bb23b36 commit fa67c21

File tree

7 files changed

+43
-24
lines changed

7 files changed

+43
-24
lines changed

desktop/flipper-ui-core/src/Client.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
isFlipperMessageDebuggingEnabled,
4747
registerFlipperDebugMessage,
4848
} from './chrome/FlipperMessages';
49+
import {waitFor} from './utils/waitFor';
4950

5051
type Plugins = Set<string>;
5152
type PluginsArr = Array<string>;
@@ -173,6 +174,8 @@ export default class Client extends EventEmitter {
173174

174175
async init() {
175176
await this.loadPlugins();
177+
// if a client arrives before all plugins are loaded, we'll have to wait
178+
await waitFor(this.store, (state) => state.plugins.initialized);
176179
// this starts all sandy enabled plugins
177180
this.plugins.forEach((pluginId) =>
178181
this.startPluginIfNeeded(this.getPlugin(pluginId)),

desktop/flipper-ui-core/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Object {
8484
"disabledPlugins": Array [],
8585
"failedPlugins": Array [],
8686
"gatekeepedPlugins": Array [],
87-
"initialized": false,
87+
"initialized": true,
8888
"installedPlugins": Map {},
8989
"loadedPlugins": Map {},
9090
"marketplacePlugins": Array [],

desktop/flipper-ui-core/src/dispatcher/flipperServer.tsx

+5-5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import BaseDevice from '../devices/BaseDevice';
2222
import {ClientDescription, timeout} from 'flipper-common';
2323
import {reportPlatformFailures} from 'flipper-common';
2424
import {sideEffect} from '../utils/sideEffect';
25+
import {waitFor} from '../utils/waitFor';
2526

2627
export function connectFlipperServerToStore(
2728
server: FlipperServer,
@@ -112,17 +113,16 @@ export function connectFlipperServerToStore(
112113
'Flipper server started and accepting device / client connections',
113114
);
114115

115-
server
116-
.exec('device-list')
116+
// this flow is spawned delibarately from this main flow
117+
waitFor(store, (state) => state.plugins.initialized)
118+
.then(() => server.exec('device-list'))
117119
.then((devices) => {
118120
// register all devices
119121
devices.forEach((device) => {
120122
handleDeviceConnected(server, store, logger, device);
121123
});
122124
})
123-
.then(() => {
124-
return server.exec('client-list');
125-
})
125+
.then(() => server.exec('client-list'))
126126
.then((clients) => {
127127
clients.forEach((client) => {
128128
handleClientConnected(server, store, logger, client);

desktop/flipper-ui-core/src/dispatcher/handleOpenPluginDeeplink.tsx

+2-16
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import React from 'react';
1111
import {Dialog, getFlipperLib} from 'flipper-plugin';
1212
import {isTest} from 'flipper-common';
1313
import {getUser} from '../fb-stubs/user';
14-
import {State, Store} from '../reducers/index';
14+
import {Store} from '../reducers/index';
1515
import {checkForUpdate} from '../fb-stubs/checkForUpdate';
1616
import {getAppVersion} from '../utils/info';
1717
import {UserNotSignedInError} from 'flipper-common';
@@ -39,6 +39,7 @@ import {
3939
OpenPluginParams,
4040
} from '../deeplinkTracking';
4141
import {getRenderHostInstance} from '../RenderHost';
42+
import {waitFor} from '../utils/waitFor';
4243

4344
export function parseOpenPluginParams(query: string): OpenPluginParams {
4445
// 'flipper://open-plugin?plugin-id=graphql&client=facebook&devices=android,ios&chrome=1&payload='
@@ -272,21 +273,6 @@ async function waitForLogin(store: Store) {
272273
return waitFor(store, (state) => !!state.user?.id);
273274
}
274275

275-
// make this more reusable?
276-
function waitFor(
277-
store: Store,
278-
predicate: (state: State) => boolean,
279-
): Promise<void> {
280-
return new Promise<void>((resolve) => {
281-
const unsub = store.subscribe(() => {
282-
if (predicate(store.getState())) {
283-
unsub();
284-
resolve();
285-
}
286-
});
287-
});
288-
}
289-
290276
async function verifyFlipperIsUpToDate(title: string) {
291277
if (!isProduction() || isTest()) {
292278
return;

desktop/flipper-ui-core/src/startFlipperDesktop.tsx

+2-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,6 @@ function init(flipperServer: FlipperServer) {
143143
setLoggerInstance(logger);
144144
startGlobalErrorHandling();
145145
loadTheme(settings.darkMode);
146-
connectFlipperServerToStore(flipperServer, store, logger);
147146

148147
// rehydrate app state before exposing init
149148
const persistor = persistStore(store, undefined, () => {
@@ -163,6 +162,8 @@ function init(flipperServer: FlipperServer) {
163162
}
164163
});
165164

165+
connectFlipperServerToStore(flipperServer, store, logger);
166+
166167
ReactDOM.render(
167168
<AppFrame logger={logger} persistor={persistor} />,
168169
document.getElementById('root'),

desktop/flipper-ui-core/src/test-utils/MockFlipper.tsx

+3-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
ClientResponseType,
2020
} from 'flipper-common';
2121
import {PluginDefinition} from '../plugin';
22-
import {registerPlugins} from '../reducers/plugins';
22+
import {pluginsInitialized, registerPlugins} from '../reducers/plugins';
2323
import {getLogger} from 'flipper-common';
2424
import {initializeFlipperLibImplementation} from '../utils/flipperLibImplementation';
2525
import pluginManager from '../dispatcher/pluginManager';
@@ -28,6 +28,7 @@ import ArchivedDevice from '../devices/ArchivedDevice';
2828
import {ClientQuery, DeviceOS} from 'flipper-common';
2929
import {TestDevice} from './TestDevice';
3030
import {getRenderHostInstance} from '../RenderHost';
31+
import {waitFor} from '../utils/waitFor';
3132

3233
export interface AppOptions {
3334
plugins?: PluginDefinition[];
@@ -95,6 +96,7 @@ export default class MockFlipper {
9596
this._logger,
9697
);
9798
this._store.dispatch(registerPlugins(plugins ?? []));
99+
this._store.dispatch(pluginsInitialized());
98100
}
99101

100102
public async initWithDeviceAndClient(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @format
8+
*/
9+
10+
import {State, Store} from '../reducers/index';
11+
12+
export async function waitFor(
13+
store: Store,
14+
predicate: (state: State) => boolean,
15+
): Promise<void> {
16+
if (predicate(store.getState())) {
17+
return;
18+
}
19+
return new Promise<void>((resolve) => {
20+
const unsub = store.subscribe(() => {
21+
if (predicate(store.getState())) {
22+
unsub();
23+
resolve();
24+
}
25+
});
26+
});
27+
}

0 commit comments

Comments
 (0)