From 8f2632f45686d0eaf950d45c2501fa5a74a94e76 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Sun, 26 Apr 2026 16:34:34 +0200 Subject: [PATCH] Add 12 ADRs documenting frontend architecture decisions --- docs/decisions/.gitkeep | 0 .../0001-migration-from-nextjs-to-angular.md | 21 ++++++++++++ ...0002-standalone-components-no-ngmodules.md | 20 ++++++++++++ ...3-dual-ui-library-progressive-migration.md | 28 ++++++++++++++++ .../0004-http-interceptors-pipeline.md | 29 +++++++++++++++++ .../0005-jwt-authentication-localstorage.md | 26 +++++++++++++++ .../0006-data-loading-via-route-resolvers.md | 30 +++++++++++++++++ .../0007-build-tool-angular-cli-esbuild.md | 20 ++++++++++++ docs/decisions/0008-testing-karma-jasmine.md | 21 ++++++++++++ ...0009-pre-commit-hooks-lint-staged-husky.md | 27 ++++++++++++++++ .../0010-node-version-management-nvm.md | 19 +++++++++++ .../0011-feature-domain-organisation.md | 32 +++++++++++++++++++ .../0012-scss-bootstrap-import-deprecation.md | 29 +++++++++++++++++ 13 files changed, 302 insertions(+) delete mode 100644 docs/decisions/.gitkeep create mode 100644 docs/decisions/0001-migration-from-nextjs-to-angular.md create mode 100644 docs/decisions/0002-standalone-components-no-ngmodules.md create mode 100644 docs/decisions/0003-dual-ui-library-progressive-migration.md create mode 100644 docs/decisions/0004-http-interceptors-pipeline.md create mode 100644 docs/decisions/0005-jwt-authentication-localstorage.md create mode 100644 docs/decisions/0006-data-loading-via-route-resolvers.md create mode 100644 docs/decisions/0007-build-tool-angular-cli-esbuild.md create mode 100644 docs/decisions/0008-testing-karma-jasmine.md create mode 100644 docs/decisions/0009-pre-commit-hooks-lint-staged-husky.md create mode 100644 docs/decisions/0010-node-version-management-nvm.md create mode 100644 docs/decisions/0011-feature-domain-organisation.md create mode 100644 docs/decisions/0012-scss-bootstrap-import-deprecation.md diff --git a/docs/decisions/.gitkeep b/docs/decisions/.gitkeep deleted file mode 100644 index e69de29..0000000 diff --git a/docs/decisions/0001-migration-from-nextjs-to-angular.md b/docs/decisions/0001-migration-from-nextjs-to-angular.md new file mode 100644 index 0000000..21fdfe1 --- /dev/null +++ b/docs/decisions/0001-migration-from-nextjs-to-angular.md @@ -0,0 +1,21 @@ +# ADR 0001: Migration from Next.js/React to Angular + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +The project was initially built with Next.js (React). The development team already maintained an existing Angular stack for other projects. Running two separate frontend frameworks in parallel created an unnecessary knowledge split and increased maintenance overhead. + +Server-side rendering, which is Next.js's main differentiator, was not required for this application — all data is user-specific and served via authenticated API calls, making SSR provide little to no benefit. + +## Decision + +Migrate the frontend to Angular, reusing the existing Angular expertise and toolchain. The Next.js/React code (pages, components, utilities, middleware) is abandoned entirely. + +## Consequences + +- **Positive:** Single frontend framework across projects. Reuse of existing Angular expertise, libraries, and patterns. +- **Positive:** No SSR complexity. The app is a pure SPA, which fits Angular's model. +- **Negative:** Complete rewrite of frontend code. No incremental migration path between React and Angular. +- **Residual:** Next.js artifacts (`.next/`, `next.config.js`, `src/modules/`, `src/middleware.ts`, etc.) were removed as part of the cleanup after the migration. diff --git a/docs/decisions/0002-standalone-components-no-ngmodules.md b/docs/decisions/0002-standalone-components-no-ngmodules.md new file mode 100644 index 0000000..b72fa35 --- /dev/null +++ b/docs/decisions/0002-standalone-components-no-ngmodules.md @@ -0,0 +1,20 @@ +# ADR 0002: Standalone Components Without NgModules + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Historically, Angular required all components, directives, and pipes to be declared inside NgModules, which acted as compilation units. Angular 14 introduced standalone components as an opt-in alternative. Angular 17+ made them the recommended default and they became the foundation of the new `@angular/build:application` builder. + +## Decision + +The entire application uses standalone components with no NgModules. The app entry point is `app.config.ts` (providers) + `app.routes.ts` (routing), with no `AppModule`. + +## Consequences + +- **Positive:** Each component explicitly declares its own `imports`, making dependencies visible and reducing hidden coupling. +- **Positive:** Better tree-shaking — unused imports in one component don't affect others. +- **Positive:** Aligns with Angular's long-term direction; compatible with the esbuild-based application builder. +- **Negative:** Slightly more verbose component decorators (each must list its own `imports`). +- **Note:** `CommonModule` should not be imported wholesale; use specific primitives (`NgClass`, `DatePipe`, etc.) or the newer `@if`/`@for` control flow syntax instead. diff --git a/docs/decisions/0003-dual-ui-library-progressive-migration.md b/docs/decisions/0003-dual-ui-library-progressive-migration.md new file mode 100644 index 0000000..b4493a4 --- /dev/null +++ b/docs/decisions/0003-dual-ui-library-progressive-migration.md @@ -0,0 +1,28 @@ +# ADR 0003: Dual UI Library — Progressive Migration Strategy + +**Date:** 2026-04-26 +**Status:** Accepted (transitional) + +## Context + +The application currently uses two UI libraries simultaneously: + +- **Angular Material** — component library (buttons, dialogs, tables, forms, etc.) +- **Bootstrap 5** — layout utilities, typography, spacing, and some components + +This coexistence is a legacy of a progressive migration: Bootstrap was the original styling layer, and Angular Material was introduced incrementally. Maintaining both libraries increases bundle size, creates styling inconsistencies, and adds cognitive overhead. + +## Decision + +Accept the dual-library state as a transitional phase. The long-term goal is to consolidate to a single UI library. Neither Angular Material nor Bootstrap is guaranteed to be the final choice — the decision will be made when consolidation is actively prioritised, potentially in favour of a different library altogether. + +In the meantime: + +- Angular Material handles interactive components (dialogs, forms, navigation). +- Bootstrap handles layout (grid, spacing utilities) and fills gaps where Material has no equivalent. + +## Consequences + +- **Positive:** No big-bang UI rewrite required at this stage. +- **Negative:** Two styling systems in tension. Bootstrap's `@import`-based Sass is incompatible with the `@use` module system; Dart Sass deprecation warnings are silenced in `angular.json` as a workaround (see ADR 0012). +- **Future action:** When consolidation is prioritised, evaluate the available options (Material 3, PrimeNG, Spartan/shadcn-angular, or custom) and supersede this ADR. diff --git a/docs/decisions/0004-http-interceptors-pipeline.md b/docs/decisions/0004-http-interceptors-pipeline.md new file mode 100644 index 0000000..90c2f13 --- /dev/null +++ b/docs/decisions/0004-http-interceptors-pipeline.md @@ -0,0 +1,29 @@ +# ADR 0004: HTTP Interceptors Pipeline + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Every API call requires the same three cross-cutting concerns: + +1. Prepend the API base URL so services don't hard-code it. +2. Attach the JWT bearer token when one exists. +3. Unwrap error responses into a consistent shape. + +Handling these ad hoc in each service would duplicate logic and couple services to environment details. + +## Decision + +Three functional interceptors are chained in order in `app.config.ts`: + +1. **`apiInterceptor`** — prepends `environment.apiBaseUrl` to every outgoing request URL. Consequence: all services call relative paths (e.g. `/skydive/jumps`), never absolute URLs. +2. **`tokenInterceptor`** — attaches `Authorization: Token ` when a token exists in `JwtService`. +3. **`errorInterceptor`** — unwraps `err.error` from HttpErrorResponse and rethrows, so subscribers receive the API error payload directly. + +## Consequences + +- **Positive:** Services are environment-agnostic. Swapping the API base URL requires changing only `environment.ts`. +- **Positive:** Auth header injection is transparent to all services. +- **Positive:** Error handling is uniform across the application. +- **Constraint:** Services must always use relative paths starting with `/`. Absolute URLs would get the base URL prepended, breaking them. diff --git a/docs/decisions/0005-jwt-authentication-localstorage.md b/docs/decisions/0005-jwt-authentication-localstorage.md new file mode 100644 index 0000000..1bab290 --- /dev/null +++ b/docs/decisions/0005-jwt-authentication-localstorage.md @@ -0,0 +1,26 @@ +# ADR 0005: JWT Authentication Stored in localStorage + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +The application requires persistent authentication across browser sessions. The two main options for token storage are: + +- **`localStorage`** — accessible via JavaScript; survives tab/window close; vulnerable to XSS. +- **httpOnly cookies** — inaccessible to JavaScript; protected from XSS; requires CSRF protection. + +The application is not publicly accessible — it is used internally by a known, controlled user base. The attack surface for XSS is limited. + +## Decision + +Store the JWT token in `localStorage` under the key `jwtToken`. `JwtService` handles read/write/delete operations. `UserService` exposes `currentUser` (BehaviorSubject) and `isAuthenticated` (ReplaySubject) observables consumed by guards and the layout. + +An `APP_INITIALIZER` in `app.config.ts` calls `userService.getCurrentUser()` on startup if a token exists, ensuring guards can rely on `isAuthenticated` being populated before the first navigation. + +## Consequences + +- **Positive:** Simple implementation. No server-side session management. Works seamlessly with Angular's functional guards. +- **Positive:** Token persists across browser restarts without requiring re-login. +- **Risk:** XSS attacks could exfiltrate the token. Acceptable given the non-public nature of the application. +- **Future consideration:** If the application becomes publicly accessible or handles sensitive data, migrate to httpOnly cookies with CSRF tokens. diff --git a/docs/decisions/0006-data-loading-via-route-resolvers.md b/docs/decisions/0006-data-loading-via-route-resolvers.md new file mode 100644 index 0000000..749a70b --- /dev/null +++ b/docs/decisions/0006-data-loading-via-route-resolvers.md @@ -0,0 +1,30 @@ +# ADR 0006: Data Loading via Route Resolvers + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Route components need data to render. Two common approaches exist: + +- **In-component fetching** (`ngOnInit`): component renders first in a loading state, then fetches data asynchronously. +- **Route resolvers**: data is fetched before the route activates; the component receives it immediately via `ActivatedRoute.data`. + +Loading states scattered across every component lead to inconsistent UX and duplicate skeleton/spinner logic. + +## Decision + +Use Angular route resolvers (under `src/app/core/resolvers/`) to pre-fetch data for each route. New route components should follow this pattern rather than fetching in `ngOnInit`. + +Data is accessed in components via: + +```ts +this.route.data.subscribe((data) => { ... }); +``` + +## Consequences + +- **Positive:** Components have their data available immediately — no need for per-component loading states on initial render. +- **Positive:** Loading indication is centralised at the router level (can use router events to show a global progress bar). +- **Negative:** Navigation appears "stuck" while the resolver fetches — there is no partial render before data arrives. Acceptable given the API response times in this application. +- **Constraint:** Resolvers should handle errors gracefully (redirect or return a default value) to avoid navigation hangs on API failure. diff --git a/docs/decisions/0007-build-tool-angular-cli-esbuild.md b/docs/decisions/0007-build-tool-angular-cli-esbuild.md new file mode 100644 index 0000000..be9a119 --- /dev/null +++ b/docs/decisions/0007-build-tool-angular-cli-esbuild.md @@ -0,0 +1,20 @@ +# ADR 0007: Build Tool — Angular CLI with esbuild + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Angular 17 introduced a new application builder (`@angular/build:application`) based on esbuild and Vite, replacing the legacy webpack-based `@angular-devkit/build-angular:browser` builder. The new builder delivers significantly faster cold builds and near-instant incremental rebuilds. + +## Decision + +Use the `@angular-devkit/build-angular:application` builder (esbuild-based) as configured in `angular.json`. The entry point is defined via the `browser` field rather than `main`. + +## Consequences + +- **Positive:** Build times are 3–5× faster than the webpack builder for both development and production. +- **Positive:** Native ESM output. Better code splitting. +- **Positive:** Aligns with Angular's recommended toolchain for Angular 17+. +- **Negative:** Some webpack-specific configuration options (e.g. custom webpack plugins) no longer apply. Not an issue for this project. +- **Note:** The `sass` sub-option under `stylePreprocessorOptions` (for `silenceDeprecations`) is specific to this builder's schema; the Karma test builder uses a different schema and does not support it. diff --git a/docs/decisions/0008-testing-karma-jasmine.md b/docs/decisions/0008-testing-karma-jasmine.md new file mode 100644 index 0000000..65fe7be --- /dev/null +++ b/docs/decisions/0008-testing-karma-jasmine.md @@ -0,0 +1,21 @@ +# ADR 0008: Testing — Karma + Jasmine + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Angular's default test setup uses Karma as the test runner (executing tests in a real Chrome browser) and Jasmine as the assertion/spec framework. Alternatives include Jest (runs in Node via jsdom, no real browser) and Vitest (ESM-native, faster). + +Migrating to Jest or Vitest would require changing test utilities, removing `zone.js/testing`, and potentially adjusting Angular testing module setup. + +## Decision + +Keep Karma + Jasmine. The test suite (54 specs) is small enough that Karma's startup overhead is not a bottleneck. The default Angular TestBed utilities work without modification. + +## Consequences + +- **Positive:** Zero migration effort. Full compatibility with Angular's `TestBed` and `ComponentFixture` APIs. +- **Positive:** Tests run in a real browser, catching browser-specific issues that jsdom misses. +- **Negative:** Slower startup than Jest/Vitest due to Chrome launch. Acceptable at the current test suite size. +- **Future consideration:** If the test suite grows significantly or CI performance becomes a concern, evaluate migration to Jest with `jest-preset-angular`. diff --git a/docs/decisions/0009-pre-commit-hooks-lint-staged-husky.md b/docs/decisions/0009-pre-commit-hooks-lint-staged-husky.md new file mode 100644 index 0000000..cde7087 --- /dev/null +++ b/docs/decisions/0009-pre-commit-hooks-lint-staged-husky.md @@ -0,0 +1,27 @@ +# ADR 0009: Pre-commit Quality Gates with Husky and lint-staged + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Without automated enforcement, code quality rules (linting, formatting, tests) are only checked in CI — or not at all if there is no CI pipeline. Issues are discovered late, after a commit is already in the repository. + +## Decision + +Husky manages a `pre-commit` hook that runs: + +1. **Full test suite** (`npm test -- --watch=false`) — blocks commit on any failing spec. +2. **lint-staged** — on staged files only: + - `prettier --write` on `*.{ts,html,scss,json,md}` + - `eslint --max-warnings=0` on `*.ts` + - `eslint --max-warnings=0` on `*.html` + +nvm is initialised inside the hook script to ensure the correct Node version (pinned in `.nvmrc`) is active regardless of the shell environment that triggered the commit. + +## Consequences + +- **Positive:** No lint errors or failing tests can enter the repository via a normal commit. +- **Positive:** Prettier auto-fixes formatting on commit — developers don't need to run it manually. +- **Negative:** Commits are slower (~10–20 s for the full test suite). Acceptable trade-off for a solo project. +- **Constraint:** Requires nvm to be installed on the developer's machine. The hook sources `$HOME/.nvm/nvm.sh` directly. diff --git a/docs/decisions/0010-node-version-management-nvm.md b/docs/decisions/0010-node-version-management-nvm.md new file mode 100644 index 0000000..3dbc3a6 --- /dev/null +++ b/docs/decisions/0010-node-version-management-nvm.md @@ -0,0 +1,19 @@ +# ADR 0010: Node Version Management with nvm and .nvmrc + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +Angular 21 requires Node.js >= 20.19 or >= 22.12. The system Node version on a development machine may not meet this requirement, causing silent build failures or CLI errors. + +## Decision + +A `.nvmrc` file at the repository root pins the Node version to `20.19.6`. Developers use `nvm use` to switch to this version. The Husky pre-commit hook explicitly calls `nvm use 20.19.6` before running tests to ensure the correct version is active in the hook's shell environment. + +## Consequences + +- **Positive:** Consistent Node version across all commands (dev server, build, tests, pre-commit hook). +- **Positive:** `.nvmrc` documents the required Node version explicitly in the repository. +- **Constraint:** Requires nvm. Developers using other version managers (fnm, volta) must align manually. +- **Note:** The `package.json` `engines` field also declares the required Node range (`^20.19.0 || ^22.12.0`) as a secondary signal. diff --git a/docs/decisions/0011-feature-domain-organisation.md b/docs/decisions/0011-feature-domain-organisation.md new file mode 100644 index 0000000..6b4765b --- /dev/null +++ b/docs/decisions/0011-feature-domain-organisation.md @@ -0,0 +1,32 @@ +# ADR 0011: Feature Domain Organisation + +**Date:** 2026-04-26 +**Status:** Accepted + +## Context + +The application covers four independent functional areas, each with its own data model, API backend, and user audience: + +- **Skydive** — jump logbook, statistics, QCM exam preparation. +- **CMS** — articles, pages, user profiles. +- **E-commerce** — product catalogue by category. +- **Hero Wars** — analytics dashboard for a game guild (static weekly snapshots). + +Without an explicit organisational boundary, code from different domains would intermingle, making it harder to reason about and maintain each area independently. + +## Decision + +Services, routes, and API prefixes are organised by domain: + +- Services under `src/app/core/services//` +- Routes composed from `auth.routes.ts` and `noauth.routes.ts`, with domain-specific guards (`authGuard`, `adminGuard`) +- API service domains: `/skydive`, `/cms`, `/ecommerce`, `/herowars` + +This structure mirrors the backend's route grouping (`src/routes/api//`), making the full-stack data flow traceable. + +## Consequences + +- **Positive:** Each domain can be understood and modified independently. +- **Positive:** Consistent mapping between frontend service paths and backend API routes. +- **Negative:** Cross-domain features (e.g. shared auth, user profile) must be placed in `core/` to avoid circular dependencies. +- **Note:** Hero Wars uses static JSON data files (`src/files-data/`) rather than live API calls, which is intentional — the data is updated manually via weekly snapshots. diff --git a/docs/decisions/0012-scss-bootstrap-import-deprecation.md b/docs/decisions/0012-scss-bootstrap-import-deprecation.md new file mode 100644 index 0000000..2de72fc --- /dev/null +++ b/docs/decisions/0012-scss-bootstrap-import-deprecation.md @@ -0,0 +1,29 @@ +# ADR 0012: SCSS Strategy — Bootstrap Variable Overrides and Sass @import Deprecation + +**Date:** 2026-04-26 +**Status:** Accepted (workaround pending Bootstrap 6) + +## Context + +Bootstrap 5 relies on Sass's `@import` rule and `!default` variable overrides: our custom `_variable.scss` must be loaded before Bootstrap's `variables.scss` so that Bootstrap picks up our values via `!default`. + +Dart Sass 1.80+ deprecates `@import` in favour of the `@use` module system. The `@use` system uses isolated module scopes, which breaks the `!default` override mechanism: variables loaded via `@use` in our file do not flow into Bootstrap's module scope. + +Migrating to `@use` for the Bootstrap import chain would require either: + +- Waiting for Bootstrap 6, which is expected to adopt `@use` natively. +- Rewriting the entire Bootstrap variable override mechanism using `@use "bootstrap" with (...)`. + +Neither option is viable in the short term. + +## Decision + +- Suppress the Sass `@import` deprecation warning in `angular.json` via `stylePreprocessorOptions.sass.silenceDeprecations: ["import"]`. +- Keep the existing `@import`-based Bootstrap loading chain intact. +- Fix other Dart Sass deprecations properly: `color.red/green/blue()` replaced with `color.channel()`, global `if()` replaced with `@if`/`@else`. + +## Consequences + +- **Positive:** Bootstrap customisation works correctly with no behaviour change. +- **Negative:** Deprecation silenced rather than fixed. `@import` will be removed in Dart Sass 3.0. +- **Future action:** When Bootstrap 6 is released with `@use` support, migrate the import chain and remove the `silenceDeprecations` entry. Supersede this ADR at that point.