Conversation
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens TypeScript typing in the browser-based repo review webapp (React + MUI + Pyodide), primarily by replacing any with explicit interfaces and Pyodide types, and adding the pyodide npm package for type availability.
Changes:
- Added explicit TS interfaces/types for app state, check results, and MUI event handlers in
webapp.tsx. - Typed the Pyodide integration helpers (
prepare_pyodide,run_process,load_known_checks,generate_html) inutils/pyodide.ts. - Updated frontend devDependencies/lockfile to include
pyodide(and re-ordered TypeScript ESLint deps).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/repo-review-app/webapp.tsx | Adds concrete app/result types and event typings; adjusts error handling and HTML copy flow. |
| src/repo-review-app/utils/pyodide.ts | Introduces Pyodide type imports and typed function signatures for Pyodide runtime helpers. |
| package.json | Adds pyodide to devDependencies and includes TypeScript ESLint plugin/parser entries. |
| bun.lock | Locks newly added pyodide (and transitive deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| interface AppState { | ||
| show: string; | ||
| results: Record<string, CheckItem[]> | CheckItem[]; | ||
| pyFamilies: PyProxy | null; | ||
| pyChecks: PyProxy | null; | ||
| snackbarOpen: boolean; | ||
| snackbarMsg: string; | ||
| snackbarSeverity: "info" | "error" | "warning" | "success"; | ||
| repo: string; | ||
| ref: string; | ||
| refType: "branch" | "tag"; | ||
| refs: Refs; | ||
| msg: string; | ||
| progress: boolean; | ||
| loadingRefs: boolean; | ||
| err_msg: string; | ||
| skip_reason: string; | ||
| url: string; | ||
| knownChecks: Record<string, CheckItem[]> | null; | ||
| knownFamilies: Record<string, { name: string; description?: string }>; | ||
| infoOpen: boolean; | ||
| pyodideProgress: number; | ||
| pyodideLoading: boolean; | ||
| pyodideMessage?: string; | ||
| } |
There was a problem hiding this comment.
AppState doesn't declare a families field, but the component reads this.state.families in render() and sets it in handleCompute(). This will fail type-checking and makes state shape unclear; add families: Record<string, { name: string; description?: string }> to AppState and initialize it in the constructor state.
| // first keep items that are not passing (i.e., state !== true) | ||
| let kept = items.filter((it: any) => it.state !== true); | ||
| let kept = items.filter((it: CheckItem) => it.state !== true); | ||
| // if 'err', then keep only those that are not undefined (i.e., errors only) | ||
| if (this.state.show === "err") { | ||
| kept = kept.filter((it: any) => it.state !== undefined); | ||
| kept = kept.filter((it: CheckItem) => it.state !== undefined); | ||
| } |
There was a problem hiding this comment.
The --show=err filter doesn't actually filter to errors only: after removing passing checks, it keeps any item where state !== undefined, which includes entries with state === null (e.g., from loadKnownChecks) and would be shown as errors. If err is meant to show failures only, filter with state === false (and ensure the non-run/known-check state uses a distinct value that won't be treated as an error).
| repo: string; | ||
| ref: string; | ||
| refType: "branch" | "tag"; | ||
| refs: Refs; |
There was a problem hiding this comment.
AppState.refType is a strict union ("branch" | "tag"), but handleRefChange(refType: string) (and the Autocomplete onChange cast option.type as string) will still flow arbitrary strings into state. Tighten handleRefChange's parameter type to match the union and avoid the as string cast so the new typings actually enforce valid values.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Adding more typescript types, using gpt-5-mini in copilot CLI.