Five more explicit review points, on the same PR branch. ## Every interface key commented Audited all 6 interfaces in the codebase. Two had partially-commented members (violates the "every key gets /** */" rule): AuthResult (apps/api/auth.service.ts) and SafeUserProfile (packages/shared) — both now fully commented. The other four (AuthTokenPayload, AuthContextValue, ErrorHandlingResult, ApiErrorResponse) were already compliant. ## Removed the Express namespace augmentation apps/api/src/types/express.d.ts (renamed to express-request.augment.ts in the last round) is gone entirely. requireAuth now attaches the authenticated profile to `res.locals.userProfile` — Express's own built-in per-request mechanism for exactly this — typed via a new AuthLocals interface and `Response<unknown, AuthLocals>`, instead of a project-wide `declare global` silently changing every Request's type whether or not it went through the middleware. ## ErrorHandlerService confirmed framework-agnostic It already had zero Express import. Documented this explicitly (in the package's index.ts and the new backend-architecture.md spec) as a deliberate split: ErrorHandlerService is framework-agnostic (would work behind Fastify too), ExpressServer/createErrorMiddleware are the actual Express integration layer. ## packages/express-tools: server init + route/middleware utilities New ExpressServer class, modeled on the pattern shared as a reference (adapted, not copied 1:1 — deliberately left out the reference's custom runtime param-type-validation system, since zod already does that job in this codebase and running two parallel validation mechanisms would be redundant, not "propre"): - setupCore() — the common cors/json/cookie-parser stack - addRoute() — registers a route, warns+skips instead of silently double-registering the same method+path - addMiddleware() / mountRouter() / setErrorHandler() - listen() - .instance — the raw Express app, for supertest Also added wrapAsyncHandler() — forwards a thrown/rejected error from an async handler to next(err) automatically, removing the manual try/catch/next(err) every route needed. apps/api/src/app.ts now builds via ExpressServer (createServer(), consumed by both server.ts's .listen() and createApp()'s .instance for tests). auth.routes.ts's signup/login handlers use wrapAsyncHandler instead of manual try/catch. cookie-parser/cors moved out of apps/api's own dependencies entirely — they're express-tools' concern now. ## assertIsNever (packages/shared/src/tools/) Exhaustiveness-check helper for switch/if-chains over a union: takes a `never`-typed value and throws, so a forgotten case in a later-added union member becomes a compile error instead of a silent runtime fallthrough. Verified for real (not just written and assumed correct): wrote a throwaway switch missing a case and confirmed `tsc` rejects it with the exact expected error, then deleted the scratch file. No existing switch/if-chain over a union in the codebase yet to retrofit it into — noted as ready for when one appears (e.g. the not-yet-built batch-cooking calculation module or recipe-import pipeline). ## specs/ updated New specs/backend-architecture.md — ExpressServer, wrapAsyncHandler, the res.locals decision (with the "why not declare global" reasoning spelled out), assertIsNever. error-handling.md and frontend-architecture.md cross-link to it instead of duplicating. README covers the same, briefly. ## Verification Full lint/mocha/cucumber/build green. Re-ran `node dist/server.js` standalone (mirrors Docker, no tsx) after the ExpressServer refactor: /health, a 404 (numeric 4040), and a real signup + GET /me round trip confirming res.locals-based auth actually works at runtime, not just that tsc accepts the types.
136 lines
5.6 KiB
Markdown
136 lines
5.6 KiB
Markdown
# Architecture backend — Projet Batch-cooking
|
|
|
|
> Documentation de l'organisation d'`apps/api` et de l'outillage partagé
|
|
> (`packages/express-tools`, `packages/shared`).
|
|
|
|
---
|
|
|
|
## `packages/express-tools` — outillage Express générique
|
|
|
|
Package séparé, réutilisable par n'importe quel service Express du monorepo (pas
|
|
seulement `apps/api`) : pas de logique métier, juste de l'infra Express.
|
|
|
|
### `ExpressServer` — init serveur, routes, middlewares
|
|
|
|
Enveloppe une application Express derrière une API typée, au lieu que chaque
|
|
service refasse le même `express()` à la main :
|
|
|
|
```ts
|
|
const server = new ExpressServer();
|
|
server.setupCore({ corsOrigin: env.CORS_ORIGIN }); // cors + json + cookie-parser
|
|
server.addRoute("get", "/health", (_req, res) => res.status(200).json({ status: "ok" }));
|
|
server.mountRouter("/auth", authRouter);
|
|
server.addMiddleware(notFoundHandler);
|
|
server.setErrorHandler(createErrorMiddleware(errorHandlerService));
|
|
server.listen(port, () => console.log(`Listening on ${port}`));
|
|
```
|
|
|
|
- `setupCore(options)` — middleware stack commun (CORS avec credentials, JSON,
|
|
cookies).
|
|
- `addRoute(method, path, ...handlers)` — enregistre une route ; avertit et
|
|
ignore au lieu d'écraser silencieusement si la même route (méthode + chemin)
|
|
est déjà enregistrée.
|
|
- `addMiddleware` / `mountRouter` / `setErrorHandler` — ajout de middleware
|
|
générique, montage d'un `Router` complet, middleware d'erreur final (4
|
|
arguments — doit être ajouté en dernier).
|
|
- `.instance` — l'app Express brute, nécessaire pour les outils de test
|
|
(supertest) qui attendent une instance `Express`, pas le wrapper.
|
|
- `.listen(port, onListening?)` — démarre le serveur.
|
|
|
|
`apps/api/src/app.ts` expose deux fonctions : `createServer(): ExpressServer`
|
|
(utilisée par `server.ts`, qui appelle `.listen()`) et `createApp(): Express`
|
|
(= `createServer().instance`, utilisée par les tests).
|
|
|
|
### `wrapAsyncHandler` — plus de try/catch répété dans les routes
|
|
|
|
```ts
|
|
router.post("/signup", wrapAsyncHandler(async (req, res) => {
|
|
const profile = await signup(req.body); // une erreur/rejet ici va automatiquement à next()
|
|
res.status(201).json(profile);
|
|
}));
|
|
```
|
|
|
|
Sans ça, une exception dans un handler `async` ne remonte jamais tout seule au
|
|
middleware d'erreur d'Express — chaque route devait faire son propre
|
|
`try { ... } catch (err) { next(err); }`. `wrapAsyncHandler` l'automatise.
|
|
|
|
### `HttpError` / `ErrorHandlerService` / `createErrorMiddleware`
|
|
|
|
Voir [error-handling.md](./error-handling.md) pour le détail. À noter :
|
|
`ErrorHandlerService` **n'a aucune dépendance à Express** — c'est un service
|
|
générique `erreur → { status, body }` qui fonctionnerait à l'identique derrière
|
|
Fastify ou n'importe quel autre framework. `ExpressServer` et
|
|
`createErrorMiddleware` sont la vraie couche Express : elles adaptent des
|
|
pièces indépendantes du framework (comme `ErrorHandlerService`) à l'API
|
|
d'Express. C'est pour ça que `ErrorHandlerService` n'importe jamais `express`.
|
|
|
|
---
|
|
|
|
## Auth : `res.locals`, pas d'augmentation du namespace Express
|
|
|
|
`requireAuth` (`apps/api/src/middlewares/require-auth.ts`) attache le profil
|
|
authentifié à **`res.locals.userProfile`**, typé via l'interface `AuthLocals` :
|
|
|
|
```ts
|
|
export interface AuthLocals {
|
|
userProfile: SafeUserProfile;
|
|
}
|
|
|
|
export async function requireAuth(req: Request, res: Response<unknown, AuthLocals>, next: NextFunction) {
|
|
// ...
|
|
res.locals.userProfile = safeProfile;
|
|
next();
|
|
}
|
|
```
|
|
|
|
Un handler derrière ce middleware type sa réponse `Response<unknown, AuthLocals>`
|
|
et lit `res.locals.userProfile` sans cast :
|
|
|
|
```ts
|
|
authRouter.get("/me", requireAuth, (_req, res: Response<unknown, AuthLocals>) => {
|
|
res.status(200).json(res.locals.userProfile);
|
|
});
|
|
```
|
|
|
|
**Pourquoi pas `declare global { namespace Express { interface Request {...} } }`**
|
|
(l'approche initialement utilisée, retirée depuis) : `res.locals` est le
|
|
mécanisme natif d'Express prévu exactement pour ça (faire passer des données
|
|
d'un middleware au handler suivant), typé par route via un paramètre
|
|
générique — pas une augmentation globale et permanente qui change
|
|
silencieusement le type de **toutes** les `Request` du projet, qu'elles soient
|
|
passées par ce middleware ou non.
|
|
|
|
---
|
|
|
|
## `packages/shared` — `assertIsNever`
|
|
|
|
`packages/shared/src/tools/assert-is-never.ts` — vérification d'exhaustivité
|
|
pour un `switch`/`if`-chain sur une union :
|
|
|
|
```ts
|
|
switch (shape.kind) {
|
|
case "circle": return Math.PI * shape.radius ** 2;
|
|
case "square": return shape.side ** 2;
|
|
default: return assertIsNever(shape); // erreur de compilation si un cas manque
|
|
}
|
|
```
|
|
|
|
Si un membre de l'union n'est pas traité par une branche précédente, `shape`
|
|
n'est plus de type `never` au niveau du `default` → **erreur de compilation**
|
|
(vérifié : `tsc` rejette bien un cas manquant). Lève aussi une vraie erreur au
|
|
runtime, en filet de sécurité si une valeur invalide échappe au système de
|
|
types (ex. donnée externe non validée).
|
|
|
|
Pas encore de point d'usage réel dans le code métier actuel (aucun
|
|
switch/if-chain exhaustif sur une union n'existe encore) — prêt à l'emploi dès
|
|
qu'un cas s'y prête (le module « Calcul batch-cooking » ou le pipeline d'import
|
|
de recette, tous deux encore à construire, en auront probablement).
|
|
|
|
---
|
|
|
|
## Pas de fichiers `.d.ts` écrits à la main
|
|
|
|
Voir [frontend-architecture.md](./frontend-architecture.md#note-sur-les-fichiers-dts)
|
|
pour le détail côté `apps/web`. Côté `apps/api` : aucune augmentation de type
|
|
globale (`declare global`) n'est utilisée — voir la section `res.locals`
|
|
ci-dessus, qui est précisément ce qui aurait nécessité ce genre de fichier.
|