fix(app): Remove Electron RPC remote objects from Redux state

app-shell/src/main.js

@@ -43,7 +43,7 @@ function startUp() {
 
   // wire modules to UI dispatches
   const dispatch = action => {
-    log.debug('Sending action via IPC to renderer', { action })
+    log.silly('Sending action via IPC to renderer', { action })
     mainWindow.webContents.send('dispatch', action)
   }
 

app-shell/src/preload.js

@@ -3,11 +3,19 @@
 // defines subset of Electron API that renderer process is allowed to access
 // for security reasons
 import { ipcRenderer, remote } from 'electron'
+import cloneDeep from 'lodash/cloneDeep'
+
+const { getConfig } = remote.require('./config')
+const { getRobots } = remote.require('./discovery')
+const { CURRENT_VERSION, CURRENT_RELEASE_NOTES } = remote.require('./update')
 
 global.APP_SHELL_REMOTE = {
   ipcRenderer,
+  CURRENT_VERSION,
+  CURRENT_RELEASE_NOTES,
+  INITIAL_CONFIG: cloneDeep(getConfig()),
+  INITIAL_ROBOTS: getRobots(),
+
+  // TODO(mc, 2019-08-05): remove when we remove __buildrootEnabled FF
   apiUpdate: remote.require('./api-update'),
-  config: remote.require('./config'),
-  discovery: remote.require('./discovery'),
-  update: remote.require('./update'),
 }

app/src/config/__tests__/config.test.js

@@ -1,9 +1,7 @@
 // config tests
 import { updateConfig, configReducer, getConfig } from '..'
 
-import * as mockShell from '../../shell'
-
-jest.mock('../../shell', () => ({ getShellConfig: jest.fn() }))
+jest.mock('../../shell/remote', () => ({ INITIAL_CONFIG: { isConfig: true } }))
 
 describe('config', () => {
   let state
@@ -36,8 +34,6 @@ describe('config', () => {
     })
 
     test('gets store and overrides from remote for initial state', () => {
-      mockShell.getShellConfig.mockReturnValue({ isConfig: true })
-
       expect(configReducer(null, {})).toEqual({ isConfig: true })
     })
 

app/src/config/index.js

@@ -3,104 +3,12 @@
 import { setIn } from '@thi.ng/paths'
 import remove from 'lodash/remove'
 
-import { getShellConfig } from '../shell'
+import remote from '../shell/remote'
 
 import type { State, Action, ThunkAction } from '../types'
-import type { LogLevel } from '../logger'
+import type { Config, UpdateConfigAction } from './types'
 
-type UrlProtocol = 'file:' | 'http:'
-
-export type UpdateChannel = 'latest' | 'beta' | 'alpha'
-
-export type DiscoveryCandidates = string | Array<string>
-
-// TODO(mc, 2018-05-17): put this type somewhere common to app and app-shell
-export type Config = {
-  devtools: boolean,
-
-  // app update config
-  update: {
-    channel: UpdateChannel,
-  },
-
-  // robot update config
-  buildroot: {
-    manifestUrl: string,
-  },
-
-  // logging config
-  log: {
-    level: {
-      file: LogLevel,
-      console: LogLevel,
-    },
-  },
-
-  // ui and browser config
-  ui: {
-    width: number,
-    height: number,
-    url: {
-      protocol: UrlProtocol,
-      path: string,
-    },
-    webPreferences: {
-      webSecurity: boolean,
-    },
-  },
-
-  analytics: {
-    appId: string,
-    optedIn: boolean,
-    seenOptIn: boolean,
-  },
-
-  // deprecated; remove with first migration
-  p10WarningSeen: {
-    [id: string]: ?boolean,
-  },
-
-  support: {
-    userId: string,
-    createdAt: number,
-    name: string,
-    email: ?string,
-  },
-
-  discovery: {
-    candidates: DiscoveryCandidates,
-  },
-
-  // internal development flags
-  devInternal?: {
-    allPipetteConfig?: boolean,
-    tempdeckControls?: boolean,
-    enableThermocycler?: boolean,
-    enablePipettePlus?: boolean,
-    enableBuildRoot?: boolean,
-  },
-}
-
-type UpdateConfigAction = {|
-  type: 'config:UPDATE',
-  payload: {|
-    path: string,
-    value: any,
-  |},
-  meta: {|
-    shell: true,
-  |},
-|}
-
-type SetConfigAction = {|
-  type: 'config:SET',
-  payload: {|
-    path: string,
-    value: any,
-  |},
-|}
-
-export type ConfigAction = UpdateConfigAction | SetConfigAction
+export * from './types'
 
 // trigger a config value update to the app-shell via shell middleware
 export function updateConfig(path: string, value: any): UpdateConfigAction {
@@ -113,9 +21,8 @@ export function updateConfig(path: string, value: any): UpdateConfigAction {
 
 // config reducer
 export function configReducer(state: ?Config, action: Action): Config {
-  // initial state
-  // getShellConfig makes a sync RPC call, so use sparingly
-  if (!state) return getShellConfig()
+  // initial state from app-shell preloaded remote
+  if (!state) return remote.INITIAL_CONFIG
 
   switch (action.type) {
     case 'config:SET':
@@ -142,16 +49,17 @@ export function addManualIp(ip: string): ThunkAction {
     const previous: ?string = [].concat(candidates).find(i => i === ip)
     let nextCandidatesList = candidates
     if (!previous) nextCandidatesList = nextCandidatesList.concat(ip)
+
     return dispatch(updateConfig('discovery.candidates', nextCandidatesList))
   }
 }
 
 export function removeManualIp(ip: string): ThunkAction {
   return (dispatch, getState) => {
     const candidates = [].concat(getConfig(getState()).discovery.candidates)
-    remove(candidates, c => {
-      return c === ip
-    })
+
+    remove(candidates, c => c === ip)
+
     return dispatch(updateConfig('discovery.candidates', candidates))
   }
 }

app/src/config/types.js

@@ -0,0 +1,95 @@
+// @flow
+import type { LogLevel } from '../logger'
+
+export type UrlProtocol = 'file:' | 'http:'
+
+export type UpdateChannel = 'latest' | 'beta' | 'alpha'
+
+export type DiscoveryCandidates = string | Array<string>
+
+export type Config = {
+  devtools: boolean,
+
+  // app update config
+  update: {
+    channel: UpdateChannel,
+  },
+
+  // robot update config
+  buildroot: {
+    manifestUrl: string,
+  },
+
+  // logging config
+  log: {
+    level: {
+      file: LogLevel,
+      console: LogLevel,
+    },
+  },
+
+  // ui and browser config
+  ui: {
+    width: number,
+    height: number,
+    url: {
+      protocol: UrlProtocol,
+      path: string,
+    },
+    webPreferences: {
+      webSecurity: boolean,
+    },
+  },
+
+  analytics: {
+    appId: string,
+    optedIn: boolean,
+    seenOptIn: boolean,
+  },
+
+  // deprecated; remove with first migration
+  p10WarningSeen: {
+    [id: string]: ?boolean,
+  },
+
+  support: {
+    userId: string,
+    createdAt: number,
+    name: string,
+    email: ?string,
+  },
+
+  discovery: {
+    candidates: DiscoveryCandidates,
+  },
+
+  // internal development flags
+  devInternal?: {
+    allPipetteConfig?: boolean,
+    tempdeckControls?: boolean,
+    enableThermocycler?: boolean,
+    enablePipettePlus?: boolean,
+    enableBuildRoot?: boolean,
+  },
+}
+
+export type UpdateConfigAction = {|
+  type: 'config:UPDATE',
+  payload: {|
+    path: string,
+    value: any,
+  |},
+  meta: {|
+    shell: true,
+  |},
+|}
+
+export type SetConfigAction = {|
+  type: 'config:SET',
+  payload: {|
+    path: string,
+    value: any,
+  |},
+|}
+
+export type ConfigAction = UpdateConfigAction | SetConfigAction

app/src/discovery/__tests__/reducer.test.js

@@ -1,8 +1,8 @@
 // discovery reducer test
 import { discoveryReducer } from '..'
 
-jest.mock('../../shell', () => ({
-  getShellRobots: () => [
+jest.mock('../../shell/remote', () => ({
+  INITIAL_ROBOTS: [
     { name: 'foo', ip: '192.168.1.1', port: 31950 },
     { name: 'bar', ip: '192.168.1.2', port: 31950 },
   ],

app/src/discovery/index.js

@@ -4,7 +4,7 @@ import groupBy from 'lodash/groupBy'
 import mapValues from 'lodash/mapValues'
 import some from 'lodash/some'
 
-import { getShellRobots } from '../shell'
+import remote from '../shell/remote'
 import * as actions from './actions'
 
 import type { Service } from '@opentrons/discovery-client'
@@ -29,15 +29,14 @@ export type DiscoveryState = {|
 export const RESTART_PENDING: RestartStatus = 'pending'
 export const RESTART_DOWN: RestartStatus = 'down'
 
-// getShellRobots makes a sync RPC call, so use sparingly
-const initialState: DiscoveryState = {
+const INITIAL_STATE: DiscoveryState = {
   scanning: false,
-  robotsByName: normalizeRobots(getShellRobots()),
   restartsByName: {},
+  robotsByName: normalizeRobots(remote.INITIAL_ROBOTS),
 }
 
 export function discoveryReducer(
-  state: DiscoveryState = initialState,
+  state: DiscoveryState = INITIAL_STATE,
   action: Action
 ): DiscoveryState {
   switch (action.type) {

app/src/epic.js

@@ -2,8 +2,8 @@
 // root application epic
 import { combineEpics } from 'redux-observable'
 
-import { buildrootUpdateEpic } from './shell'
 import { discoveryEpic } from './discovery'
 import { robotApiEpic } from './robot-api'
+import { shellEpic } from './shell'
 
-export default combineEpics(buildrootUpdateEpic, discoveryEpic, robotApiEpic)
+export default combineEpics(discoveryEpic, robotApiEpic, shellEpic)

app/src/index.js

@@ -10,7 +10,7 @@ import { ConnectedRouter, routerMiddleware } from 'connected-react-router'
 import { createEpicMiddleware } from 'redux-observable'
 
 import createLogger from './logger'
-import { checkShellUpdate, shellMiddleware } from './shell'
+import { checkShellUpdate } from './shell'
 
 import { apiClientMiddleware as robotApiMiddleware } from './robot'
 import { initializeAnalytics, analyticsMiddleware } from './analytics'
@@ -31,7 +31,6 @@ const middleware = applyMiddleware(
   thunk,
   epicMiddleware,
   robotApiMiddleware(),
-  shellMiddleware,
   analyticsMiddleware,
   supportMiddleware,
   routerMiddleware(history)

app/src/shell/__mocks__/remote.js

@@ -2,23 +2,20 @@
 // keep in sync with app-shell/src/preload.js
 'use strict'
 
+const EventEmitter = require('events')
+
+class MockIpcRenderer extends EventEmitter {
+  send = jest.fn()
+}
+
 module.exports = {
-  ipcRenderer: {
-    on: jest.fn(),
-    send: jest.fn(),
-  },
+  ipcRenderer: new MockIpcRenderer(),
   apiUpdate: {
     getUpdateInfo: jest.fn(),
     getUpdateFileContents: jest.fn(),
   },
-  config: {
-    getConfig: jest.fn(),
-  },
-  discovery: {
-    getRobots: jest.fn(),
-  },
-  update: {
-    CURRENT_VERSION: '0.0.0',
-    CURRENT_RELEASE_NOTES: 'Release notes for 0.0.0',
-  },
+  CURRENT_VERSION: '0.0.0',
+  CURRENT_RELEASE_NOTES: 'Release notes for 0.0.0',
+  INITIAL_CONFIG: {},
+  INITIAL_ROBOTS: [],
 }