```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
```