-
Notifications
You must be signed in to change notification settings - Fork 0
[WIP] Document data and metadata API for frontend use #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ctql/types Co-authored-by: huangyiirene <7665279+huangyiirene@users.noreply.github.com>
Co-authored-by: huangyiirene <7665279+huangyiirene@users.noreply.github.com>
Co-authored-by: huangyiirene <7665279+huangyiirene@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces comprehensive API type definitions to enable type-safe frontend integration with ObjectQL's Data and Metadata APIs. The changes establish a clear contract between backend and frontend applications through the @objectql/types package.
Changes:
- Added complete TypeScript interfaces for Data API and Metadata API in
@objectql/types - Implemented REST-based client classes (
DataApiClientandMetadataApiClient) in the SDK package - Created extensive usage documentation with React and Vue.js integration examples
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/foundation/types/src/api.ts | Defines API request/response types, error codes, pagination metadata, and client interfaces |
| packages/foundation/types/src/index.ts | Exports the new API types for consumption |
| packages/drivers/sdk/src/index.ts | Implements DataApiClient and MetadataApiClient using the defined types |
| docs/api/client-sdk.md | Provides comprehensive usage guide with CRUD examples, React hooks, Vue composables, and best practices |
packages/foundation/types/src/api.ts
Outdated
| export interface DataApiResponse<T = any> { | ||
| /** Error information if the operation failed */ | ||
| error?: ApiError; | ||
| /** Additional response fields for successful operations */ | ||
| [key: string]: any; | ||
| } |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of any in the generic default T = any and in the index signature [key: string]: any reduces type safety. Consider using unknown instead of any to force proper type checking at usage sites, or provide more specific type constraints where the response structure is known.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 890b156. Replaced all any types with unknown throughout the API definitions.
packages/foundation/types/src/api.ts
Outdated
| export interface DataApiListParams { | ||
| /** Filter expression (can be FilterExpression array or JSON string) */ | ||
| filter?: any; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filter property is typed as any, which bypasses TypeScript's type checking. Consider defining a proper filter type (e.g., FilterExpression or similar) to maintain type safety, or at minimum use unknown to require type assertions at usage sites.
| export interface DataApiListParams { | |
| /** Filter expression (can be FilterExpression array or JSON string) */ | |
| filter?: any; | |
| export type DataApiFilterInput = unknown | string; | |
| export interface DataApiListParams { | |
| /** Filter expression (can be FilterExpression array or JSON string) */ | |
| filter?: DataApiFilterInput; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit b9ed832. Updated to use FilterExpression | string type from query.ts for better type safety.
packages/foundation/types/src/api.ts
Outdated
| */ | ||
| export interface DataApiBulkUpdateRequest { | ||
| /** Filter criteria to select records to update */ | ||
| filters: any; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filters property uses any type. Consider using a more specific type like FilterExpression[] or the filter type used elsewhere in the codebase for consistency and type safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit b9ed832. Updated to use FilterExpression type consistently.
packages/foundation/types/src/api.ts
Outdated
| */ | ||
| export interface DataApiBulkDeleteRequest { | ||
| /** Filter criteria to select records to delete */ | ||
| filters: any; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filters property uses any type. This should match the filter type definition used in DataApiBulkUpdateRequest and other filter-related interfaces for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit b9ed832. Updated to use FilterExpression type matching other filter-related interfaces.
packages/drivers/sdk/src/index.ts
Outdated
| const json = await response.json(); | ||
|
|
||
| if (json.error) { | ||
| throw new Error(`${json.error.code}: ${json.error.message}`); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the coding guidelines, errors should be thrown as ObjectQLError from @objectql/types rather than generic Error. Update to: throw new ObjectQLError({ code: json.error.code, message: json.error.message })
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit b9ed832. Updated to throw ObjectQLError with structured error details.
packages/drivers/sdk/src/index.ts
Outdated
| const json = await response.json(); | ||
|
|
||
| if (json.error) { | ||
| throw new Error(`${json.error.code}: ${json.error.message}`); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the coding guidelines, errors should be thrown as ObjectQLError from @objectql/types rather than generic Error. Update to: throw new ObjectQLError({ code: json.error.code, message: json.error.message })
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit b9ed832. Updated to throw ObjectQLError with structured error details.
|
@copilot 帮我修正 |
|
@copilot 继续 |
…ctQLError class Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…ror type safety Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
✅ All code review feedback has been addressed: Commits:
Changes:
The PR is now ready for final review. |
|
@copilot 2026-01-14T13:47:48.4150260Z ##[group]Run pnpm run build |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 继续 |
|
@copilot 继续 |
The CI build error has been fixed in commit 7b3ec7f. The issue was that The types package builds successfully now. |
|
@copilot fix pnpm test error |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in commit a1aa07f. Updated the jest module mapper in SDK package to correctly resolve
This should resolve the test import errors. |
Plan: Add API Type Definitions for Frontend
api.tsfile in@objectql/typeswith:index.tsOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.