chore: add TypeScript annotations to data and core modules #9749
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds TypeScript annotations to improve type safety across the cvat-data and cvat-core modules. The changes focus on refining the decode context image workflow with explicit typing and adding strong typings for lambda manager and CVAT core API surfaces.
- Added explicit TypeScript interfaces and type annotations to worker message handling and decode functions
- Enhanced type safety for lambda manager methods with proper parameter and return type definitions
- Updated CVAT core API surface types to use proper server proxy types instead of generic 'any' types
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cvat-data/src/ts/unzip_imgs.worker.ts | Added UnzipMessage interface and typed onmessage event handler |
| cvat-data/src/ts/cvat-data.ts | Converted decodeContextImages to typed function with interface and fixed worker reference |
| cvat-core/src/webhook.ts | Added proper typing for webhook owner data using SerializedUser |
| cvat-core/src/lambda-manager.ts | Added type annotations for method parameters and return types |
| cvat-core/src/index.ts | Replaced 'any' types with proper server proxy types for API methods |
| cvat-core/src/event.ts | Updated dump method return type to SerializedEvent |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| this.zipWorker.onerror(new ErrorEvent('error', { | ||
| decodeZipWorker.onerror(new ErrorEvent('error', { | ||
| error: event.data.error, | ||
| })); |
There was a problem hiding this comment.
Calling decodeZipWorker.onerror directly is incorrect. The onerror property is an event handler that should be triggered by the browser, not called manually. This should likely be reject(event.data.error) instead to properly handle the error in the promise chain.
| })); | |
| release(); | |
| reject(event.data.error); |
Copilot uses AI. Check for mistakes.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #9749 +/- ##
===========================================
- Coverage 77.24% 75.28% -1.97%
===========================================
Files 408 410 +2
Lines 44758 45187 +429
Branches 4049 4082 +33
===========================================
- Hits 34575 34019 -556
- Misses 10183 11168 +985
🚀 New features to boost your workflow:
|
| }; | ||
| users: { | ||
| get: any; | ||
| get: typeof serverProxy.users.get; |
There was a problem hiding this comment.
The type is not correct. It serverProxy.users.get returns SerializedUser[].
The API method actually returns User[]
| }; | ||
| cloudStorages: { | ||
| get: any; | ||
| get: typeof serverProxy.cloudStorages.get; |
There was a problem hiding this comment.
The type is not correct, check implementation
| get: any; | ||
| activate: any; | ||
| deactivate: any; | ||
| get: typeof serverProxy.organizations.get; |
There was a problem hiding this comment.
The type is not correct, check implementation
| activate: (organization: Organization) => void; | ||
| deactivate: () => void; |
There was a problem hiding this comment.
Not correct, both return promises
| }; | ||
| webhooks: { | ||
| get: any; | ||
| get: typeof serverProxy.webhooks.get; |
There was a problem hiding this comment.
Type is not correct, check implementation
|
|
||
| export interface TrackerResults { | ||
| states: any[]; | ||
| states: Record<string, unknown>[]; |
There was a problem hiding this comment.
That seems to be a array, not array of object
| } | ||
|
|
||
| async requests(): Promise<any[]> { | ||
| async requests(): Promise<Record<string, unknown>[]> { |
There was a problem hiding this comment.
The type might be much more specific, as described in API scheme
| description?: string; | ||
| is_active?: boolean; | ||
| owner?: any; | ||
| owner?: SerializedUser | User; |
There was a problem hiding this comment.
owner is always SerializedUser here or null.
Summary
Testing
npx tsc --noEmit --skipLibCheck(cvat-data)npx tsc --noEmit --skipLibCheck(cvat-core) (fails: numerous TS errors)https://chatgpt.com/codex/tasks/task_b_689c5ef03ec0832c83f793cf6138dd84