batchCooking/specs/dev-conventions.md
Nicolas baa102222e docs(conventions): impose une couverture de tests pour chaque ajout
Trois nouvelles règles dans "## Tests" :
- ajout front autonome (components/ui/*) -> Cypress mode composant
- ajout front non autonome (dépend de son layout/page) -> Cypress
  mode layout, spec .cy.ts classique dans cypress/e2e/
- nouvelle fonctionnalité front (parcours utilisateur) -> Cypress e2e,
  scénario Gherkin "En tant que... je veux..." (s'ajoute au test
  layout, ne le remplace pas)
- ajout back testable -> Mocha + Chai dans apps/api/test/

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 12:44:04 +02:00

304 lines
15 KiB
Markdown

# Conventions de développement — Projet Batch-cooking
> Règles de style/architecture qui s'appliquent à **tout** le code du monorepo,
> transverses aux specs par domaine ([backend-architecture.md](./backend-architecture.md),
> [frontend-architecture.md](./frontend-architecture.md),
> [error-handling.md](./error-handling.md)). Ce document répond à « comment
> j'écris du code ici », les autres répondent à « comment tel module est
> construit ».
---
## TypeScript — style général
- **Commenter le plus possible** : chaque interface, type, classe, fonction,
méthode et membre de classe porte un commentaire JSDoc (`/** ... */`). Le
commentaire explique le *pourquoi* (la décision, le compromis, le piège
évité), pas juste une paraphrase de la signature — la plupart des exemples
de ce repo font ça abondamment, c'est la norme attendue, pas une option.
- **Jamais de mélange HTML/TypeScript** : le JSX vit uniquement dans les
fichiers `.tsx` de composant. Toute logique (appels API, validation,
dérivation d'état, formatage) est extraite dans un `.ts` plain importé par
le composant. Pas de `style={{}}` inline ni de CSS-in-JS — le style va dans
un fichier `.scss` (voir plus bas).
- **Modificateurs de visibilité explicites** : dans toute classe, chaque
membre et méthode porte `public`, `private` ou `protected` — jamais le
défaut implicite (`public`) de TypeScript.
### Classes, jamais d'objets littéraux pour de la logique de service
Un objet qui regroupe du comportement (un logger, la gestion d'erreurs, un
client API, une résolution de libellés — dès qu'il y a un verbe, pas
seulement de la donnée) est une vraie `class`, jamais un objet littéral
assigné à un `const` :
```ts
// ❌ jamais ça, même pour un singleton sans état avec un seul appelant
export const logger = {
info(message: string) { /* ... */ },
};
// ✅ toujours ça
export class LoggerService {
public info(message: string): void { /* ... */ }
private _emit(level: LogLevel, message: string): void { /* ... */ }
}
/** Instance unique partagée — ce service est sans état, pas besoin d'une par appelant. */
export const logger = new LoggerService();
```
Exemples dans le repo : `LoggerService` (`apps/api/src/lib/logger.service.ts`),
`ErrorHandlerService` (`packages/error-tools`), `ExpressServer`
(`packages/express-tools`), `ApiClient`/`ErrorMessageService` (`apps/web`).
Un objet purement data (une table de correspondance, un objet qui *implémente*
une interface orientée donnée sans logique propre — ex. `theMealDbAdapter`,
`INGREDIENT_LABELS_EN`) n'est pas concerné, il reste un objet littéral.
React fonctionnel (hooks, function components, contexts) n'est **pas** forcé
dans des classes — cette règle vise spécifiquement la logique de service, pas
l'idiome React.
### Préfixe `_` sur tout membre `private`/`protected`
Tout champ ou méthode `private`/`protected` d'une classe porte le préfixe `_`
`emit``_emit`, `app``_app`. `public` ne porte jamais ce préfixe.
Rétroactif : appliqué à chaque classe existante du repo (pas seulement au
nouveau code) — voir `LoggerService._emit`/`_minSeverity`,
`ApiClient._request`, `ErrorHandlerService._fromZodError`/`_fromHttpError`/
`_fromUnknownError`, `ExpressServer._app`/`_registeredRoutes`.
### `await` toujours encapsulé dans `try`/`catch`
Aucun `await` nu (non encapsulé) : chaque appel `await` vit dans un bloc
`try`/`catch`. Conséquence directe — une fonction/méthode `async` contient
forcément au moins un `await`, donc **tout son corps** vit dans un
`try`/`catch`, pas seulement la ou les lignes qui awaitent. L'erreur
attrapée doit être traitée de façon utile pour ce point d'appel (log via
`LoggerService`, `throw`/retour d'une erreur typée, dégradation
gracieuse) — jamais avalée silencieusement — en cohérence avec le
traitement d'erreur déjà en place ailleurs dans le fichier (ex. les
chaînes `.catch()` du front). Rétroactif, même logique que les deux règles
ci-dessus.
Périmètre : code applicatif uniquement (routes, services, composants,
hooks, middlewares). Les fichiers de test (`apps/api/test/**`) et les
scripts one-off (`prisma/seed.ts`, `apps/api/src/scripts/seed-runtime.ts`,
`cypress.config.ts`) en sont exclus — un test s'appuie sur la propagation
du rejet d'un `await` non encapsulé pour faire échouer le test
(idiome chai/mocha) ; l'encapsuler forcerait soit un `re-throw` inutile,
soit risquerait d'avaler un vrai échec de test.
Deuxième exception : les handlers de route Express (`*.routes.ts`) passés à
`wrapAsyncHandler` (`packages/express-tools/src/async-handler.ts`) — son
rôle documenté est justement de transmettre une erreur/rejet à `next(err)`
pour qu'elle atteigne le middleware d'erreur centralisé, sans try/catch
répété dans chaque route. La règle vise le code service/hook/composant
qui n'est pas déjà filtré par ce mécanisme.
`lint/complexity/noUselessCatch` est désactivé dans `biome.json` pour
cette raison précise : cette règle Biome interdit un `catch` qui ne fait
que `throw err;`, exactement la forme que prend un `try`/`catch` ajouté
uniquement pour respecter la convention ci-dessus quand rien de plus
utile n'est à faire au niveau de cet appel (le middleware d'erreur logge
déjà tout centralement — voir plus bas). Les deux règles sont
mutuellement exclusives ; la convention du repo l'emporte.
### Élégance avant rapidité
Préférer une solution propre, bien structurée et délibérée à une solution
rapide/bricolée — y compris quand le correctif rapide "marche". C'est le fil
conducteur des règles ci-dessus, pas une règle séparée.
---
## Lint & format — Biome
`biome.json` à la racine, un seul config pour tout le monorepo (`pnpm lint` /
`pnpm lint:fix` / `pnpm format` à la racine, jamais un linter par package).
**Formatage** : indentation 2 espaces, largeur de ligne 100, guillemets
doubles, point-virgules toujours, virgules finales partout
(`trailingCommas: "all"`), tri automatique des imports (`assist.organizeImports`).
**Règles actives** au-delà du preset `recommended` :
| Règle | Effet |
|---|---|
| `suspicious.noExplicitAny` | interdit `any` explicite (déjà dans `recommended`, listée ici pour la visibilité) — utiliser un type précis ou `unknown` + vérification |
| `suspicious.noConsole` (`allow: ["error","warn","info","debug","table","assert"]`) | interdit `console.log` nu ; les niveaux nommés restent autorisés, mais uniquement depuis `LoggerService` (voir plus bas) — le reste du code ne touche jamais `console` directement |
| `nursery.noFloatingPromises` | interdit une Promise ni `await`-ée, ni `.then(ok, err)`/`.catch()`, ni `void`-ée, ni `return`-ée — un `useEffect` qui lance un fetch fire-and-forget doit soit avoir un `.catch()`, soit être explicitement `void navigate(...)` |
`noFloatingPromises` est une règle *type-aware* (nursery, nécessite Biome
≥ 2.0) — activée directement sous `rules.nursery` sans passer par
`linter.domains` (qui allumerait tout un lot d'autres règles type-aware hors
scope, ex. `noUnresolvedImports`).
Pour un flottant légitime (effet de bord volontairement non attendu, ex. une
navigation après un `catch` qui ne peut pas échouer utilement) : `void
maPromise();` — jamais un `// biome-ignore` pour ce cas précis, seulement pour
les rares cas où la règle se trompe vraiment (voir le commentaire au-dessus de
chaque `biome-ignore` existant, jamais silencieux).
---
## Logs de fonctionnement côté serveur
`apps/api/src/lib/logger.service.ts``LoggerService`, instance unique
`export const logger = new LoggerService()`. Seul endroit du code autorisé à
appeler `console.*` (via un `biome-ignore` justifié) ; partout ailleurs,
`noConsole` l'interdit — le but est qu'un log traverse toujours
`logger.debug/info/warn/error(message, meta?)`, jamais un `console.log` nu.
- Émet une ligne JSON structurée (`timestamp`, `level`, `message`, + `meta`
fusionné) — pas du texte libre, pour rester grep/parse-able par
`docker logs`/Portainer ou un agrégateur de logs.
- Seuil filtré par `NODE_ENV` : `debug` complet en développement, `warn`+ en
test (pour ne pas alourdir la sortie de Mocha), `info`+ en production.
- `apps/api/src/middlewares/request-logger.ts` — une ligne par requête
terminée (méthode/chemin/statut/durée), montée en tout premier dans
`app.ts`, avant même `setupCore` (CORS/JSON/cookies).
- `apps/api/src/middlewares/error-logger.ts` — monté juste avant
`createErrorMiddleware` ; réutilise `errorHandlerService.handle()` (pur,
sans effet de bord) pour classifier l'erreur avant que la réponse ne soit
construite : `warn` pour un `4xx` routinier (validation, 404, 401...),
`error` (avec la stack) pour un `5xx`/une exception non prévue.
`apps/web` n'a pas d'équivalent aujourd'hui — `noConsole` s'y applique aussi,
mais rien ne journalise encore côté client (hors ce que le navigateur capture
lui-même).
---
## SCSS
- SCSS (`sass`), jamais de CSS brut ni de CSS-in-JS.
- Commenter le SCSS aussi librement que le TypeScript — expliquer ce qu'une
règle affecte et pourquoi, pas juste ce qu'elle fait.
- Styles globaux (reset, tokens de thème) dans une feuille globale ;
styles de page/composant colocalisés au même niveau que les autres fichiers
du composant (`LoginPage.tsx` + `LoginPage.scss` côte à côte).
- Un fichier de tokens réutilisable (`_theme.scss` ou équivalent : couleurs,
espacements, typographie) importé partout où nécessaire, pour qu'un
re-stylage reste un changement centralisé.
---
## i18n
Tout texte affiché (formulaires, boutons, messages d'erreur) vient d'un
fichier de locale JSON (`apps/web/src/locales/<lng>/translation.json`),
jamais codé en dur dans un composant — voir
[frontend-architecture.md](./frontend-architecture.md#i18n-internationalisation)
pour le détail (i18next/react-i18next, une seule langue aujourd'hui `fr`).
---
## Gestion des erreurs
Contrat complet documenté à part : [error-handling.md](./error-handling.md).
En bref — `ErrorCode` (numérique, jamais codé en dur), `HttpError`
(`packages/error-tools`, framework-agnostic), `ErrorHandlerService.handle()`
comme unique point de mapping erreur → réponse HTTP, `ErrorMessageService`
côté web pour traduire un code en libellé affiché (jamais `err.message`
directement à l'utilisateur).
---
## Tests
### Couverture obligatoire pour tout ajout
- **Ajout front autonome** (un composant réutilisable, sans routeur ni
backend — `components/ui/*`) : test Cypress en **mode composant**
(`cypress/component/*.cy.tsx`, voir `CheckboxOption.cy.tsx`/
`RadioOption.cy.tsx`) — monte le composant seul, sans app autour.
- **Ajout front non autonome** (n'a de sens que dans son contexte de
page/layout — un élément de sidebar, une section d'une page existante) :
test Cypress en **mode layout**, un spec `.cy.ts` classique dans
`cypress/e2e/` (voir `layout.cy.ts`, `sidebar.cy.ts`,
`planning-page.cy.ts`) — pas de scénario Gherkin, juste la page routée
normalement.
- **Toute nouvelle fonctionnalité front** (un vrai parcours utilisateur, pas
juste un composant/élément isolé) : test Cypress **e2e**, un scénario
Gherkin ("En tant que... je veux...") dans un `.feature` + ses définitions
d'étapes, en réutilisant `cypress/support/step_definitions/
common.steps.ts` quand c'est possible (voir `planning.feature`,
`recipe-sources.feature`). S'ajoute au test "mode layout" ci-dessus, ne le
remplace pas — une fonctionnalité a généralement les deux : le layout qui
l'affiche, et le parcours qui l'utilise.
- **Tout ajout back testable** (logique pure, endpoint, service — tout ce
qui n'est pas du pur câblage/de la config) : test Mocha + Chai dans
`apps/api/test/`, même convention que le reste de la suite (voir
ci-dessous). "Testable" exclut les routes déjà couvertes par les tests
d'intégration du module (pas de doublon), pas la logique métier
elle-même.
### Conventions générales
- **`apps/api`** — Mocha + Chai, contre une vraie base Postgres isolée
(`.env.test`, jamais la même base que `pnpm dev:api`), pas de mocks de la
base ou des services internes. Seule exception : le premier module à parler
à un vrai réseau externe (`the-meal-db.ts`) stub `globalThis.fetch` — pas de
librairie de mock HTTP dans le repo, un simple `fetch` réassignable suffit
pour l'unique cas qui en a besoin.
- **Données de test générées, jamais des données réalistes en dur** :
`@faker-js/faker` pour tout nom/email/mot de passe de fixture — jamais un
nom/email qui ressemble à une vraie personne codé en dur dans un test.
- **`apps/web`** — Cypress (+ Cucumber pour certains scénarios), mocke l'API
via `cy.intercept` plutôt que de dépendre d'un vrai backend ; le comportement
réel de l'API est couvert côté `apps/api`. Toute valeur `ErrorCode` utilisée
dans un mock passe par `ErrorCode.XXX`, jamais une valeur numérique/chaîne
codée en dur — même règle que le code applicatif.
- Un module *pur* (pas de DB/réseau) est toujours testable isolément de sa
variante DB-backed — convention systématique dans ce repo : `matchXxx()`
pur + `loadXxxCatalog()` qui touche la base, jamais les deux mélangés dans
la même fonction (voir `ingredient-matcher.ts`, `tech-step-matcher.ts`,
`recipe-translation.ts`).
---
## Monorepo & packages partagés
`pnpm-workspace.yaml` : `apps/*` + `packages/*`. `packages/shared`,
`packages/error-tools`, `packages/express-tools` et `packages/date-tools` ont
un vrai build (`tsc` → `dist/`) et sont **consommés en JS compilé**, jamais en
TS brut — nécessaire pour un runtime Node pur sans transpilation à la volée
(l'image Docker de prod). Concrètement : après avoir modifié un fichier dans
un de ces packages, il faut le rebuilder (`pnpm --filter <package> build`, ou
juste `pnpm build` à la racine) pour que `apps/api`/`apps/web` voient le
changement en dev — un `tsx watch`/Vite ne recompile pas le package pour vous.
Séparation stricte par dépendance, pas par convention de nommage :
`packages/error-tools` n'importe jamais `express` (sinon il vivrait dans
`express-tools`) ; `packages/express-tools` dépend d'`error-tools`, jamais
l'inverse.
---
## Git & Pull Requests
- **Jamais de commit direct sur `main`** — toujours une branche de feature,
même pour un correctif d'une ligne. Tout ce qui arrive sur `main` passe par
une PR revue (l'historique de `main` n'est fait que de commits de merge de
PR).
- **Messages de commit en français**, format
`type(scope): description` (`feat`, `fix`, `style`, `docs`, `chore`...),
scope = le domaine touché (`recipes`, `planning`, `api`, `layout`...).
Toujours un trailer `Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>`
quand le commit vient de Claude Code.
- PRs plutôt petites et scopées à un sujet — l'historique de ce repo est fait
de nombreuses PRs courtes (`feat(recipes): ...`, `fix(planning): ...`)
plutôt que de grosses PRs multi-sujets, sauf demande explicite contraire.
---
## Variables d'environnement & secrets
- Aucun secret n'a de valeur par défaut codée en dur (`JWT_SECRET`,
`POSTGRES_PASSWORD`...) — `docker-compose.yml`/`config/env.ts` échouent
bruyamment si absent, jamais un fallback devinable.
- `.env.example` ne contient que des valeurs bidon (`changeme`), jamais de
vrai identifiant, même de dev.
- `.env` (dev) et `.env.test` (suite Mocha) pointent vers des bases Postgres
**différentes**`resetDatabase()` fait un `TRUNCATE` avant chaque test,
partager la base viderait les données de dev à chaque run de tests.