|
| 1 | +--- |
| 2 | +name: removing-typescript-suppressions |
| 3 | +description: Replaces @ts-expect-error and @ts-ignore directives with minimal type-safe fixes. Use when removing TypeScript compiler suppressions. |
| 4 | +--- |
| 5 | + |
| 6 | +# Removing TypeScript suppressions |
| 7 | + |
| 8 | +Replace suppressions by repairing the contract that caused the compiler error. |
| 9 | +Do not silence the error elsewhere or widen types beyond the real runtime shape. |
| 10 | + |
| 11 | +## Workflow |
| 12 | + |
| 13 | +### 1. Establish the scope |
| 14 | + |
| 15 | +Read `docs/guidance/typescript.md`, `docs/guidance/engineering.md`, and the |
| 16 | +guidance for the affected files. Identify the correct comparison ref and list |
| 17 | +only suppressions introduced by the change under review: |
| 18 | + |
| 19 | +```bash |
| 20 | +git diff --unified=2 <base-ref>...HEAD -- <paths> \ |
| 21 | + | rg -n '^\+.*@ts-(expect-error|ignore)|^diff --git|^@@' |
| 22 | +``` |
| 23 | + |
| 24 | +Do not expand the task to old suppressions unless the user asks. Preserve a |
| 25 | +suppression used deliberately to test a compiler error when that error is the |
| 26 | +behavior under test. |
| 27 | + |
| 28 | +### 2. Read the ownership path |
| 29 | + |
| 30 | +Read each affected file and trace the value to its source type, runtime owner, |
| 31 | +and consumers. Check declarations, store types, generated API types, library |
| 32 | +return types, test globals, and the relevant `tsconfig` before choosing a fix. |
| 33 | + |
| 34 | +Do not accept the suppression comment as the diagnosis. Verify the failing code |
| 35 | +path and runtime shape. |
| 36 | + |
| 37 | +### 3. Expose the real errors |
| 38 | + |
| 39 | +Remove the in-scope suppressions, then run the narrowest typecheck that owns the |
| 40 | +files. Use the compiler diagnostics to group failures by root cause instead of |
| 41 | +patching each line independently. |
| 42 | + |
| 43 | +Common commands in this repository: |
| 44 | + |
| 45 | +```bash |
| 46 | +pnpm typecheck |
| 47 | +pnpm typecheck:browser |
| 48 | +pnpm typecheck:desktop |
| 49 | +pnpm typecheck:scripts |
| 50 | +pnpm typecheck:website |
| 51 | +``` |
| 52 | + |
| 53 | +### 4. Repair the contract |
| 54 | + |
| 55 | +Prefer, in order: |
| 56 | + |
| 57 | +1. correct types at the source |
| 58 | +2. control-flow narrowing |
| 59 | +3. an existing domain type or generated API type |
| 60 | +4. `unknown` at a genuine compatibility boundary, followed by runtime narrowing |
| 61 | + |
| 62 | +Avoid `any`, `as any`, a replacement assertion, a broader optional type, or a |
| 63 | +new wrapper that merely hides the mismatch. Keep public API types stable and do |
| 64 | +not expose internal store types through public facades. |
| 65 | + |
| 66 | +### 5. Verify behavior and absence |
| 67 | + |
| 68 | +Run the owning typecheck, focused tests, lint, formatting, and a whitespace |
| 69 | +check. Use `pnpm exec vitest run` for Vitest, `pnpm test:browser:local` or |
| 70 | +`pnpm test:browser` for Playwright, and the owning repository script for other |
| 71 | +test types. Confirm that the diff adds no suppression: |
| 72 | + |
| 73 | +```bash |
| 74 | +pnpm exec eslint <changed-files> |
| 75 | +pnpm exec oxfmt --check <changed-files> |
| 76 | +git diff --check <base-ref>...HEAD -- <paths> |
| 77 | +git diff --unified=0 <base-ref>...HEAD -- <paths> \ |
| 78 | + | rg '^\+.*@ts-(expect-error|ignore)' |
| 79 | +``` |
| 80 | + |
| 81 | +The final `rg` command should return no matches. Run broader checks when the fix |
| 82 | +changes a shared type, public contract, store, or cross-package boundary. |
| 83 | + |
| 84 | +## Repair patterns |
| 85 | + |
| 86 | +### Narrow optional browser globals once |
| 87 | + |
| 88 | +Copy an optional global to a local and guard it. This preserves narrowing across |
| 89 | +callbacks and asynchronous code. |
| 90 | + |
| 91 | +```typescript |
| 92 | +const app = window.app |
| 93 | +if (!app) throw new Error('ComfyUI app is not initialized') |
| 94 | + |
| 95 | +await app.api.getNodeDefs() |
| 96 | +``` |
| 97 | + |
| 98 | +### Use the owner instead of casting a facade |
| 99 | + |
| 100 | +When a public facade intentionally omits internal collections, import the store |
| 101 | +or service that owns those collections. Do not widen the facade or cast through |
| 102 | +it for one caller. |
| 103 | + |
| 104 | +### Narrow value-or-factory unions |
| 105 | + |
| 106 | +Resolve callbacks before using their values: |
| 107 | + |
| 108 | +```typescript |
| 109 | +const label = |
| 110 | + typeof command.label === 'function' ? command.label() : command.label |
| 111 | +``` |
| 112 | + |
| 113 | +Resolve a factory default before passing it to another resolver. Keep untyped |
| 114 | +legacy data as `unknown`. Narrow objects before reading properties, functions |
| 115 | +before calling them, and returned values before use. |
| 116 | + |
| 117 | +### Read map identity from `Object.entries` |
| 118 | + |
| 119 | +If an object's key is the identifier, do not invent an `id` property on its |
| 120 | +values: |
| 121 | + |
| 122 | +```typescript |
| 123 | +Object.entries(dialogs).map(([id, dialog]) => ({ id, title: dialog.title })) |
| 124 | +``` |
| 125 | + |
| 126 | +### Handle nullable factories before dereferencing |
| 127 | + |
| 128 | +Respect library return types such as `LiteGraph.createNode(): LGraphNode | null`: |
| 129 | + |
| 130 | +```typescript |
| 131 | +const node = liteGraph.createNode(nodeName, displayName) |
| 132 | +if (!node?.widgets?.length) return {} |
| 133 | +``` |
| 134 | + |
| 135 | +Use optional chaining only when the missing value and the empty value have the |
| 136 | +same behavior. Otherwise, use a guard with a useful error. |
| 137 | + |
| 138 | +### Use named payloads across callback boundaries |
| 139 | + |
| 140 | +Heterogeneous array arguments often lose positional types in `page.evaluate` |
| 141 | +and similar APIs. Pass an object instead of asserting a tuple: |
| 142 | + |
| 143 | +```typescript |
| 144 | +await page.evaluate( |
| 145 | + ({ nodeName, displayName, inputNames }) => { |
| 146 | + // use the independently typed fields |
| 147 | + }, |
| 148 | + { nodeName, displayName, inputNames } |
| 149 | +) |
| 150 | +``` |
| 151 | + |
| 152 | +### Validate serialization boundaries |
| 153 | + |
| 154 | +Type values produced by code you own before passing them to |
| 155 | +`Object.fromEntries`. If JSON or browser data arrives as `unknown`, validate its |
| 156 | +full nested shape before assigning a domain type. A result annotation does not |
| 157 | +validate data. |
| 158 | + |
| 159 | +### Make test preconditions executable |
| 160 | + |
| 161 | +If a test needs optional output, narrow it with a runtime guard that produces a |
| 162 | +clear behavioral failure: |
| 163 | + |
| 164 | +```typescript |
| 165 | +if (!serialized.inputs || !serialized.outputs) { |
| 166 | + throw new Error('Expected serialized node labels') |
| 167 | +} |
| 168 | +``` |
| 169 | + |
| 170 | +Do not replace the suppression with a non-null assertion. |
| 171 | + |
| 172 | +### Install minimal test globals without claiming full DOM types |
| 173 | + |
| 174 | +When Node tests need a small browser shim, add the property to the host object |
| 175 | +instead of assigning `{}` to a full `Window` type: |
| 176 | + |
| 177 | +```typescript |
| 178 | +if (typeof window === 'undefined') { |
| 179 | + Object.assign(globalThis, { window: {} }) |
| 180 | +} |
| 181 | +``` |
| 182 | + |
| 183 | +### Delete documentation-only values |
| 184 | + |
| 185 | +If strict checking reports an unused constant kept only as documentation, |
| 186 | +delete it. Put useful context on the value that the code actually checks. |
| 187 | + |
| 188 | +## Guardrails |
| 189 | + |
| 190 | +- Do not hand-declare server response types. Import generated shared types. |
| 191 | +- Do not grow `ExtensionManager` or another public API to expose private state. |
| 192 | +- Put reusable type guards in leaf modules with runtime-free `import type` |
| 193 | + dependencies. |
| 194 | +- Do not change runtime behavior while repairing types unless the old code was |
| 195 | + demonstrably inconsistent with its runtime contract. Cover such a bug with a |
| 196 | + focused test. |
| 197 | +- Treat review findings and their proposed fixes as claims. Reproduce the error |
| 198 | + against current code, then fix or reject each finding with concrete evidence. |
0 commit comments