useAsync hook for create instance POST #245
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| export { useBreadcrumbs } from './use-breadcrumbs' | ||
| export { useAsync } from './use-async' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { renderHook, act } from '@testing-library/react-hooks' | ||
| import { useAsync } from './use-async' | ||
|
|
||
| describe('useAsync', () => { | ||
| it('starts with null data, null error, and pending false', () => { | ||
| const { result } = renderHook(() => useAsync(() => Promise.resolve(1))) | ||
|
|
||
| expect(result.current.data).toBeNull() | ||
| expect(result.current.error).toBeNull() | ||
| expect(result.current.pending).toBe(false) | ||
| }) | ||
|
|
||
| it('with promise in flight, has pending true, null error, and null data', async () => { | ||
| const { result, waitForNextUpdate } = renderHook(() => | ||
| useAsync(() => Promise.resolve(1)) | ||
| ) | ||
| act(() => { | ||
| result.current.run() | ||
| }) | ||
|
|
||
| expect(result.current.pending).toBe(true) | ||
| expect(result.current.data).toBeNull() | ||
| expect(result.current.error).toBeNull() | ||
|
|
||
| await waitForNextUpdate() | ||
|
There was a problem hiding this comment. This |
||
| }) | ||
|
|
||
| it('after promise resolves, has data, null error, and pending false', async () => { | ||
| const { result, waitForNextUpdate } = renderHook(() => | ||
| useAsync(() => Promise.resolve(1)) | ||
| ) | ||
| act(() => { | ||
| result.current.run() | ||
| }) | ||
| await waitForNextUpdate() | ||
|
|
||
| expect(result.current.data).toBe(1) | ||
| expect(result.current.error).toBeNull() | ||
| expect(result.current.pending).toBe(false) | ||
| }) | ||
|
|
||
| it('after promise rejects, has error, null data, and pending false', async () => { | ||
| const { result, waitForNextUpdate } = renderHook(() => | ||
| useAsync(() => Promise.reject(1)) | ||
| ) | ||
| act(() => { | ||
| result.current.run() | ||
| }) | ||
| await waitForNextUpdate() | ||
|
|
||
| expect(result.current.error).toBe(1) | ||
| expect(result.current.data).toBeNull() | ||
| expect(result.current.pending).toBe(false) | ||
| }) | ||
| }) | ||
|
There was a problem hiding this comment. Unlike the pending one, this one could actually be done like this: const { result } = renderHook(() => useAsync(() => Promise.reject(1)))
await act(result.current.execute)However, understanding the difference requires a pretty deep understanding of |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { useState } from 'react' | ||
|
|
||
| // TODO: this is what the API currently response with for 400s, but it may not | ||
| // cover all API errors, and it definitely doesn't cover places where we might | ||
| // use useAsync for something other than a POST to our own API | ||
| interface ResponseError { | ||
| request_id: string | ||
| message: string | ||
| error_code: number | null | ||
| } | ||
|
|
||
| interface AsyncResult<R> { | ||
| run: () => Promise<void> | ||
| pending: boolean | ||
| data: R | null | ||
| error: ResponseError | null | ||
| } | ||
|
|
||
| export function useAsync<R>(asyncFunction: () => Promise<R>): AsyncResult<R> { | ||
| const [pending, setPending] = useState(false) | ||
| const [data, setData] = useState<R | null>(null) | ||
| const [error, setError] = useState<ResponseError | null>(null) | ||
|
|
||
| const run = async () => { | ||
| setPending(true) | ||
| setData(null) | ||
| setError(null) | ||
|
|
||
| try { | ||
| const response = await asyncFunction() | ||
| setData(response) | ||
| } catch (errorResponse) { | ||
| const error = | ||
| typeof errorResponse.json === 'function' | ||
| ? await errorResponse.json() | ||
| : errorResponse | ||
| setError(error) | ||
| } finally { | ||
| setPending(false) | ||
| } | ||
| } | ||
|
|
||
| return { run, pending, data, error } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,20 @@ | ||
| import type { ChangeEvent } from 'react' | ||
| import { useEffect } from 'react' | ||
| import React, { useState } from 'react' | ||
| import styled from 'styled-components' | ||
| import { useParams, useHistory } from 'react-router-dom' | ||
|
|
||
| import { | ||
| Breadcrumbs, | ||
| Button, | ||
| Icon, | ||
| PageHeader, | ||
| NumberField, | ||
| TextField, | ||
| TextWithIcon, | ||
| } from '@oxide/ui' | ||
| import { useBreadcrumbs, useAsync } from '../../hooks' | ||
| import { api } from '@oxide/api' | ||
|
|
||
| const Title = styled(TextWithIcon).attrs({ | ||
| text: { variant: 'title', as: 'h1' }, | ||
|
|
@@ -14,15 +26,82 @@ const Title = styled(TextWithIcon).attrs({ | |
| } | ||
| ` | ||
|
|
||
| const Box = styled.div` | ||
| border: 1px solid white; | ||
| padding: 1rem; | ||
| ` | ||
|
|
||
| const FormContainer = styled.div` | ||
| margin-top: ${({ theme }) => theme.spacing(4)}; | ||
| ${({ theme }) => theme.spaceBetweenY(4)} | ||
| ` | ||
|
|
||
| type Params = { | ||
| projectName: string | ||
| } | ||
|
|
||
| const InstancesPage = () => { | ||
| const breadcrumbs = useBreadcrumbs() | ||
|
|
||
| const history = useHistory() | ||
| const { projectName } = useParams<Params>() | ||
|
|
||
| // form state | ||
| const [instanceName, setInstanceName] = useState('') | ||
| const [ncpus, setNcpus] = useState(1) | ||
|
|
||
| const createInstance = useAsync(() => | ||
| api.apiProjectInstancesPost({ | ||
| projectName, | ||
| apiInstanceCreateParams: { | ||
| bootDiskSize: 1, | ||
| description: `An instance in project: ${projectName}`, | ||
| hostname: 'oxide.com', | ||
| memory: 10, | ||
| name: instanceName, | ||
| ncpus, | ||
| }, | ||
| }) | ||
| ) | ||
|
Comment on lines
+53
to
+65
There was a problem hiding this comment. a little worried we might run into a closure async issue with this pattern, as the values in state may have been updated and this closure not re-calculated before being sent off ... I would prefer to see the object currently being passed to There was a problem hiding this comment. I think the version of this I wrote at my last job worked that way. Looks like the react-async version lets you do it either way, which is interesting. Of the top of my head, I don't think it's a problem because I considered it doing it the other way, but what's nice about this way is it keeps the interface of |
||
|
|
||
| const onCreateClick = async () => { | ||
| // TODO: validate client-side before attempting post | ||
| await createInstance.run() | ||
| } | ||
|
|
||
| // redirect on successful post | ||
| useEffect(() => { | ||
| if (createInstance.data) { | ||
| history.push(`/projects/${projectName}/instances`) | ||
| } | ||
| }, [createInstance.data, history, projectName]) | ||
|
|
||
| return ( | ||
| <> | ||
| <Breadcrumbs data={breadcrumbs} /> | ||
| <PageHeader> | ||
| <Title>Create Instance</Title> | ||
| </PageHeader> | ||
| <FormContainer> | ||
| <Box>Post error: {JSON.stringify(createInstance.error)}</Box> | ||
| <TextField | ||
| value={instanceName} | ||
| required | ||
| onChange={(e: ChangeEvent<HTMLInputElement>) => | ||
| setInstanceName(e.target.value) | ||
| } | ||
| placeholder="db1" | ||
| > | ||
| Instance name | ||
| </TextField> | ||
| <NumberField value={ncpus} handleChange={setNcpus}> | ||
| Number of CPUs | ||
| </NumberField> | ||
|
|
||
| <Button onClick={onCreateClick} disabled={createInstance.pending}> | ||
| Create instance | ||
| </Button> | ||
| </FormContainer> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI because I fought with this quite a bit:
In order to catch the promise in-flight, this needs the braces, i.e., it cannot be
or
Because then it knows we're doing something async and warns that we have to put an
awaitin front like this:But that ruins this test, because if we wait for the promise to complete, it is no longer pending.