Missing closing braces caused Spring to inject the literal placeholder string instead of resolving the property, silently ignoring any app.admin.username / app.admin.password env-var overrides. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
149 lines
6.3 KiB
Markdown
149 lines
6.3 KiB
Markdown
# Frontend TODO
|
||
|
||
Findings from architectural review. Ordered roughly by severity.
|
||
|
||
---
|
||
|
||
## Bugs
|
||
|
||
### Backend URL hardcoded as `localhost` in the session hook
|
||
**File:** `src/hooks.server.ts:20`
|
||
```ts
|
||
const response = await fetch('http://localhost:8080/api/users/me', {
|
||
```
|
||
The `userGroup` handle (which runs on every request) calls the backend via a hardcoded `localhost:8080` URL. The `API_INTERNAL_URL` env var used in `handleFetch` is not applied here. Inside Docker, `localhost` from the frontend container does not resolve to the backend container — requests will fail silently (the catch swallows the error) and `event.locals.user` will be `undefined` for every request.
|
||
|
||
**Fix:** Centralise the base URL:
|
||
```ts
|
||
const API_BASE = env.API_INTERNAL_URL ?? 'http://localhost:8080';
|
||
// then use it in both userGroup and handleFetch
|
||
```
|
||
|
||
---
|
||
|
||
### `import { env } from 'process'` bypasses SvelteKit's env system
|
||
**File:** `src/hooks.server.ts:4`
|
||
```ts
|
||
import { env } from 'process';
|
||
```
|
||
This is a raw Node.js import that bypasses SvelteKit's `$env/dynamic/private` and `$env/static/private` modules. It won't work if the adapter is ever changed (e.g., to Deno or a serverless edge runtime), and it skips SvelteKit's build-time validation that env vars are actually set.
|
||
|
||
**Fix:**
|
||
```ts
|
||
import { env } from '$env/dynamic/private';
|
||
```
|
||
|
||
---
|
||
|
||
## Design Issues
|
||
|
||
### Every page load hits the backend twice minimum
|
||
**File:** `src/hooks.server.ts:15–34`
|
||
|
||
The `userGroup` hook calls `GET /api/users/me` on every single server-side request to check the session. Then each page's `+page.server.ts` load function makes its own API calls. For a search page that's three sequential backend round-trips before the user sees anything.
|
||
|
||
**Fix options:**
|
||
- Cache the user in the session (if Spring Session is adopted on the backend) — validate the session cookie once, not per request
|
||
- If sticking with Basic Auth: trust the cookie value directly and only call `/api/users/me` when the user object is actually needed (e.g., in `+layout.server.ts`), not unconditionally in the hook
|
||
|
||
---
|
||
|
||
### Two conflicting package managers
|
||
**Files:** `package-lock.json` (npm), `yarn.lock` (Yarn)
|
||
|
||
Both lock files exist, meaning the project has been installed with both npm and Yarn at different times. This leads to divergent dependency trees and confusing contributor setup.
|
||
|
||
**Fix:** Pick one (npm is already the default in the devcontainer), delete the other lock file, and add an `.npmrc` or `package.json` `engines` field to enforce it:
|
||
```json
|
||
"engines": { "npm": ">=10" }
|
||
```
|
||
|
||
---
|
||
|
||
### i18n is a stub — only German is implemented
|
||
**Files:** `messages/en.json`, `messages/es.json`
|
||
|
||
English and Spanish message files contain only the scaffolding example key `hello_world`. All actual UI strings are rendered directly in German inside Svelte components and are not extracted into the message catalogue.
|
||
|
||
**Fix options:**
|
||
- If multi-language support is a real requirement: extract all German strings from `.svelte` files into `messages/de.json` and provide translations in `en.json` / `es.json`
|
||
- If it's not a requirement: remove Paraglide, the `messages/` directory, and the i18n hook to reduce complexity
|
||
|
||
---
|
||
|
||
### No `Dockerfile` — frontend cannot run in Docker
|
||
**File:** `docker-compose.yml` (frontend service commented out)
|
||
|
||
The frontend service in `docker-compose.yml` is commented out because no `Dockerfile` exists. The SvelteKit Node adapter is already configured, so producing a Dockerfile is straightforward.
|
||
|
||
**Fix:** Add `frontend/Dockerfile`:
|
||
```dockerfile
|
||
FROM node:24-alpine AS builder
|
||
WORKDIR /app
|
||
COPY package*.json ./
|
||
RUN npm ci
|
||
COPY . .
|
||
RUN npm run build
|
||
|
||
FROM node:24-alpine
|
||
WORKDIR /app
|
||
COPY --from=builder /app/build ./build
|
||
COPY --from=builder /app/package.json .
|
||
EXPOSE 3000
|
||
CMD ["node", "build"]
|
||
```
|
||
Then uncomment and complete the frontend service in `docker-compose.yml`, adding `API_INTERNAL_URL=http://backend:8080`.
|
||
|
||
---
|
||
|
||
### Demo routes are leftover scaffolding
|
||
**Files:** `src/routes/demo/+page.svelte`, `src/routes/demo/paraglide/+page.svelte`
|
||
|
||
These routes were generated by the SvelteKit + Paraglide scaffolding tool. They serve no function in the application and are publicly accessible.
|
||
|
||
**Fix:** Delete `src/routes/demo/` entirely.
|
||
|
||
---
|
||
|
||
## Missing Capabilities
|
||
|
||
### No shared TypeScript types with the backend
|
||
The frontend manually constructs objects and parses JSON responses without any type definitions that match the backend's DTOs (`DocumentUpdateDTO`, `GroupDTO`, etc.). A breaking change in the backend API — renaming a field, changing a type — won't be caught until runtime.
|
||
|
||
**Fix options (pick one):**
|
||
- **Simple:** Define matching TypeScript interfaces manually in `src/lib/types.ts` for all backend response shapes and use them in load functions
|
||
- **Robust:** Add `springdoc-openapi` to the backend and use `openapi-typescript` to generate types from the OpenAPI spec as part of the build pipeline
|
||
|
||
---
|
||
|
||
### No global error page
|
||
SvelteKit renders a default unstyled error page for unhandled errors and 404s. There is no `src/routes/+error.svelte` in the project.
|
||
|
||
**Fix:** Create `src/routes/+error.svelte` with the application's layout and a user-friendly message. Use SvelteKit's `$page.status` and `$page.error` stores to display appropriate messages for 404 vs 500.
|
||
|
||
---
|
||
|
||
### No loading state during navigation
|
||
Long-running searches or slow backend responses give the user no visual feedback. SvelteKit's `$navigating` store is available but appears unused.
|
||
|
||
**Fix:** Add a global navigation progress indicator in `+layout.svelte`:
|
||
```svelte
|
||
<script>
|
||
import { navigating } from '$app/stores';
|
||
</script>
|
||
{#if $navigating}
|
||
<div class="progress-bar" />
|
||
{/if}
|
||
```
|
||
|
||
---
|
||
|
||
### `handleFetch` redirect inside a hook can cause silent failures
|
||
**File:** `src/hooks.server.ts:46`
|
||
```ts
|
||
throw redirect(302, '/login');
|
||
```
|
||
Throwing a redirect inside `handleFetch` during a server-side load that makes multiple API calls (e.g., the search page calling both `/api/documents/search` and `/api/persons`) means only the first missing-token fetch triggers the redirect — subsequent ones may behave unpredictably.
|
||
|
||
**Fix:** Move the auth guard to `+layout.server.ts` as the single authoritative redirect point, and let `handleFetch` simply pass through without the token (letting the backend return 401). Handle 401 responses from the backend in load functions with an explicit `redirect(302, '/login')`.
|