From 6fb28df855b5dfa52160b203fa1ad8ebf1f39106 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 21 Jun 2021 08:35:52 -0700 Subject: [PATCH] Make Store initialization independent of module order Summary: Changed some imports, and again the Flipper initialisation broke. Refactored the store initialization to create nowhere module local constants, which prevents generally against module loading issues, making it possible to load all code first, and then intialise things through the `init()` method, which should make Flipper initialisation a lot more robust to changes Reviewed By: passy Differential Revision: D29233603 fbshipit-source-id: 322cb87cba23228b1d7a88634b7b3995e27cc277 --- .../dispatcher/__tests__/iOSDevice.node.tsx | 4 +- .../src/dispatcher/__tests__/plugins.node.tsx | 4 +- desktop/app/src/init.tsx | 31 ++-- desktop/app/src/reducers/index.tsx | 160 +++++++++--------- .../__tests__/LaunchEmulator.spec.tsx | 4 +- desktop/app/src/store.tsx | 54 +++--- desktop/app/src/test-utils/MockFlipper.tsx | 4 +- desktop/app/src/utils/__tests__/info.node.tsx | 4 +- desktop/app/src/utils/notifications.tsx | 4 +- .../__tests__/DatabaseDetailSidebar.node.tsx | 4 +- desktop/static/index.dev.html | 2 +- desktop/static/index.html | 2 +- 12 files changed, 148 insertions(+), 129 deletions(-) diff --git a/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx b/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx index 98679be75..374203c65 100644 --- a/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx +++ b/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx @@ -12,11 +12,11 @@ import { getAllPromisesForQueryingDevices, } from '../iOSDevice'; import configureStore from 'redux-mock-store'; -import reducers, {State} from '../../reducers/index'; +import {State, createRootReducer} from '../../reducers/index'; import {getInstance} from '../../fb-stubs/Logger'; const mockStore = configureStore([])( - reducers(undefined, {type: 'INIT'}), + createRootReducer()(undefined, {type: 'INIT'}), ); const logger = getInstance(); diff --git a/desktop/app/src/dispatcher/__tests__/plugins.node.tsx b/desktop/app/src/dispatcher/__tests__/plugins.node.tsx index d92474b87..88a2ef4a6 100644 --- a/desktop/app/src/dispatcher/__tests__/plugins.node.tsx +++ b/desktop/app/src/dispatcher/__tests__/plugins.node.tsx @@ -20,7 +20,7 @@ import {BundledPluginDetails, InstalledPluginDetails} from 'flipper-plugin-lib'; import path from 'path'; import {remote} from 'electron'; import {FlipperPlugin} from '../../plugin'; -import reducers, {State} from '../../reducers/index'; +import {createRootReducer, State} from '../../reducers/index'; import {getInstance} from '../../fb-stubs/Logger'; import configureStore from 'redux-mock-store'; import {TEST_PASSING_GK, TEST_FAILING_GK} from '../../fb-stubs/GK'; @@ -33,7 +33,7 @@ import loadDynamicPlugins from '../../utils/loadDynamicPlugins'; const loadDynamicPluginsMock = mocked(loadDynamicPlugins); const mockStore = configureStore([])( - reducers(undefined, {type: 'INIT'}), + createRootReducer()(undefined, {type: 'INIT'}), ); const logger = getInstance(); diff --git a/desktop/app/src/init.tsx b/desktop/app/src/init.tsx index 291cbe638..5e76a2784 100644 --- a/desktop/app/src/init.tsx +++ b/desktop/app/src/init.tsx @@ -24,7 +24,7 @@ import {initLauncherHooks} from './utils/launcher'; import {setPersistor} from './utils/persistor'; import React from 'react'; import path from 'path'; -import {store} from './store'; +import {getStore} from './store'; import {cache} from '@emotion/css'; import {CacheProvider} from '@emotion/react'; import {enableMapSet} from 'immer'; @@ -59,8 +59,6 @@ if (process.env.NODE_ENV === 'development' && os.platform() === 'darwin') { global.electronRequire('mac-ca'); } -const logger = initLogger(store); - enableMapSet(); GK.init(); @@ -127,7 +125,7 @@ class AppFrame extends React.Component< ) : ( <_LoggerContext.Provider value={logger}> - + @@ -181,6 +179,18 @@ function setProcessState(store: Store) { } function init() { + const store = getStore(); + const logger = initLogger(store); + + // rehydrate app state before exposing init + const persistor = persistStore(store, undefined, () => { + // Make sure process state is set before dispatchers run + setProcessState(store); + dispatcher(store, logger); + }); + + setPersistor(persistor); + initializeFlipperLibImplementation(store, logger); _setGlobalInteractionReporter((r) => { logger.track('usage', 'interaction', r); @@ -213,18 +223,13 @@ function init() { ); } -// rehydrate app state before exposing init -const persistor = persistStore(store, undefined, () => { - // Make sure process state is set before dispatchers run - setProcessState(store); - dispatcher(store, logger); - // make init function callable from outside - window.Flipper.init = init; +setImmediate(() => { + // make sure all modules are loaded + // @ts-ignore + window.flipperInit = init; window.dispatchEvent(new Event('flipper-store-ready')); }); -setPersistor(persistor); - const CodeBlock = styled(Input.TextArea)({ ...theme.monospace, color: theme.textColorSecondary, diff --git a/desktop/app/src/reducers/index.tsx b/desktop/app/src/reducers/index.tsx index 0b12c8da4..2f9f60468 100644 --- a/desktop/app/src/reducers/index.tsx +++ b/desktop/app/src/reducers/index.tsx @@ -140,82 +140,84 @@ const launcherSettingsStorage = new LauncherSettingsStorage( resolve(launcherConfigDir(), 'flipper-launcher.toml'), ); -export default combineReducers({ - application, - connections: persistReducer( - { - key: 'connections', - storage, - whitelist: [ - 'userPreferredDevice', - 'userPreferredPlugin', - 'userPreferredApp', - 'enabledPlugins', - 'enabledDevicePlugins', - ], - transforms: [ - setTransformer({ - whitelist: ['enabledDevicePlugins', 'userStarredDevicePlugins'], - }), - ], - version: devicesPersistVersion, - migrate: createMigrate(devicesPersistMigrations), - }, - connections, - ), - pluginStates, - pluginMessageQueue: pluginMessageQueue as any, - notifications: persistReducer( - { - key: 'notifications', - storage, - whitelist: ['blacklistedPlugins', 'blacklistedCategories'], - }, - notifications, - ), - plugins: persistReducer( - { - key: 'plugins', - storage, - whitelist: ['marketplacePlugins', 'uninstalledPluginNames'], - transforms: [setTransformer({whitelist: ['uninstalledPluginNames']})], - version: pluginsPersistVersion, - migrate: createMigrate(pluginsPersistMigrations), - }, - plugins, - ), - supportForm, - pluginManager, - user: persistReducer( - { - key: 'user', - storage, - }, - user, - ), - settingsState: persistReducer( - {key: 'settings', storage: settingsStorage}, - settings, - ), - launcherSettingsState: persistReducer( - { - key: 'launcherSettings', - storage: launcherSettingsStorage, - serialize: false, - // @ts-ignore: property is erroneously missing in redux-persist type definitions - deserialize: false, - }, - launcherSettings, - ), - healthchecks: persistReducer( - { - key: 'healthchecks', - storage, - whitelist: ['acknowledgedProblems'], - }, - healthchecks, - ), - usageTracking, - pluginDownloads, - pluginLists, -}); +export function createRootReducer() { + return combineReducers({ + application, + connections: persistReducer( + { + key: 'connections', + storage, + whitelist: [ + 'userPreferredDevice', + 'userPreferredPlugin', + 'userPreferredApp', + 'enabledPlugins', + 'enabledDevicePlugins', + ], + transforms: [ + setTransformer({ + whitelist: ['enabledDevicePlugins', 'userStarredDevicePlugins'], + }), + ], + version: devicesPersistVersion, + migrate: createMigrate(devicesPersistMigrations), + }, + connections, + ), + pluginStates, + pluginMessageQueue: pluginMessageQueue as any, + notifications: persistReducer( + { + key: 'notifications', + storage, + whitelist: ['blacklistedPlugins', 'blacklistedCategories'], + }, + notifications, + ), + plugins: persistReducer( + { + key: 'plugins', + storage, + whitelist: ['marketplacePlugins', 'uninstalledPluginNames'], + transforms: [setTransformer({whitelist: ['uninstalledPluginNames']})], + version: pluginsPersistVersion, + migrate: createMigrate(pluginsPersistMigrations), + }, + plugins, + ), + supportForm, + pluginManager, + user: persistReducer( + { + key: 'user', + storage, + }, + user, + ), + settingsState: persistReducer( + {key: 'settings', storage: settingsStorage}, + settings, + ), + launcherSettingsState: persistReducer( + { + key: 'launcherSettings', + storage: launcherSettingsStorage, + serialize: false, + // @ts-ignore: property is erroneously missing in redux-persist type definitions + deserialize: false, + }, + launcherSettings, + ), + healthchecks: persistReducer( + { + key: 'healthchecks', + storage, + whitelist: ['acknowledgedProblems'], + }, + healthchecks, + ), + usageTracking, + pluginDownloads, + pluginLists, + }); +} diff --git a/desktop/app/src/sandy-chrome/appinspect/__tests__/LaunchEmulator.spec.tsx b/desktop/app/src/sandy-chrome/appinspect/__tests__/LaunchEmulator.spec.tsx index 62079e8a5..4d60b1852 100644 --- a/desktop/app/src/sandy-chrome/appinspect/__tests__/LaunchEmulator.spec.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/__tests__/LaunchEmulator.spec.tsx @@ -13,7 +13,7 @@ import {Provider} from 'react-redux'; import {createStore} from 'redux'; import {LaunchEmulatorDialog} from '../LaunchEmulator'; -import {rootReducer} from '../../../store'; +import {createRootReducer} from '../../../reducers'; import {act} from 'react-dom/test-utils'; import {sleep} from '../../../utils'; @@ -24,7 +24,7 @@ jest.mock('../../../devices/AndroidDevice', () => ({ import {launchEmulator} from '../../../devices/AndroidDevice'; test('Can render and launch android apps', async () => { - const store = createStore(rootReducer); + const store = createStore(createRootReducer()); const onClose = jest.fn(); const renderer = render( diff --git a/desktop/app/src/store.tsx b/desktop/app/src/store.tsx index 8f39de246..f57712773 100644 --- a/desktop/app/src/store.tsx +++ b/desktop/app/src/store.tsx @@ -9,31 +9,43 @@ import './global'; import {createStore} from 'redux'; -import reducers, {Actions, State as StoreState, Store} from './reducers/index'; +import { + createRootReducer, + Actions, + State as StoreState, + Store, +} from './reducers/index'; import {stateSanitizer} from './utils/reduxDevToolsConfig'; import isProduction from './utils/isProduction'; -import {_SandyPluginDefinition} from 'flipper-plugin'; -export const store: Store = createStore( - rootReducer, - // @ts-ignore Type definition mismatch - window.__REDUX_DEVTOOLS_EXTENSION__ - ? window.__REDUX_DEVTOOLS_EXTENSION__({ - // @ts-ignore: stateSanitizer is not part of type definition. - stateSanitizer, - }) - : undefined, -); +let store: Store; -export function rootReducer( - state: StoreState | undefined, - action: Actions, -): StoreState { - return reducers(state, action); +function initStore() { + const rootReducer = createRootReducer(); + + store = createStore( + rootReducer, + // @ts-ignore Type definition mismatch + window.__REDUX_DEVTOOLS_EXTENSION__ + ? window.__REDUX_DEVTOOLS_EXTENSION__({ + // @ts-ignore: stateSanitizer is not part of type definition. + stateSanitizer, + }) + : undefined, + ); + + if (!isProduction()) { + // For debugging purposes only + // @ts-ignore + window.flipperStore = store; + } + return store; } -if (!isProduction()) { - // For debugging purposes only - // @ts-ignore - window.flipperStore = store; +// grab store lazily, to not break module loading order... +export function getStore() { + if (!store) { + return initStore(); + } + return store; } diff --git a/desktop/app/src/test-utils/MockFlipper.tsx b/desktop/app/src/test-utils/MockFlipper.tsx index edb61684c..5321c1658 100644 --- a/desktop/app/src/test-utils/MockFlipper.tsx +++ b/desktop/app/src/test-utils/MockFlipper.tsx @@ -9,7 +9,7 @@ import {createStore} from 'redux'; import BaseDevice from '../devices/BaseDevice'; -import {rootReducer} from '../store'; +import {createRootReducer} from '../reducers'; import {Store} from '../reducers/index'; import Client, {ClientQuery, FlipperClientConnection} from '../Client'; import {buildClientId} from '../utils/clientUtils'; @@ -75,7 +75,7 @@ export default class MockFlipper { } public async init({plugins}: AppOptions = {}) { - this._store = createStore(rootReducer); + this._store = createStore(createRootReducer()); this._logger = getInstance(); this.unsubscribePluginManager = pluginManager(this._store, this._logger, { runSideEffectsSynchronously: true, diff --git a/desktop/app/src/utils/__tests__/info.node.tsx b/desktop/app/src/utils/__tests__/info.node.tsx index e27b8fd91..46f4fe688 100644 --- a/desktop/app/src/utils/__tests__/info.node.tsx +++ b/desktop/app/src/utils/__tests__/info.node.tsx @@ -9,7 +9,7 @@ import {Store} from '../../reducers/index'; import {createStore} from 'redux'; -import {rootReducer} from '../../store'; +import {createRootReducer} from '../../reducers'; import initialize, {getInfo} from '../info'; import {registerLoadedPlugins} from '../../reducers/plugins'; import {TestUtils} from 'flipper-plugin'; @@ -37,7 +37,7 @@ describe('info', () => { let mockStore: Store; beforeEach(() => { - mockStore = createStore(rootReducer); + mockStore = createStore(createRootReducer()); mockStore.dispatch({type: 'INIT'}); }); diff --git a/desktop/app/src/utils/notifications.tsx b/desktop/app/src/utils/notifications.tsx index 72b1b1945..29b5f61e5 100644 --- a/desktop/app/src/utils/notifications.tsx +++ b/desktop/app/src/utils/notifications.tsx @@ -11,7 +11,7 @@ import {notification, Typography} from 'antd'; import React from 'react'; import {ConsoleLogs} from '../chrome/ConsoleLogs'; import {setStaticView} from '../reducers/connections'; -import {store} from '../store'; +import {getStore} from '../store'; import {Layout} from '../ui'; import {v4 as uuid} from 'uuid'; @@ -29,7 +29,7 @@ export function showErrorNotification(message: string, description?: string) { See{' '} { - store.dispatch(setStaticView(ConsoleLogs)); + getStore().dispatch(setStaticView(ConsoleLogs)); notification.close(key); }}> logs diff --git a/desktop/plugins/public/databases/__tests__/DatabaseDetailSidebar.node.tsx b/desktop/plugins/public/databases/__tests__/DatabaseDetailSidebar.node.tsx index 6ae6b0e56..46a0c7302 100644 --- a/desktop/plugins/public/databases/__tests__/DatabaseDetailSidebar.node.tsx +++ b/desktop/plugins/public/databases/__tests__/DatabaseDetailSidebar.node.tsx @@ -11,7 +11,7 @@ import {render, fireEvent} from '@testing-library/react'; import React from 'react'; // TODO T71355623 // eslint-disable-next-line flipper/no-relative-imports-across-packages -import reducers, {Store} from '../../../../app/src/reducers'; +import {Store, createRootReducer} from '../../../../app/src/reducers'; import configureStore from 'redux-mock-store'; import {Provider} from 'react-redux'; @@ -46,7 +46,7 @@ const values: Array = [ ]; const mockStore: Store = configureStore([])( - reducers(undefined, {type: 'INIT'}), + createRootReducer()(undefined, {type: 'INIT'}), ) as Store; beforeEach(() => { diff --git a/desktop/static/index.dev.html b/desktop/static/index.dev.html index 2b3e481b8..7c686980b 100644 --- a/desktop/static/index.dev.html +++ b/desktop/static/index.dev.html @@ -86,7 +86,7 @@ openError('Script failure. Check Chrome console for more info.'); }; - window.addEventListener('flipper-store-ready', () => global.Flipper.init()); + window.addEventListener('flipper-store-ready', () => global.flipperInit()); document.body.appendChild(script); } diff --git a/desktop/static/index.html b/desktop/static/index.html index 0508ff97e..c497636bd 100644 --- a/desktop/static/index.html +++ b/desktop/static/index.html @@ -15,7 +15,7 @@ global.electronRequire = window.require;