Fix CreateTeam forms UI problems and add field error parsing to Cyberstorm forms #917
Conversation
Oksamies
changed the title
| return new ApiError(`${response.status}: ${response.statusText}`); | ||
| return new ApiError( | ||
| `${response.status}: ${response.statusText}`, | ||
| await response.json() |
There was a problem hiding this comment.
You need to try-catch this, it might not be json
| } | ||
|
|
||
| getFieldErrors(): { [key: string]: string[] } { | ||
| // TODO: Implement | ||
| return {}; | ||
| return { ...this.response }; |
There was a problem hiding this comment.
Not guaranteed to exist if parsing is done properly
| @@ -3,17 +3,22 @@ export function isApiError(e: Error | ApiError | unknown): e is ApiError { | |||
| } | |||
|
|
|||
| export class ApiError extends Error { | |||
| constructor(message?: string) { | |||
| response: { [key: string]: string[] } | undefined; | |||
There was a problem hiding this comment.
Calling this responseData or such would be more appropriate. response would be appropriate if you stored the actual response object itself.
| ...(args.method === "GET" || args.method === "HEAD" || args.method === null | ||
| ? {} | ||
| : { body: args.body }), |
There was a problem hiding this comment.
This is wrong, whether a request body should be sent has nothing to do with the request method. It's just more common for that to be done with POST requests.
If having undefined in the body was causing issues, check if null works instead:
| ...(args.method === "GET" || args.method === "HEAD" || args.method === null | |
| ? {} | |
| : { body: args.body }), | |
| body: body ?? null, |
According to the typescript type hints at least, that should be valid.
There was a problem hiding this comment.
This suggestion isn't necessary if you follow the larger change suggestion, leaving it as a note anyway.
| @@ -16,15 +16,18 @@ export async function apiFetch2(args: apiFetchArgs) { | |||
| const url = getUrl(args.config, args.path, args.query); | |||
|
|
|||
| const response = await fetch(url, { | |||
| method: args.method ? args.method : "GET", | |||
There was a problem hiding this comment.
Don't do this, change the type hint for apiFetchArgs to require the type to be provided instead if you want to enforce it.
The best option would probably be to move both the method and body arguments under the same object and let the caller control any of the normal fetch parameters like so:
export type apiFetchArgs = {
config: RequestConfig;
path: string;
query?: string;
request?: Omit<RequestInit, "headers">;
};Then you can just merge it to the fetch call as-is:
const response = await fetch(url, {
...(args.request ?? {}),
headers: {
...BASE_HEADERS,
...getAuthHeaders(args.config),
},
});It should be the caller's responsibility to call the function correctly, and typescript's is used to make sure the caller knows what "correctly" means.
No description provided.