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

refactor(app): Implement modules API-client with redux-observable #3395

Merged
merged 5 commits into from
May 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type DP = {| fetchModules: () => mixed |}
type Props = { ...OP, ...SP, ...DP }

const TITLE = 'Modules'
const POLL_MODULE_INTERVAL_MS = 2000
const POLL_MODULE_INTERVAL_MS = 5000

export default connect<Props, OP, SP, DP, State, Dispatch>(
mapStateToProps,
Expand Down
6 changes: 6 additions & 0 deletions app/src/epic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @flow
// root application epic
import { combineEpics } from 'redux-observable'
import { robotApiEpic } from './robot-api'

export default combineEpics(robotApiEpic)
8 changes: 4 additions & 4 deletions app/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import { createEpicMiddleware } from 'redux-observable'
import createLogger from './logger'
import { checkShellUpdate, shellMiddleware } from './shell'

import { robotApiEpic } from './robot-api'
import { apiClientMiddleware as robotApiMiddleware } from './robot'
import { initializeAnalytics, analyticsMiddleware } from './analytics'
import { initializeSupport, supportMiddleware } from './support'
import { startDiscovery, discoveryMiddleware } from './discovery'

import reducer from './reducer'
import rootReducer from './reducer'
import rootEpic from './epic'

// components
import App from './components/App'
Expand All @@ -44,9 +44,9 @@ const composeEnhancers =
window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ maxAge: 200 })) ||
compose

const store = createStore(reducer, composeEnhancers(middleware))
const store = createStore(rootReducer, composeEnhancers(middleware))

epicMiddlware.run(robotApiEpic)
epicMiddlware.run(rootEpic)

