Skip to content

Commit

Permalink
refactor(dashboard): pass deps to useEffect hook
Browse files Browse the repository at this point in the history
Previously we passed an empty array as a dependency to the useEffect
hooks that were responsible for data fetching to ensure they only run
once on mount. This is considered a bad practice so now we've refactored the
data fetching logic so that we can explicitly pass all dependencies.

We do this by removing the action creators and calling the load
functions directly from the container components.
  • Loading branch information
eysi09 committed Sep 29, 2019
1 parent 65e4d0f commit 2f291ec
Show file tree
Hide file tree
Showing 14 changed files with 451 additions and 516 deletions.
4 changes: 2 additions & 2 deletions dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@
"uuid": "^3.3.2"
},
"scripts": {
"start": "CI=true react-scripts start",
"start": "react-scripts start",
"build": "react-scripts build && ./bin/copy-to-static",
"dev": "EXTEND_ESLINT=true npm run start",
"dev": "EXTEND_ESLINT=true CI=true npm run start",
"test-watch": "react-scripts test",
"test": "CI=true react-scripts test",
"eject": "react-scripts eject"
Expand Down
260 changes: 260 additions & 0 deletions dashboard/src/api/actions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
/*
* Copyright (C) 2018 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import produce from "immer"
import { groupBy } from "lodash"

import { ServiceLogEntry } from "garden-service/build/src/types/plugin/service/getServiceLogs"
import { StatusCommandResult } from "garden-service/build/src/commands/get/get-status"
import { TaskResultOutput } from "garden-service/build/src/commands/get/get-task-result"
import { ConfigDump } from "garden-service/build/src/garden"
import { TestResultOutput } from "garden-service/build/src/commands/get/get-test-result"
import { GraphOutput } from "garden-service/build/src/commands/get/get-graph"
import {
Entities,
Module,
Service,
Task,
Test,
ApiDispatch,
} from "../contexts/api"
import {
fetchLogs,
fetchStatus,
fetchTaskResult,
fetchConfig,
fetchTestResult,
fetchGraph,
FetchTaskResultParams,
FetchTestResultParams,
} from "./api"

/**
* This file contains the API action functions.
*
* The actions are responsible for dispatching the appropriate action types and normalising the
* API response.
*/

export async function loadConfig(dispatch: ApiDispatch) {
const requestKey = "config"

dispatch({ requestKey, type: "fetchStart" })
let res: ConfigDump

try {
res = await fetchConfig()
} catch (error) {
dispatch({ requestKey, type: "fetchFailure", error })
return
}

const processResults = (entities: Entities) => processConfig(entities, res)

dispatch({ type: "fetchSuccess", requestKey, processResults })
}

function processConfig(entities: Entities, config: ConfigDump) {
let modules: { [moduleName: string]: Module } = {}
let services: { [serviceName: string]: Service } = {}
let tasks: { [taskName: string]: Task } = {}
let tests: { [testKey: string]: Test } = {}

for (const cfg of config.moduleConfigs) {

const module: Module = {
name: cfg.name,
type: cfg.type,
path: cfg.path,
repositoryUrl: cfg.repositoryUrl,
description: cfg.description,
services: cfg.serviceConfigs.map(service => service.name),
tests: cfg.testConfigs.map(test => `${cfg.name}.${test.name}`),
tasks: cfg.taskConfigs.map(task => task.name),
taskState: "taskComplete",
}
modules[cfg.name] = module
for (const serviceConfig of cfg.serviceConfigs) {
services[serviceConfig.name] = {
...services[serviceConfig.name],
config: serviceConfig,
}
}
for (const testConfig of cfg.testConfigs) {
const testKey = `${cfg.name}.${testConfig.name}`
tests[testKey] = {
...tests[testKey],
config: testConfig,
}
}
for (const taskConfig of cfg.taskConfigs) {
tasks[taskConfig.name] = {
...tasks[taskConfig.name],
config: taskConfig,
}
}
}

return produce(entities, draft => {
draft.modules = modules
draft.services = services
draft.tests = tests
draft.tasks = tasks
draft.project.root = config.projectRoot
})
}

export async function loadLogs(dispatch: ApiDispatch, serviceNames: string[]) {
const requestKey = "logs"

dispatch({ requestKey, type: "fetchStart" })

let res: ServiceLogEntry[]
try {
res = await fetchLogs({ serviceNames })
} catch (error) {
dispatch({ requestKey, type: "fetchFailure", error })
return
}

const processResults = (entities: Entities) => processLogs(entities, res)

dispatch({ type: "fetchSuccess", requestKey, processResults })
}

function processLogs(entities: Entities, logs: ServiceLogEntry[]) {
return produce(entities, draft => {
draft.logs = groupBy(logs, "serviceName")
})
}

export async function loadStatus(dispatch: ApiDispatch) {
const requestKey = "status"

dispatch({ requestKey, type: "fetchStart" })

let res: StatusCommandResult
try {
res = await fetchStatus()
} catch (error) {
dispatch({ requestKey, type: "fetchFailure", error })
return
}

const processResults = (entities: Entities) => processStatus(entities, res)

dispatch({ type: "fetchSuccess", requestKey, processResults })
}

function processStatus(entities: Entities, status: StatusCommandResult) {
return produce(entities, draft => {
for (const serviceName of Object.keys(status.services)) {
draft.services[serviceName] = {
...draft.services[serviceName],
status: status.services[serviceName],
}
}
for (const testName of Object.keys(status.tests)) {
draft.tests[testName] = {
...draft.tests[testName],
status: status.tests[testName],
}
}
for (const taskName of Object.keys(status.tasks)) {
draft.tasks[taskName] = {
...draft.tasks[taskName],
status: status.tasks[taskName],
}
}
draft.providers = status.providers
})
}

interface LoadTaskResultParams extends FetchTaskResultParams {
dispatch: ApiDispatch
}

export async function loadTaskResult({ dispatch, ...fetchParams }: LoadTaskResultParams) {
const requestKey = "taskResult"

dispatch({ requestKey, type: "fetchStart" })

let res: TaskResultOutput
try {
res = await fetchTaskResult(fetchParams)
} catch (error) {
dispatch({ requestKey, type: "fetchFailure", error })
return
}

const processResults = (entities: Entities) => processTaskResult(entities, res)

dispatch({ type: "fetchSuccess", requestKey, processResults })
}

function processTaskResult(entities: Entities, result: TaskResultOutput) {
return produce(entities, draft => {
draft.tasks = draft.tasks || {}
draft.tasks[result.name] = draft.tasks[result.name] || {}
draft.tasks[result.name].result = result
})
}

interface LoadTestResultParams extends FetchTestResultParams {
dispatch: ApiDispatch
}

export async function loadTestResult({ dispatch, ...fetchParams }: LoadTestResultParams) {
const requestKey = "testResult"

dispatch({ requestKey, type: "fetchStart" })

let res: TestResultOutput
try {
res = await fetchTestResult(fetchParams)
} catch (error) {
dispatch({ requestKey, type: "fetchFailure", error })
return
}

const processResults = (entities: Entities) => processTestResult(entities, res)

dispatch({ type: "fetchSuccess", requestKey, processResults })
}

function processTestResult(entities: Entities, result: TestResultOutput) {
return produce(entities, draft => {
draft.tests = draft.tests || {}
draft.tests[result.name] = draft.tests[result.name] || {}
draft.tests[result.name].result = result
})
}

export async function loadGraph(dispatch: ApiDispatch) {
const requestKey = "graph"

dispatch({ requestKey, type: "fetchStart" })

let res: GraphOutput
try {
res = await fetchGraph()
} catch (error) {
dispatch({ requestKey, type: "fetchFailure", error })
return
}

const processResults = (entities: Entities) => processGraph(entities, res)

dispatch({ type: "fetchSuccess", requestKey, processResults })
}

function processGraph(entities: Entities, graph: GraphOutput) {
return produce(entities, draft => {
draft.graph = graph
})
}
37 changes: 19 additions & 18 deletions dashboard/src/contexts/ws-handlers.ts → dashboard/src/api/ws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import { ServerWebsocketMessage } from "garden-service/build/src/server/server"
import { Events } from "garden-service/build/src/events"

import {
Store,
Entities,
Action,
SupportedEventName,
supportedEventNames,
} from "./api"
import getApiUrl from "../api/get-api-url"
} from "../contexts/api"
import getApiUrl from "./get-api-url"
import produce from "immer"

