OpenVelo Agent
87 lines
4.9 KiB
Markdown
87 lines
4.9 KiB
Markdown
# Implementation Plan: Fix Angular build error — Property 'code' does not exist on type 'CreateAdminResult'
|
|
|
|
## 1. Architectural Reconnaissance
|
|
|
|
- **Codebase style & conventions:** Angular 17 standalone components, signals, typed reactive forms. `frontend/tsconfig.json` uses `strict: false` and `strictTemplates: false`. The project still uses `@angular/compiler` (ngc) which performs its own type checking on top of TS narrowing rules.
|
|
- **Data Layer:** N/A — this is a frontend-only type-narrowing fix.
|
|
- **Test Framework & Structure:** `tests/jest.config.js`. Tests live under `tests/frontend/`. All compile via `npm test`.
|
|
- **Required Tools & Dependencies:** No new dependencies. The fix is purely TypeScript / component-internal.
|
|
|
|
## 2. Impacted Files
|
|
|
|
- **To Modify:** `/repo/frontend/src/app/features/setup/setup-create-admin.component.ts` — fix the type-narrowing that Angular's compiler refuses to accept.
|
|
- **No new files, no test changes, no docs changes.**
|
|
|
|
## 3. Proposed Changes
|
|
|
|
### Root cause
|
|
|
|
In `setup-create-admin.component.ts:88-117` the `submit()` method awaits the service, then reads `result.ok`. After the `if (result.ok) { ... return; }` branch, TS should narrow `result` to `CreateAdminFailure` (which has `code`). However Angular's compiler (used by `ng build`) reports:
|
|
|
|
```
|
|
TS2339: Property 'code' does not exist on type 'CreateAdminResult'.
|
|
Property 'code' does not exist on type 'CreateAdminSuccess'.
|
|
```
|
|
|
|
at the two reads `result.code` on lines 109 and 116. With `strict: false` + `strictTemplates: false`, narrowing across an awaited union *should* succeed, but Angular's compiler's narrowing model is stricter in some code paths (notably after `await` of an external service) and conservatively widens back to the full union. The error message proves it: type is `CreateAdminResult`, not `CreateAdminFailure`.
|
|
|
|
### Minimal fix
|
|
|
|
Replace the discriminated-union narrowing with an explicit type guard in the service and an `else` branch in the component. This avoids relying on the compiler's narrowing at all.
|
|
|
|
### Step-by-step
|
|
|
|
1. **Add a type predicate to the service** (`setup-create-admin.service.ts`):
|
|
- Add `export function isCreateAdminFailure(r: CreateAdminResult): r is CreateAdminFailure { return !r.ok; }`.
|
|
- No behaviour change.
|
|
|
|
2. **Refactor `submit()`** in `setup-create-admin.component.ts`:
|
|
- Replace the `if (result.ok) { ... return; } ... if (result.code === ...) ...` sequence with an `if (result.ok) { ... return; } const failure = result;` pattern. Because `result.ok` is `true | false`, the negation narrows the local copy identically. The safest cross-compiler approach is:
|
|
```ts
|
|
if (result.ok) {
|
|
this.auth.setSession(result.accessToken, result.user);
|
|
this.bootstrap.markInitialized();
|
|
await this.router.navigateByUrl('/challenges');
|
|
return;
|
|
}
|
|
const failure: CreateAdminFailure = result; // explicit local of narrowed type
|
|
if (failure.code === 'USERNAME_TAKEN') {
|
|
this.serverError.set('Username already exists');
|
|
this.serverErrorCode.set('USERNAME_TAKEN');
|
|
return;
|
|
}
|
|
this.serverError.set('Setup failed, please retry');
|
|
this.serverErrorCode.set(failure.code);
|
|
```
|
|
- Import `CreateAdminFailure` from `./setup-create-admin.service`.
|
|
- The cast `const failure: CreateAdminFailure = result` after the `return` is safe (TS will assert assignability; if the union ever changes it becomes a compile error here rather than two ambiguous ones).
|
|
|
|
Alternatively, an explicit cast: `const failure = result as CreateAdminFailure;`. Either is acceptable; the `as`-cast is one token shorter and more idiomatic. Choose:
|
|
|
|
```ts
|
|
const failure = result as CreateAdminFailure;
|
|
```
|
|
|
|
3. **No HTML/CSS/service file changes.**
|
|
|
|
## 4. Test Strategy
|
|
|
|
- **Target Unit Test File:** no changes. The existing `tests/frontend/setup-create-admin.spec.ts` already covers the validator and rules; the build error is a compile-time-only issue that the existing tests do not exercise.
|
|
- **Mocking Strategy:** N/A.
|
|
- **Verification:**
|
|
1. `npm --workspace frontend run build` must complete without `TS2339` errors.
|
|
2. `npm test` must still pass (20 suites, 97 tests).
|
|
- **Do not** touch any test file. The fix is internal to the component.
|
|
|
|
## 5. Risk / Non-Goals
|
|
|
|
- **No behaviour change.** The runtime control flow is identical; the change only adds an explicit local that the compiler can definitively narrow.
|
|
- **No public API change.** `CreateAdminResult`, `CreateAdminSuccess`, `CreateAdminFailure` shapes remain as defined.
|
|
- **No docs change required** — implementation detail only.
|
|
|
|
## 6. Execution Checklist
|
|
|
|
1. Edit `setup-create-admin.component.ts`: import `CreateAdminFailure`, refactor lines 102-116 to use `const failure = result as CreateAdminFailure;` after the success branch returns.
|
|
2. Run `npm --workspace frontend run build` and confirm 0 errors.
|
|
3. Run `npm test` and confirm `Test Suites: 20 passed, 20 total`.
|