diff --git a/docs/typescript/type-safety.md b/docs/typescript/type-safety.md index a557c7e73..1f8653fcf 100644 --- a/docs/typescript/type-safety.md +++ b/docs/typescript/type-safety.md @@ -14,12 +14,14 @@ Type assertions bypass TypeScript's type checking. Instead, use proper type guar ### DOM Elements ❌ **Wrong:** + ```typescript const el = e.target as HTMLInputElement el.value ``` ✅ **Correct:** + ```typescript if (e.target instanceof HTMLInputElement) { e.target.value @@ -29,12 +31,14 @@ if (e.target instanceof HTMLInputElement) { ### Optional Properties on Objects ❌ **Wrong:** + ```typescript const obj = value as { prop?: string } if (obj.prop) { ... } ``` ✅ **Correct:** + ```typescript if ('prop' in value && typeof value.prop === 'string') { value.prop @@ -44,12 +48,14 @@ if ('prop' in value && typeof value.prop === 'string') { ### Constructor/Static Properties ❌ **Wrong:** + ```typescript const ctor = node.constructor as { type?: string } const type = ctor.type ``` ✅ **Correct:** + ```typescript const ctor = node.constructor const type = 'type' in ctor && typeof ctor.type === 'string' ? ctor.type : undefined @@ -64,12 +70,14 @@ When you find yourself needing a cast, ask: "Can I fix the type definition inste If a property exists at runtime but not in the type, add it to the interface: ❌ **Wrong:** + ```typescript const box = this.search_box as { close?: () => void } box?.close?.() ``` ✅ **Correct:** + ```typescript // Update the type definition search_box?: HTMLDivElement & ICloseable @@ -83,6 +91,7 @@ this.search_box?.close() If a callback receives a different type than declared, fix the callback signature: ❌ **Wrong:** + ```typescript onDrawTooltip?: (link: LLink) => void // Later... @@ -90,6 +99,7 @@ onDrawTooltip(link as LLink) // link is actually LinkSegment ``` ✅ **Correct:** + ```typescript onDrawTooltip?: (link: LinkSegment) => void // Later... @@ -117,33 +127,110 @@ for (const panel of panels) { } ``` -## When Casts Are Unavoidable +## Never Use `@ts-ignore` or `@ts-expect-error` -Some casts are genuinely unavoidable. Document why: +These directives suppress all errors on a line, making it easy to accidentally mask serious bugs. Instead: + +1. Fix the underlying type issue +2. Use a type guard to narrow the type +3. If truly unavoidable, use a targeted cast with explanation + +❌ **Wrong:** ```typescript -/** - * @deprecated Prefer {@link structuredClone} - * Note: JSON.parse returns `unknown`, so type assertions are unavoidable here. - * This function is deprecated precisely because it cannot be made type-safe. - */ -cloneObject(obj: T): T { - const cloned: unknown = JSON.parse(JSON.stringify(obj)) - return cloned as T // Unavoidable - JSON.parse returns unknown +// @ts-expect-error - doesn't work otherwise +node.customProperty = value +``` + +✅ **Correct:** + +```typescript +interface ExtendedNode extends LGraphNode { + customProperty?: string +} + +function isExtendedNode(node: LGraphNode): node is ExtendedNode { + return 'customProperty' in node +} + +if (isExtendedNode(node)) { + node.customProperty = value } ``` -### Acceptable Cast Scenarios +## Use `unknown` Instead of `any` -1. **`JSON.parse` results** - Returns `unknown`, must be cast -2. **Conditional types in return positions** - TypeScript can't always narrow these -3. **Third-party library limitations** - When you can't fix the source types +When you don't know the type, use `unknown` and narrow with type guards: + +❌ **Wrong:** + +```typescript +function process(data: any) { + return data.value // No type checking +} +``` + +✅ **Correct:** + +```typescript +function process(data: unknown) { + if (typeof data === 'object' && data !== null && 'value' in data) { + return data.value + } + return undefined +} +``` + +## Use Type Annotations for Object Literals + +Prefer type annotations over assertions for object literals - this catches refactoring bugs: + +❌ **Wrong:** + +```typescript +const config = { + naem: 'test' // Typo not caught +} as Config +``` + +✅ **Correct:** + +```typescript +const config: Config = { + naem: 'test' // Error: 'naem' does not exist on type 'Config' +} +``` + +## Prefer Interfaces Over Type Aliases for Objects + +Use interfaces for object types - they have better error messages and performance: + +❌ **Wrong:** + +```typescript +type NodeConfig = { + id: string + name: string +} +``` + +✅ **Correct:** + +```typescript +interface NodeConfig { + id: string + name: string +} +``` + +Use type aliases for unions, primitives, and tuples where interfaces don't apply. ## Prefer Specific Over General Use the most specific type possible: ❌ **Wrong:** + ```typescript const value = obj as any const value = obj as unknown @@ -151,6 +238,7 @@ const value = obj as Record ``` ✅ **Correct:** + ```typescript // Use the actual expected type const value: ISerialisedNode = obj @@ -163,24 +251,144 @@ if (isSerialisedNode(obj)) { ... } For union types, use proper narrowing instead of casting: ❌ **Wrong:** + ```typescript // NodeId = number | string (node.id as number) - (other.id as number) ``` ✅ **Correct:** + ```typescript if (typeof node.id === 'number' && typeof other.id === 'number') { node.id - other.id } ``` +## Primitive Types + +Use lowercase primitive types, never boxed object types: + +❌ **Wrong:** + +```typescript +function greet(name: String): String +function count(items: Object): Number +``` + +✅ **Correct:** + +```typescript +function greet(name: string): string +function count(items: object): number +``` + +## Callback Types + +### Return Types + +Use `void` for callbacks whose return value is ignored: + +❌ **Wrong:** + +```typescript +interface Options { + onComplete?: () => any +} +``` + +✅ **Correct:** + +```typescript +interface Options { + onComplete?: () => void +} +``` + +### Optional Parameters + +Don't make callback parameters optional unless you intend to invoke the callback with varying argument counts: + +❌ **Wrong:** + +```typescript +interface Callbacks { + onProgress?: (current: number, total?: number) => void +} +``` + +✅ **Correct:** + +```typescript +interface Callbacks { + onProgress?: (current: number, total: number) => void +} +``` + +Callbacks can always ignore parameters they don't need. + +## Function Overloads + +### Ordering + +Put specific overloads before general ones (TypeScript uses first match): + +❌ **Wrong:** + +```typescript +declare function fn(x: unknown): unknown +declare function fn(x: HTMLElement): HTMLElement +``` + +✅ **Correct:** + +```typescript +declare function fn(x: HTMLElement): HTMLElement +declare function fn(x: unknown): unknown +``` + +### Prefer Optional Parameters Over Overloads + +❌ **Wrong:** + +```typescript +interface Example { + diff(one: string): number + diff(one: string, two: string): number + diff(one: string, two: string, three: string): number +} +``` + +✅ **Correct:** + +```typescript +interface Example { + diff(one: string, two?: string, three?: string): number +} +``` + +## Generics + +Never create a generic type that doesn't use its type parameter: + +❌ **Wrong:** + +```typescript +declare function fn(): void +``` + +✅ **Correct:** + +```typescript +declare function fn(arg: T): T +``` + ## Summary -1. **Never use `as`** unless absolutely unavoidable +1. **Never use `as`** outside of custom type guards 2. **Use `instanceof`** for class/DOM element checks 3. **Use `'prop' in obj`** for property existence checks 4. **Use `typeof`** for primitive type checks 5. **Fix source types** instead of casting at usage sites 6. **Create type guards** for repeated patterns -7. **Document unavoidable casts** with clear explanations +7. **Use `void`** for ignored callback returns