mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-09 17:40:09 +00:00
docs: add type safety guidelines from as-any remediation
Lessons learned from purging 40 `as any` instances from browser_tests. TypeScript guidance additions: - Type assertion hierarchy (from virtuous to forbidden) - Zod schema rules (no z.any(), no test settings in production) - Public API contract preservation Playwright guidance additions: - Acceptable vs forbidden type assertion patterns - Test-local type helpers pattern (e.g., TestSettingId) - How to access internal store state without polluting APIs Amp-Thread-ID: https://ampcode.com/threads/T-019c1833-2352-728b-a523-a8f440fd3ba1 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -29,6 +29,52 @@ This is the **only context** where non-null assertions are acceptable.
|
||||
**TODO:** Consolidate these references into a central utility (e.g., `getApp()`) that
|
||||
performs proper runtime type checking, removing the need for scattered `!` assertions.
|
||||
|
||||
## Type Assertions in E2E Tests
|
||||
|
||||
E2E tests may use **specific** type assertions when needed, but **never** `as any`.
|
||||
|
||||
### Acceptable Patterns
|
||||
|
||||
```typescript
|
||||
// ✅ Non-null assertions for window globals
|
||||
window.app!.extensionManager
|
||||
|
||||
// ✅ Specific type assertions with documentation
|
||||
// Extensions can register arbitrary setting IDs
|
||||
id: 'TestSetting' as TestSettingId
|
||||
|
||||
// ✅ Test-local type helpers
|
||||
type TestSettingId = keyof Settings
|
||||
```
|
||||
|
||||
### Forbidden Patterns
|
||||
|
||||
```typescript
|
||||
// ❌ Never use `as any`
|
||||
settings: testData as any
|
||||
|
||||
// ❌ Never modify production types to satisfy test errors
|
||||
// Don't add test settings to src/schemas/apiSchema.ts
|
||||
|
||||
// ❌ Don't chain through unknown to bypass types
|
||||
data as unknown as SomeType // Use sparingly, document why
|
||||
```
|
||||
|
||||
### Accessing Internal State
|
||||
|
||||
When tests need internal store properties (e.g., `.workflow`, `.focusMode`):
|
||||
|
||||
```typescript
|
||||
// ✅ Access stores directly in page.evaluate
|
||||
await page.evaluate(() => {
|
||||
const store = useWorkflowStore()
|
||||
return store.activeWorkflow
|
||||
})
|
||||
|
||||
// ❌ Don't change public API types to expose internals
|
||||
// Keep app.extensionManager typed as ExtensionManager, not WorkspaceStore
|
||||
```
|
||||
|
||||
## Test Tags
|
||||
|
||||
Tags are respected by config:
|
||||
|
||||
@@ -14,6 +14,28 @@ globs:
|
||||
- Type assertions are a last resort; they lead to brittle code
|
||||
- Avoid `@ts-expect-error` - fix the underlying issue instead
|
||||
|
||||
### Type Assertion Hierarchy
|
||||
|
||||
When you must handle uncertain types, prefer these approaches in order:
|
||||
|
||||
1. ✅ **No assertion** — Properly typed from the start
|
||||
2. ✅ **Type narrowing** — `if ('prop' in obj)` or type guards
|
||||
3. ⚠️ **Specific assertion** — `as SpecificType` when you truly know the type
|
||||
4. ⚠️ **`unknown` with narrowing** — For genuinely unknown data
|
||||
5. ❌ **`as any`** — FORBIDDEN
|
||||
|
||||
### Zod Schema Rules
|
||||
|
||||
- Never use `z.any()` — it disables validation and propagates `any` into types
|
||||
- Use `z.unknown()` if the type is genuinely unknown, then narrow it
|
||||
- Never add test-only settings/types to production schemas
|
||||
|
||||
### Public API Contracts
|
||||
|
||||
- Keep public API types stable (e.g., `ExtensionManager` interface)
|
||||
- Don't expose internal implementation types (e.g., Pinia store internals)
|
||||
- Reactive refs (`ComputedRef<T>`) should be unwrapped before exposing
|
||||
|
||||
## Utility Libraries
|
||||
|
||||
- Use `es-toolkit` for utility functions (not lodash)
|
||||
|
||||
Reference in New Issue
Block a user