July 28th, 2022

(hidden) l7Tq2Dibk

{{[[TODO]]}} (hidden) N4xSt36b2 [[(hidden) BC3_6P1lo]]

(hidden) 831F8QWHJ

(hidden) sOS5s93aR

(hidden) fVCKGGr-i

https://stately.ai/blog/redux-is-half-of-a-pattern-12

```patch From 0950207cf20e4fb94ba7ebb97ef278d9adbf7b79 Mon Sep 17 00:00:00 2001 From: Kipras Melnikovas <kipras@kipras.org> Date: Fri, 29 Jul 2022 00:42:25 +0300 Subject: [PATCH] refactor FileUpload logic - avoid impossible states (& make code much cleaner) classic. i notice that we have `uploadState` JSX, which has content if `uploadError &&` _and_ `loading &&`, meaning it's possible to achieve an "impossible state", when both `uploadError` and `loading` are `true`. one could say that "we ensure that both of these are never true", but that's irrelevant, both because 1. it's possible code will change in the future thus it's easier to mess this up, 2. we can do better this is where state machines & flow charts (xstate et al) come into play. but for starters, a reducer will do (it's half the pattern, see [1], but will be enough since here we have a simple use case). [1] https://stately.ai/blog/redux-is-half-of-a-pattern-12 Signed-off-by: Kipras Melnikovas <kipras@kipras.org> --- client/api/hooks/use-organization-files.tsx | 58 ++++++++++++------- .../elements/account-files/account-files.tsx | 41 ++++++------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/client/api/hooks/use-organization-files.tsx b/client/api/hooks/use-organization-files.tsx index 5357e21..2a6c34a 100644 --- a/client/api/hooks/use-organization-files.tsx +++ b/client/api/hooks/use-organization-files.tsx @@ -1,4 +1,4 @@ -import React, { createContext, useContext, useEffect, useState } from 'react'; +import React, { createContext, Reducer, useContext, useEffect, useReducer } from 'react'; import { useQuery } from 'react-query'; import { useFilePicker } from 'use-file-picker'; @@ -22,12 +22,21 @@ export const deleteFileMutation = async (fileId: number, orgId: number) => { invalidateFiles(orgId); }; +export type FileUploadStatus = + | { type: 'idle' } + | { type: 'loading'; progress: number } + | { type: 'error'; error: string } + | { type: 'success' }; + +export type FileUploadReducer = Reducer<FileUploadStatus, FileUploadStatus>; +const fileUploadReducer: FileUploadReducer = (_prevState, action) => { + return action; +}; + const FileContext = createContext<{ openFileSelector: () => void; - error: string; - loading: boolean; - uploadProgress: number; - setError: (s: string) => void; + uploadStatus: FileUploadStatus; + clearUploadStatus: () => void; } | null>(null); export const useOpenAndUploadOrganizationFile = () => { @@ -44,44 +53,51 @@ export const FileProvider: React.FC<{ orgId: number }> = ({ orgId, children }) = readAs: 'ArrayBuffer', maxFileSize: 50, }); - const [loading, setLoading] = useState(false); - const [uploadProgress, setUploadProgress] = useState(0); - const [uploadError, setUploadError] = useState(''); + + const [uploadStatus, dispatchUploadStatus] = useReducer<FileUploadReducer>( + fileUploadReducer, // + { + type: 'idle', + }, + ); useEffect(() => { if (errors[0]?.fileSizeToolarge) { - setUploadError('File is too large. Maximum upload file size: 50 MB.'); + dispatchUploadStatus({ + type: 'error', + error: 'File is too large. Maximum upload file size: 50 MB.', + }); } }, [errors]); useEffect(() => { const file = filesContent[0]; if (file) { - setLoading(true); - setUploadError(''); + dispatchUploadStatus({ type: 'loading', progress: 0 }); api.file - .uploadFile({ orgId, file, onUploadProgress: setUploadProgress }) + .uploadFile({ + orgId, + file, + onUploadProgress: (progress) => + dispatchUploadStatus({ type: 'loading', progress }), + }) .then(() => { + dispatchUploadStatus({ type: 'success' }); invalidateFiles(orgId); - setLoading(false); - setUploadProgress(0); }) .catch(() => { - setLoading(false); - setUploadProgress(0); - setUploadError('Something went wrong!'); + dispatchUploadStatus({ type: 'error', error: 'Something went wrong!' }); }); } }, [filesContent, orgId]); + return ( <FileContext.Provider value={{ openFileSelector, - loading, - uploadProgress, - error: uploadError, - setError: setUploadError, + uploadStatus, + clearUploadStatus: () => dispatchUploadStatus({ type: 'idle' }), }} > {children} diff --git a/client/view/elements/account-files/account-files.tsx b/client/view/elements/account-files/account-files.tsx index 8d3e9e5..e124005 100644 --- a/client/view/elements/account-files/account-files.tsx +++ b/client/view/elements/account-files/account-files.tsx @@ -11,6 +11,7 @@ import { } from '../../../api/hooks/use-organization-files'; import { File } from '../../../api/types'; import { fakeTranslator } from '../../../application-state/translator/fake-translator'; +import { assertNever } from '../../../tools/assert-never'; import { Box } from '../../primitives'; import { ErrorFallback } from '../../primitives/error-fallback'; import { ItemWrapperStack } from '../../primitives/item-wrapper'; @@ -28,11 +29,9 @@ interface Props { export const AccountFiles: FC<Props> = ({ files, isError, isLoading, orgId }) => { const lastFileUploaded = fakeTranslator.gettextFake('Last file uploaded'); const { - openFileSelector, - uploadProgress, - loading, - setError, - error: uploadError, + openFileSelector, // + uploadStatus, + clearUploadStatus, } = useOpenAndUploadOrganizationFile(); const fileHandlers = useMemo( @@ -55,22 +54,20 @@ export const AccountFiles: FC<Props> = ({ files, isError, isLoading, orgId }) => return <ErrorFallback refetch={() => invalidateFiles(orgId)} />; } - const uploadState = ( - <> - {uploadError && ( - <Spacing bottom="m"> - <Message variant={'negative'} visible onClose={() => setError('')}> - {uploadError} - </Message> - </Spacing> - )} - {loading && ( - <Spacing bottom="m"> - <Progressbar percent={uploadProgress} variant={'positive'} /> - </Spacing> - )} - </> - ); + const uploadState = + uploadStatus.type === 'error' ? ( + <Spacing bottom="m"> + <Message variant={'negative'} visible onClose={clearUploadStatus}> + {uploadStatus.error} + </Message> + </Spacing> + ) : uploadStatus.type === 'loading' ? ( + <Spacing bottom="m"> + <Progressbar percent={uploadStatus.progress} variant={'positive'} /> + </Spacing> + ) : uploadStatus.type === 'idle' || uploadStatus.type === 'success' ? null : ( + assertNever(uploadStatus) + ); if (files.length === 0) { return ( @@ -84,7 +81,7 @@ export const AccountFiles: FC<Props> = ({ files, isError, isLoading, orgId }) => { <Spacing top="m"> <Button - disabled={loading} + disabled={uploadStatus.type === 'loading'} aria-label="file" offset="left" size="m" -- 2.37.1 ```

fwiw, instead of renaming every prop, might as well not destructure

{{[[TODO]]}} not sure if i like the naming yet. seems opposite to what it should be (in vs out)

{{[[TODO]]}} (hidden) o2zGieZFY

(hidden) jlSn_rrrr

(hidden) GBFBPhoF8

(hidden) 1R7OoRiDQ

pretty similar to reflog, tho u get to take metadata __at the time that you perform the checkout__, which could prove more useful instead of trying to infer from the reflog

(hidden) vCEfh8jyI

(hidden) Ih-d9cZwl

19ish [[(hidden) 1emn3OovN]]

21 [[(hidden) heJsCKvBw]], [[(hidden) 3OrRyssuz]] getting worse, is masked / is riding the benefit wave of [[(hidden) sr-m_VdZk]] improvements

ewww

i think yes but only if we need to do some in-between processing?