batchCooking/specs/dev-conventions.md
kyuno053 66e5666687
chore(lint): ajoute noNonNullAssertion, useConsistentArrayType, useConsistentTypeDefinitions (#62)
Trois règles style ajoutées à biome.json :
- noNonNullAssertion (warn) : déconseille `valeur!`
- useConsistentArrayType (syntax: shorthand) : impose T[] (déjà la
  norme partout dans le repo, aucune réécriture nécessaire)
- useConsistentTypeDefinitions (style: interface) : impose `interface`
  pour tout type objet

0 violation sur les 200 fichiers du repo — déjà conforme. Documenté
dans specs/dev-conventions.md (tableau des règles Biome actives).

Corrige aussi la config du poste : node_modules pointait encore vers
Biome 1.9.4 alors que package.json déclare ^2.5.9 depuis le merge de
la #59 (ce checkout n'avait jamais été réinstallé depuis) — pnpm
install corrige, aucun changement de lockfile.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-21 14:02:45 +02:00

16 KiB

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, frontend-architecture.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 :

// ❌ 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(...)
style.noNonNullAssertion (warn) déconseille valeur! — préférer un vrai contrôle (if, ??, un guard) à l'affirmation "je sais que c'est non-null"
style.useConsistentArrayType (syntax: "shorthand") impose T[], jamais Array<T> — déjà la syntaxe utilisée partout dans le repo
style.useConsistentTypeDefinitions (style: "interface") impose interface pour tout type objet, jamais type X = { ... }type reste légitime pour une union/intersection/alias, seule la forme objet est concernée

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.tsLoggerService, 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 pour le détail (i18next/react-i18next, une seule langue aujourd'hui fr).


Gestion des erreurs

Contrat complet documenté à part : 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 (tscdist/) 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érentesresetDatabase() fait un TRUNCATE avant chaque test, partager la base viderait les données de dev à chaque run de tests.