From 65fd45fc16dcb10f6066b0a71b1e3a33d2a1ae48 Mon Sep 17 00:00:00 2001 From: Yura Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Fri, 15 May 2026 10:43:18 +0200 Subject: [PATCH] Introduce result type and generic safe fetch functions. Do proper error handling/display. --- src/App.tsx | 324 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 245 insertions(+), 79 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index d18676a..a4a3aac 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,5 +1,5 @@ import { z } from "zod" -import { useReducer, useRef, useEffect, createContext, useContext } from "react" +import { useReducer, useRef, useEffect, createContext, useContext, useState } from "react" import { CssBaseline, Box, @@ -12,12 +12,51 @@ import { // TODO: Improve error-handling. Introduce some proper server response. +type Result = { tag: "ok"; value: A } | { tag: "error"; error: E } + +const Result = { + ok(value: A): Result { + return { tag: "ok", value } + }, + err(error: E): Result { + return { tag: "error", error } + }, + map(result: Result, f: (x: A) => B): Result { + switch (result.tag) { + case "ok": + return { tag: "ok", value: f(result.value) } + case "error": + return { tag: "error", error: result.error } + } + }, +} + +type RequestError = + | { tag: "networkError"; error: unknown } + | { tag: "httpError"; status: number; statusText: string } + | { tag: "jsonParseError"; error: unknown } + | { tag: "invalidResponse"; error: z.ZodError } + +const RequestError = { + toString(error: RequestError): string { + switch (error.tag) { + case "networkError": + return "Network error. Check your connection and try again." + case "httpError": + return `Server returned ${String(error.status)} ${error.statusText}.` + case "jsonParseError": + return "The server returned an invalid response." + case "invalidResponse": + return "The image data had an unexpected format." + } + }, +} + // === Image === type ImageId = string type ImageRef = string // === Dimension === - type Dimension = { width: number height: number @@ -67,6 +106,69 @@ const Page = { }, } +// === Generic safe fetch functions === +async function fetchJsonSafe(url: string): Promise> { + let response: Response + try { + response = await fetch(url) + } catch (error: unknown) { + return Result.err({ tag: "networkError", error }) + } + + if (!response.ok) { + return Result.err({ + tag: "httpError", + status: response.status, + statusText: response.statusText, + }) + } else { + try { + const json: unknown = await response.json() + return Result.ok(json) + } catch (error: unknown) { + return Result.err({ tag: "jsonParseError", error }) + } + } +} + +// Assumes `f` is some sort of a zod parsing function that may throw `z.ZodError` +async function fetchJsonSafeWith( + url: string, + f: (json: unknown) => A, +): Promise> { + const result = await fetchJsonSafe(url) + switch (result.tag) { + case "error": + return Result.err(result.error) + case "ok": + // TODO + try { + return Result.ok(f(result.value)) + } catch (error: unknown) { + if (error instanceof z.ZodError) { + return Result.err({ tag: "invalidResponse", error }) + } else { + throw error + } + } + } +} + +// === Generic Remote Data === +type Remote = { tag: "loading" } | Result + +const Remote = { + loading(): Remote { + return { tag: "loading" } + }, + error(error: E): Remote { + return { tag: "error", error } + }, + loaded(value: A): Remote { + return { tag: "ok", value } + }, +} + // === api === const picsumApiImageSchema = z.object({ id: z.string(), @@ -78,23 +180,19 @@ const picsumApiImageSchema = z.object({ }) type PicsumApiImage = z.infer -async function getPicsumImages({ page, limit }: Page): Promise { - const response = await fetch( +async function getPicsumImages({ + page, + limit, +}: Page): Promise> { + return fetchJsonSafeWith( `https://picsum.photos/v2/list?limit=${String(limit)}&page=${String(page)}`, + (json) => z.array(picsumApiImageSchema).parse(json), ) - if (!response.ok) { - throw new Error(`Failed to fetch images: ${String(response.status)}`) - } - - const json: unknown = await response.json() - const data = z.array(picsumApiImageSchema).parse(json) - - return data } -async function getImageIds(page: Page): Promise { - const data = await getPicsumImages(page) - return data.map(({ id }) => id) +async function getImageIds(page: Page): Promise> { + const result = await getPicsumImages(page) + return Result.map(result, (data) => data.map(({ id }) => id)) } // Use this for `` @@ -102,29 +200,15 @@ function getImageSource(id: ImageId, dimension: Dimension): ImageRef { return `https://picsum.photos/id/${id}/${String(dimension.width)}/${String(dimension.height)}` } -// === Generic Remote Data === -type Remote = { tag: "loading" } | { tag: "error"; error: E } | { tag: "loaded"; value: A } - -const Remote = { - loading(): Remote { - return { tag: "loading" } - }, - error(error: E): Remote { - return { tag: "error", error } - }, - loaded(value: A): Remote { - return { tag: "loaded", value } - }, -} - // === App === // TODO: introduce proper error type -type AppError = string +type AppError = RequestError type State = { imageIds: Remote page: Page selectedImage: ImageId | undefined + refreshSignal: boolean // This is here only because of the refresh button. It is an artificial way to trigger a reload without the current page changing. The value is meaningless. It just has to change. } const State = { @@ -133,12 +217,14 @@ const State = { imageIds: Remote.loading(), page: Page.init(), selectedImage: undefined, + refreshSignal: false, } }, } type Msg = - | { tag: "imagesReceived"; imageIds: ImageId[]; forPage: Page } + | { tag: "imagesReceived"; result: Result; forPage: Page } + | { tag: "retryButtonClicked" } | { tag: "previousButtonClicked" } | { tag: "nextButtonClicked" } | { tag: "imageClicked"; imageId: ImageId } @@ -151,36 +237,42 @@ function useApp(): [State, Dispatch] { // Could also cache `Promise` in case two requests are made really fast one after another (not really the case in this app so whatever) const cacheRef = useRef>(new Map()) - async function getImageIdsCached(page: Page): Promise { + async function getImageIdsCached(page: Page): Promise> { const pageKey = Page.key(page) const maybeImages = cacheRef.current.get(pageKey) if (maybeImages === undefined) { - console.log("CACHE-MISS") - const images = await getImageIds(page) - cacheRef.current.set(pageKey, images) - return images + // console.log("CACHE-MISS") + const result = await getImageIds(page) + if (result.tag === "ok") { + cacheRef.current.set(pageKey, result.value) + } + return result } else { - console.log("CACHE-HIT") - return maybeImages + // console.log("CACHE-HIT") + return Result.ok(maybeImages) } } // === initialization & reloading === useEffect(() => { // TODO: error-handling - getImageIdsCached(state.page).then((imageIds) => { - dispatch({ tag: "imagesReceived", forPage: state.page, imageIds }) + void getImageIdsCached(state.page).then((result) => { + dispatch({ tag: "imagesReceived", forPage: state.page, result }) }) - }, [state.page]) // Would have been amazing if we could put `Page.key(state.page)` inside of this. Then the trouble with identity would be gone. But we can't, because React compiler and linter would complain /facepalm + }, [state.page, state.refreshSignal]) // Would have been amazing if we could put `Page.key(state.page)` inside of this. Then the trouble with identity would be gone. But we can't, because React compiler and linter would complain /facepalm function update(state: State, msg: Msg): State { switch (msg.tag) { case "imagesReceived": if (Page.eq(state.page, msg.forPage)) { - return { ...state, imageIds: Remote.loaded(msg.imageIds) } + return { ...state, imageIds: msg.result } } else { return state } + case "retryButtonClicked": + console.log("TODO") + // TODO: This actually needs to trigger the reload somehow /facepalm. But we can refresh only on page change. + return { ...state, imageIds: Remote.loading(), refreshSignal: !state.refreshSignal } case "previousButtonClicked": { const newPage = Page.previous(state.page) if (Page.eq(newPage, state.page)) { @@ -193,11 +285,9 @@ function useApp(): [State, Dispatch] { return { ...state, page: Page.next(state.page), imageIds: Remote.loading() } } case "imageClicked": { - console.log(msg.imageId) return { ...state, selectedImage: msg.imageId } } case "modalCloseButtonClicked": { - console.log("modal closed") return { ...state, selectedImage: undefined } } default: @@ -223,16 +313,51 @@ function useDispatch() { // === Views === function RemoteImages({ images }: { images: Remote }) { + const dispatch = useDispatch() switch (images.tag) { case "loading": return case "error": - return
Error: TODO
- case "loaded": + return ( + + + + { + dispatch({ tag: "retryButtonClicked" }) + }} + /> + + + ) + case "ok": return } } +function ErrorView({ error, onRetry }: { error: AppError; onRetry: () => void }) { + return ( + + Could not load images + + {RequestError.toString(error)} + + + + ) +} + function imageGridStyle(dimension: Dimension) { return { display: "grid", @@ -246,18 +371,29 @@ function imageGridStyle(dimension: Dimension) { } // 2x5 on mobile, 5x2 on desktop (assuming 10 images per page) -function ImagesSkeleton() { +function ImagesSkeleton({ visible = true }: { visible?: boolean }) { const images = Array.from({ length: 10 }) return ( - {images.map((_, index) => ( - - ))} + {images.map((_, index) => + visible ? ( + + ) : ( + + ), + )} ) } @@ -267,26 +403,68 @@ function Images({ images }: { images: ImageId[] }) { return ( {images.map((imageId) => ( - { dispatch({ tag: "imageClicked", imageId }) }} - sx={{ - width: Dimension.medium.width, - height: Dimension.medium.height, - objectFit: "cover", - cursor: "pointer", - }} key={imageId} - alt="" /> ))} ) } +function Image({ + imageId, + dimension, + onClick, +}: { + imageId: ImageId + dimension: Dimension + onClick?: () => void +}) { + const [loaded, setLoaded] = useState(false) + + return ( + + {!loaded && ( + + )} + { + setLoaded(true) + }} + onClick={onClick} + sx={{ + position: "absolute", + inset: 0, + width: dimension.width, + height: dimension.height, + objectFit: "cover", + cursor: onClick === undefined ? "default" : "pointer", + opacity: loaded ? 1 : 0, + }} + alt="" + /> + + ) +} + function ImageModal({ selectedImage }: { selectedImage: ImageId | undefined }) { const dispatch = useDispatch() return ( @@ -298,19 +476,7 @@ function ImageModal({ selectedImage }: { selectedImage: ImageId | undefined }) { maxWidth={false} > - {selectedImage !== undefined && ( - - )} + {selectedImage !== undefined && } )