export type WsEventMessage = ServerWebsocketMessage & {
Expand Down Expand Up @@ -47,8 +47,8 @@ export function initWebSocket(dispatch: React.Dispatch<Action>) {
console.error(parsedMsg)
}
if (isSupportedEvent(parsedMsg)) {
const produceNextStore = (store: Store) => processWebSocketMessage(store, parsedMsg)
dispatch({ type: "wsMessageReceived", produceNextStore })
const processResults = (store: Entities) => processWebSocketMessage(store, parsedMsg)
dispatch({ type: "wsMessageReceived", processResults })
}
}
return function cleanUp() {
Expand All @@ -57,46 +57,47 @@ export function initWebSocket(dispatch: React.Dispatch<Action>) {
}

// Process the graph response and return a normalized store
function processWebSocketMessage(store: Store, message: WsEventMessage) {
function processWebSocketMessage(store: Entities, message: WsEventMessage) {
const taskType = message.payload["type"] === "task" ? "run" : message.payload["type"] // convert "task" to "run"
const taskState = message.name
const entityName = message.payload["name"]
return produce(store, storeDraft => {
return produce(store, draft => {
// We don't handle taskGraphComplete events
if (taskType && taskState !== "taskGraphComplete") {
storeDraft.requestStates.fetchTaskStates.loading = true
draft.project.taskGraphProcessing = true
switch (taskType) {
case "publish":
break
case "deploy":
storeDraft.entities.services[entityName] = {
...storeDraft.entities.services[entityName],
draft.services[entityName] = {
...draft.services[entityName],
taskState,
}
break
case "build":
storeDraft.entities.modules[entityName] = {
...store.entities.modules[entityName],
draft.modules[entityName] = {
...store.modules[entityName],
taskState,
}
break
case "run":
storeDraft.entities.tasks[entityName] = {
...store.entities.tasks[entityName],
draft.tasks[entityName] = {
...store.tasks[entityName],
taskState,
}
break
case "test":
storeDraft.entities.tests[entityName] = {
...store.entities.tests[entityName],
draft.tests[entityName] = {
...store.tests[entityName],
taskState,
}
break
}
}

if (taskState === "taskGraphComplete") { // add to requestState graph whenever its taskGraphComplete
storeDraft.requestStates.fetchTaskStates.loading = false
// add to requestState graph whenever its taskGraphComplete
if (taskState === "taskGraphComplete") {
draft.project.taskGraphProcessing = false
}
})
}
6 changes: 3 additions & 3 deletions dashboard/src/components/logs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ import Select from "react-select"
import Terminal from "./terminal"
import Card, { CardTitle } from "./card"
import { colors } from "../styles/variables"
import { LoadLogs } from "../contexts/api"

import { ServiceLogEntry } from "garden-service/build/src/types/plugin/service/getServiceLogs"
import { ActionIcon } from "./action-icon"

interface Props {
logs: { [serviceName: string]: ServiceLogEntry[] }
onRefresh: LoadLogs
onRefresh: (serviceNames: string[]) => void
}

interface State {
Expand Down Expand Up @@ -56,6 +55,7 @@ const selectStyles = {
}),
}

// TODO: Use functional component
class Logs extends Component<Props, State> {

constructor(props) {
Expand Down Expand Up @@ -85,7 +85,7 @@ class Logs extends Component<Props, State> {
if (!serviceNames.length) {
return
}
this.props.onRefresh({ serviceNames, force: true })
this.props.onRefresh(serviceNames)
this.setState({ loading: true })
}

Expand Down
Loading

0 comments on commit 2f291ec

Please sign in to comment.