const renderApp = () =>
ReactDom.render(
Expand Down
88 changes: 69 additions & 19 deletions app/src/robot-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,47 +25,74 @@ Before adding client state for an endpoint, either identify or create a file in
- Most endpoints can hopefully use the default endpoint epic

```js
import { createBaseRequestEpic, GET } from '../utils'
// app/src/robot-api/resources/my-thing.js

import { createBaseRequestEpic } from '../utils'
// ...
const MY_THING_PATH = '/thing/:id'
const myThingEpic = createBaseRequestEpic(GET, MY_THING_PATH)

// call createBaseRequestEpic w/ the action type that should trigger the epic
export const FETCH_MY_THING: 'robotHttp:FETCH_MY_THING' =
'robotHttp:FETCH_MY_THING'

export const fetchMyThingEpic = createBaseRequestEpic(FETCH_MY_THING)
```

- See `setTargetTempEpic` in `app/src/robot-api/resources/modules.js` for an example of an epic that needs more than the base functionality and layers that logic on top of the base

2. Add the epic to the exported combined epic (or create the combined epic if it doesn't exist)

```js
export const allThingsEpic = combineEpics(
myThingEpic
// app/src/robot-api/resources/my-thing.js

export const myThingEpic = combineEpics(
fetchMyThingEpic
// ...
)
```

3. Add an action creator to trigger the epic

```js
// app/src/robot-api/types.js

// ...
export type ApiAction =
| {| type: 'robotApi:FETCH_HEALTH', payload: ApiRequest |}
| {| type: 'robotApi:FETCH_MY_THING', payload: ApiRequest |}
```

```js
// app/src/robot-api/resources/my-thing.js

import { apiCall, GET } from '../utils'
import type { RobotHost } from '../types'
import type { RobotHost, ApiAction } from '../types'
// ...
export const fetchMyThing = (host: RobotHost, id: string) =>
apiCall({ host, method: GET, path: `/thing/${id}` })
export const fetchMyThing = (host: RobotHost, id: string): ApiAction => ({
type: FETCH_MY_THING,
payload: { host, method: GET, path: `/my-thing/${id}` },
})
```

4. Add any necessary logic to the reducer to handle your action

```js
import type { ApiAction, ThingState as State } from '../types'
import pathToRegexp from 'path-to-regexp'
import { passResponseAction } from '../utils'
// NOTE: be sure to define any necessary model types in types.js
import type { ApiActionLike, ThingState as State } from '../types'

// ...
const INITIAL_STATE: State = {}
const RE_THING_PATH = pathToRegexp(MY_THING_PATH)
const MY_THING_PATH = '/my-thing/:id'
const RE_MY_THING_PATH = pathToRegexp(MY_THING_PATH)

export function thingReducer(
export function myThingReducer(
state: State = INITIAL_STATE,
action: ApiAction
action: ApiActionLike
): State {
if (action.type === API_RESPONSE) {
const response = passResponseAction(action)

if (response) {
const { path, body } = action.payload
const thingIdMatch = path.match(RE_THING_PATH)

Expand All @@ -81,7 +108,25 @@ Before adding client state for an endpoint, either identify or create a file in
}
```

5. If epic and/or reducer were new, add them to the overall resource epic and reducer in `app/src/robot-api/resources/index.js`
5. If epic and/or reducer were new, add them to the overall resource epic and reducer

```js
// app/src/robot-api/resources/index.js

import {myThingReducer, myThingEpic} from './resources'

export const resourcesReducer = combineReducers<_, ApiActionLike>({
// ...
myThing: myThingReducer,
// ...
})

export const robotApiEpic = combineEpics(
// ...
myThingEpic
// ...
)
```

6. Add any necessary selectors

Expand All @@ -91,21 +136,26 @@ Before adding client state for an endpoint, either identify or create a file in

// ...

export function getResourceState(
export function getMyThingState(
state: AppState,
robotName: string
): ThingState | null {
const robotState = getRobotApiState(state, robotName)

return = robotState?.resources.thing || null
return = robotState?.resources.myThing || null
}
```

### note about response Flow types
### note about Flow types

Right now, our types for the HTTP response objects are non-existent. One of the misadventures with `http-api-client` was trying to strictly type all responses with Flow. It turns out this was hard, probably slowed Flow to a crawl, and didn't really get us anywhere.
In the interest of action inspectability and overall DX, internal API lifecycle objects and actions are very loosely typed. Things to watch out for:

For now, HTTP responses are typed as `any`, to be cast in the reducer. In the future, we should look into runtime schema checking to verify response shape before we cast to an actual type.
- Response body's (`responseAction.payload.body`) are currently typed as `any`, which means you'll need to cast them
- In the furture this casting should be the job of functions that also check response schemas
- rxjs operators need to be explicitly typed
- Inside epics and reducers, we're usually throwing around `ApiActionLike`s, which have `type: string` rather than string constants
- This is so we can append method and path information to action types so the Redux devtools are actually usable
- See `passRequestAction`, `passResponseAction`, and `passErrorAction` in `app/src/robot-api/utils.js` for help casting/filtering an `ApiActionLike` to something more specific

## background

Expand Down
8 changes: 4 additions & 4 deletions app/src/robot-api/fetch.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
// @flow
// simple fetch wrapper to format URL for robot and parse JSON response
import { of } from 'rxjs'
import { of as observableOf } from 'rxjs'
mcous marked this conversation as resolved.
Show resolved Hide resolved
import { fromFetch } from 'rxjs/fetch'
import { timeout, switchMap, catchError } from 'rxjs/operators'

import type { Observable } from 'rxjs'
import type { ApiCall, ApiResponse } from './types'
import type { ApiRequest, ApiResponse } from './types'

const DEFAULT_TIMEOUT_MS = 30000

export default function fetchWrapper(
call: ApiCall,
call: ApiRequest,
mcous marked this conversation as resolved.
Show resolved Hide resolved
timeoutMs: number = DEFAULT_TIMEOUT_MS
): Observable<ApiResponse> {
const { host, path, method, body: reqBody } = call
Expand All @@ -34,7 +34,7 @@ export default function fetchWrapper(
}))
),
catchError(error =>
of({
observableOf({
host,
path,
method,
Expand Down
56 changes: 29 additions & 27 deletions app/src/robot-api/reducer.js
Original file line number Diff line number Diff line change
@@ -1,47 +1,49 @@
// @flow
// api state reducer
// tracks resource and networking state per robot
import { API_CALL, API_RESPONSE, API_ERROR } from './utils'
import {
passApiAction,
passRequestAction,
passResponseAction,
passErrorAction,
} from './utils'
import { resourcesReducer } from './resources'

import type { Action } from '../types'
import type { NetworkingInstanceState, ApiState, ApiAction } from './types'
import type { ApiActionLike, ApiState, NetworkingInstanceState } from './types'

const INITIAL_INSTANCE_STATE = { networking: {}, resources: {} }

function networkingReducer(
state: NetworkingInstanceState = INITIAL_INSTANCE_STATE.networking,
action: ApiAction
action: ApiActionLike
): NetworkingInstanceState {
switch (action.type) {
case API_CALL: {
return { ...state, [action.payload.path]: { inProgress: true } }
}

case API_RESPONSE:
case API_ERROR: {
return { ...state, [action.payload.path]: { inProgress: false } }
}
const request = passRequestAction(action)
const response = passResponseAction(action) || passErrorAction(action)
let nextState = state

if (request) {
nextState = { ...state, [request.payload.path]: { inProgress: true } }
} else if (response) {
nextState = { ...state, [response.payload.path]: { inProgress: false } }
}

return state
return nextState
}

function robotApiReducer(state: ApiState = {}, action: Action): ApiState {
switch (action.type) {
case API_CALL:
case API_RESPONSE:
case API_ERROR: {
const { name } = action.payload.host
const stateByName = state[name] || INITIAL_INSTANCE_STATE

return {
...state,
[name]: {
networking: networkingReducer(stateByName.networking, action),
resources: resourcesReducer(stateByName.resources, action),
},
}
const apiAction = passApiAction(action)

if (apiAction) {
const { name } = apiAction.payload.host
const stateByName = state[name] || INITIAL_INSTANCE_STATE

return {
...state,
[name]: {
networking: networkingReducer(stateByName.networking, apiAction),
resources: resourcesReducer(stateByName.resources, apiAction),
},
}
}

Expand Down
23 changes: 14 additions & 9 deletions app/src/robot-api/resources/health.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,29 @@
import {
getRobotApiState,
createBaseRequestEpic,
apiCall,
passResponseAction,
GET,
API_RESPONSE,
} from '../utils'

import type { State as AppState } from '../../types'
import type { ActionLike, State as AppState } from '../../types'
import type { ApiAction, RobotHost, HealthState as State } from '../types'

export const FETCH_HEALTH: 'robotHttp:FETCH_HEALTH' = 'robotHttp:FETCH_HEALTH'

export const HEALTH_PATH = '/health'

export const fetchHealth = (host: RobotHost) =>
apiCall({ method: GET, path: HEALTH_PATH, host })
export const fetchHealth = (host: RobotHost): ApiAction => ({
type: FETCH_HEALTH,
payload: { host, method: GET, path: HEALTH_PATH },
})

export const healthEpic = createBaseRequestEpic(FETCH_HEALTH)

export const healthEpic = createBaseRequestEpic(GET, HEALTH_PATH)
export function healthReducer(state: State = null, action: ActionLike): State {
const response = passResponseAction(action)

export function healthReducer(state: State = null, action: ApiAction): State {
if (action.type === API_RESPONSE && action.payload.path === HEALTH_PATH) {
return ((action.payload.body: any): State)
if (response && response.payload.path === HEALTH_PATH) {
return ((response.payload.body: any): State)
}

return state
Expand Down
4 changes: 2 additions & 2 deletions app/src/robot-api/resources/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import { combineEpics } from 'redux-observable'
import { healthReducer, healthEpic } from './health'
import { modulesReducer, modulesEpic } from './modules'

import type { ApiAction } from '../types'
import type { ApiActionLike } from '../types'

export * from './health'
export * from './modules'

export const resourcesReducer = combineReducers<_, ApiAction>({
export const resourcesReducer = combineReducers<_, ApiActionLike>({
health: healthReducer,
modules: modulesReducer,
})
Expand Down
Loading