From eab28aa017584eb32c245386a21e3c4af12dbf42 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Fri, 21 Aug 2026 10:37:58 +0200 Subject: [PATCH] =?UTF-8?q?docs(specs):=20documente=20les=20conventions=20?= =?UTF-8?q?de=20d=C3=A9veloppement=20du=20repo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nouveau specs/dev-conventions.md — jusqu'ici ces règles n'existaient que dans l'historique de commits/PR (classes vs objets littéraux pour la logique de service, préfixe _ sur private/protected, règles Biome actives, log service, tests sans mocks de la DB, conventions git/PR...), rien de centralisé pour un futur contributeur (humain ou Claude Code). Référencé depuis README.md, section "Qualité / Tests". Co-Authored-By: Claude Sonnet 5 --- README.md | 4 + specs/dev-conventions.md | 239 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 specs/dev-conventions.md diff --git a/README.md b/README.md index 627e39d..72cb20e 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,10 @@ pnpm dev:web ## Qualité / Tests +Conventions de code (classes vs objets littéraux, préfixe `_` sur les membres +privés, règles Biome actives, logs côté serveur, etc.) : +[specs/dev-conventions.md](specs/dev-conventions.md). + ```bash pnpm lint # Biome (lint + format check) pnpm lint:fix # Biome --write diff --git a/specs/dev-conventions.md b/specs/dev-conventions.md new file mode 100644 index 0000000..65d6cae --- /dev/null +++ b/specs/dev-conventions.md @@ -0,0 +1,239 @@ +# 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`. + +### É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//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 + +- **`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 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 ` + 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.