mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-10-23 20:04:27 +00:00
docs(skills): strengthen UI review rules
- Clarify feature-local helper placement - Add UI error-state replacement checklist - Require negative tests for mapper predicates
This commit is contained in:
2 files changed
+56
-1
No files matched your search
@@ -79,8 +79,10 @@ Recharts/library? → CHART_COLORS constant + var()
|
||||
|
||||
### Scope Rule (ABSOLUTE)
|
||||
|
||||
- Used 2+ places → `lib/` or `types/` or `hooks/` (components go in `components/{domain}/`)
|
||||
- Used by unrelated features → `lib/` or `types/` or `hooks/` (components go in `components/{domain}/`)
|
||||
- Used by 2+ files in the same feature/domain → keep it inside that feature/domain, e.g. `_lib/`, `_hooks/`, or local `types.ts`
|
||||
- Used 1 place → keep local in feature directory
|
||||
- **Feature/domain ownership beats raw usage count**: sharing between sibling files does not automatically make a helper global
|
||||
- **This determines ALL folder structure decisions**
|
||||
|
||||
## Project Structure
|
||||
@@ -242,6 +244,27 @@ import { severityLabel } from "@/types/findings";
|
||||
|
||||
If a helper doesn't exist and will be used in 2+ places, add it to `ui/lib/` or `ui/types/` and reuse it. Keep local only if used in exactly one place.
|
||||
|
||||
## Error State Replacement Checklist (REQUIRED)
|
||||
|
||||
When replacing UI error-state logic, remove legacy state fields and branches that the new contract can no longer produce.
|
||||
|
||||
Before finishing the change, check:
|
||||
|
||||
- [ ] Are there flags, union members, buttons, or handlers that are now unreachable?
|
||||
- [ ] Does the UI still render an action for an error case the new API contract cannot emit?
|
||||
- [ ] Does retry appear only for errors where repeating the same action can help?
|
||||
- [ ] Are error mappers matching specific status/code/detail or field pointers rather than broad payload-level pointers?
|
||||
- [ ] Are negative tests present for over-broad mapper/type-guard matches?
|
||||
|
||||
```typescript
|
||||
// ❌ BAD: old branch kept after new mapper never returns needsSignOut
|
||||
if (state.needsSignOut) return <SignInWithDifferentAccount />;
|
||||
|
||||
// ✅ GOOD: remove unreachable state and render only reachable actions
|
||||
{state.canRetry && <Button onClick={retry}>Retry</Button>}
|
||||
<Button asChild><Link href="/sign-in">Go to Sign In</Link></Button>
|
||||
```
|
||||
|
||||
## Derived State Rule (REQUIRED)
|
||||
|
||||
Avoid `useState` + `useEffect` patterns that mirror props or searchParams — they create sync bugs and unnecessary re-renders. Derive values directly from the source of truth.
|
||||
|
||||
@@ -102,6 +102,38 @@ function isUser(value: unknown): value is User {
|
||||
}
|
||||
```
|
||||
|
||||
### Type Guard Tests (REQUIRED)
|
||||
|
||||
When adding or changing a type guard or mapper predicate, test both accepted and rejected shapes. A positive test alone often hides over-broad matching.
|
||||
|
||||
```typescript
|
||||
const ERROR_POINTER = {
|
||||
INVITATION_TOKEN: "/data/attributes/invitation_token",
|
||||
} as const;
|
||||
|
||||
function isInvitationTokenError(error: ApiError): boolean {
|
||||
return error.source?.pointer === ERROR_POINTER.INVITATION_TOKEN;
|
||||
}
|
||||
|
||||
it("should identify invitation token errors", () => {
|
||||
expect(
|
||||
isInvitationTokenError({
|
||||
detail: "Invalid invitation code.",
|
||||
source: { pointer: ERROR_POINTER.INVITATION_TOKEN },
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("should not identify payload-level errors as invitation token errors", () => {
|
||||
expect(
|
||||
isInvitationTokenError({
|
||||
detail: "Invalid request data.",
|
||||
source: { pointer: "/data" },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
```
|
||||
|
||||
## Coupled Optional Props (REQUIRED)
|
||||
|
||||
Do not model semantically coupled props as independent optionals — this allows invalid half-states that compile but break at runtime. Use discriminated unions with `never` to make invalid combinations impossible.
|
||||
|
||||
Reference in new issue
Block a user