Introduce README.md, AI-USE.md. Setup nix-shell for node. Tag personal comments with @PERSONAL-NOTE.
This commit is contained in:
parent
24334cb342
commit
945dcdead0
6 changed files with 96 additions and 5 deletions
1
.envrc
Normal file
1
.envrc
Normal file
|
|
@ -0,0 +1 @@
|
|||
use nix
|
||||
2
.gitignore
vendored
2
.gitignore
vendored
|
|
@ -10,3 +10,5 @@ yarn-error.log*
|
|||
pnpm-debug.log*
|
||||
|
||||
.DS_Store
|
||||
|
||||
.direnv
|
||||
|
|
|
|||
21
AI-USE.md
Normal file
21
AI-USE.md
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
Wrote most of the code by hand.
|
||||
|
||||
My own design:
|
||||
|
||||
- config/state/msg/update/effects, all the types
|
||||
- which hooks to use
|
||||
- remote/request/result
|
||||
- splitting the codebase into modules/folders/files
|
||||
|
||||
LLM-generated:
|
||||
where I mostly reviewed
|
||||
|
||||
- The Material UI / styling - after using a few components manually, I mostly let LLM decide which components to use, how to use them, and letting it decide/generate styling details.
|
||||
|
||||
LLM-assisted:
|
||||
|
||||
- Name suggestions
|
||||
- Project setup (linters, formatters, ts compiler settings)
|
||||
- Checking and Code Review
|
||||
- Snippet generation
|
||||
- Explanations of external API details, and confirming browser caching behaviour
|
||||
47
README.md
Normal file
47
README.md
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
# Picturarium
|
||||
|
||||
Picturarium is a small React image gallery using the Lorem Picsum API. It displays paginated image thumbnails, caches loaded pages during the session, and opens selected images in a larger
|
||||
modal view. The app uses TypeScript, Material UI, and explicit remote/request state handling for loading and error states.
|
||||
|
||||
# Installation
|
||||
|
||||
This assumes `node` is installed on your system.
|
||||
TODO: specify `git clone` too here.
|
||||
|
||||
```bash
|
||||
git clone // TODO
|
||||
cd picturarium
|
||||
npm install
|
||||
```
|
||||
|
||||
# Execution
|
||||
|
||||
After cloning the repo in `picturarium/`:
|
||||
|
||||
## dev
|
||||
|
||||
You can start a vite dev-server
|
||||
|
||||
```bash
|
||||
npm run dev
|
||||
```
|
||||
|
||||
## production
|
||||
|
||||
To see production build:
|
||||
|
||||
```bash
|
||||
npm run build
|
||||
npm run preview
|
||||
```
|
||||
|
||||
# Comments
|
||||
|
||||
In this codebase I sometimes use comments that start as
|
||||
|
||||
```typescript
|
||||
// @PERSONAL_NOTE
|
||||
```
|
||||
|
||||
These are just there for this particular assignment, and I wouldn't included them in a real codebase.
|
||||
They are just notes to let you know why I made specific decisions and how I think about stuff.
|
||||
7
shell.nix
Normal file
7
shell.nix
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
{ pkgs ? import <nixpkgs> {} }:
|
||||
|
||||
pkgs.mkShell {
|
||||
packages = [
|
||||
pkgs.nodejs_22
|
||||
];
|
||||
}
|
||||
|
|
@ -5,6 +5,7 @@ import { Result } from "../result"
|
|||
import { RequestError } from "../request"
|
||||
import { Remote } from "../remote"
|
||||
|
||||
// @PERSONAL_NOTE
|
||||
// NOTE: There are multiple school of thoughts on how to structure a component.
|
||||
// - What I prefer is to extract generic components or complex sub-components into their own modules (but this assignment is too simple for that).
|
||||
// I also like to keep views/states/messages/initialization/effects/update of one component in one single (even if giant) file (or a structure like `Component/...` + `Component.tsx`)
|
||||
|
|
@ -41,6 +42,7 @@ const CONFIG = {
|
|||
// desktopRows: 3,
|
||||
// } as const satisfies Config
|
||||
|
||||
// @PERSONAL_NOTE
|
||||
// Could do comptime assert: `size = mobile product`, and `size = desktop product`, but this is pretty heavy in TS.
|
||||
assertValidConfig(CONFIG)
|
||||
|
||||
|
|
@ -81,6 +83,10 @@ const Dimension = {
|
|||
// === Page ===
|
||||
type PageIndex = number
|
||||
|
||||
// @PERSONAL_NOTE
|
||||
// This is a huge overkill for this type of application - especially because page size is known at compile-time and we can't even change page size at runtime.
|
||||
// Most of the identity issues down-the line would be trivial if we just used `PageIndex` instead (which is a nice immutable value where equality is structural).
|
||||
// But in general there's no avoiding objects in js/ts, so during code-review this would allow me to demonstrate understanding of these subtle issues. That's why I chose to use it here.
|
||||
type Page = {
|
||||
index: PageIndex
|
||||
size: number
|
||||
|
|
@ -97,6 +103,7 @@ const Page = {
|
|||
return { ...page, index: page.index + 1 }
|
||||
},
|
||||
previous(page: Page): Page {
|
||||
// @PERSONAL_NOTE
|
||||
// Could do
|
||||
// if (page.index == FIRST_PAGE) { return page }
|
||||
// this preserves identity of object, so is nicer for `useEffect`, but it's waaaay to subtle. So I'm not relying on that.
|
||||
|
|
@ -158,6 +165,7 @@ function useApp(): [State, Dispatch] {
|
|||
const [state, dispatch] = useReducer(update, State.init())
|
||||
|
||||
// === Caching API calls ===
|
||||
// @PERSONAL_NOTE
|
||||
// Could also cache `Promise<ImageId[]>` in case two requests are made really fast one after another (not really the case in this app so whatever)
|
||||
const cacheRef = useRef<Map<PageKey, ImageId[]>>(new Map())
|
||||
|
||||
|
|
@ -178,11 +186,16 @@ function useApp(): [State, Dispatch] {
|
|||
}
|
||||
|
||||
// === initialization & reloading ===
|
||||
useEffect(() => {
|
||||
void getImageIdsCached(state.page).then((result) => {
|
||||
dispatch({ tag: "imagesReceived", forPage: state.page, result })
|
||||
})
|
||||
}, [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
|
||||
useEffect(
|
||||
() => {
|
||||
void getImageIdsCached(state.page).then((result) => {
|
||||
dispatch({ tag: "imagesReceived", forPage: state.page, result })
|
||||
})
|
||||
},
|
||||
// @PERSONAL_NOTE
|
||||
// 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],
|
||||
)
|
||||
|
||||
function update(state: State, msg: Msg): State {
|
||||
switch (msg.tag) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue