-
Notifications
You must be signed in to change notification settings - Fork 0
Integrate ObjectStack Protocol (spec@0.1.2, client@0.1.1) into platform core #128
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@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 integrates ObjectStack Protocol packages (@objectstack/spec@^0.1.2 and @objectstack/client@^0.1.1) into ObjectUI's core architecture, establishing protocol conformance and enabling native ObjectStack backend connectivity.
Changes:
- Added ObjectStack dependencies to
@object-ui/types,@object-ui/core, and@object-ui/data-objectqlpackages - Implemented new
ObjectStackAdapterclass that bridges the ObjectStack Client SDK with ObjectUI'sDataSourceinterface - Updated build configuration to skip TypeScript diagnostics for workspace dependencies
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/package.json | Added @objectstack/spec@^0.1.2 dependency |
| packages/types/README.md | Updated version reference from 0.1.1 to 0.1.2 |
| packages/plugin-kanban/vite.config.ts | Added skipDiagnostics and test file exclusions to build config |
| packages/plugin-charts/vite.config.ts | Added skipDiagnostics and test file exclusions to build config |
| packages/data-objectql/package.json | Added @objectstack/client@^0.1.1 dependency |
| packages/core/src/index.ts | Exported new adapters module |
| packages/core/src/adapters/objectstack-adapter.ts | New ObjectStackAdapter implementation with CRUD and bulk operations |
| packages/core/src/adapters/index.ts | Adapter module exports |
| packages/core/src/adapters/README.md | Comprehensive documentation for ObjectStack adapter |
| packages/core/package.json | Added ObjectStack client and spec dependencies |
| package.json | Removed @objectstack/spec from pretest build filter |
| MIGRATION_GUIDE_OBJECTSTACK.md | Complete migration guide for ObjectStack integration |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| const ids = data.map(item => (item as any).id).filter(Boolean); | ||
| await this.client.data.deleteMany(resource, ids); |
Copilot
AI
Jan 21, 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 variable name ids is used but the subsequent line doesn't return results matching the delete operation. Consider renaming to deletedIds for clarity about what these IDs represent.
| const ids = data.map(item => (item as any).id).filter(Boolean); | |
| await this.client.data.deleteMany(resource, ids); | |
| const deletedIds = data.map(item => (item as any).id).filter(Boolean); | |
| await this.client.data.deleteMany(resource, deletedIds); |
| case 'delete': | ||
| const ids = data.map(item => (item as any).id).filter(Boolean); | ||
| await this.client.data.deleteMany(resource, ids); | ||
| return []; | ||
| case 'update': | ||
| // For update, we need to handle each record individually | ||
| // or use the batch update if all records get the same changes | ||
| const results = await Promise.all( | ||
| data.map(item => | ||
| this.client.data.update<T>(resource, String((item as any).id), item) | ||
| ) | ||
| ); | ||
| return results; |
Copilot
AI
Jan 21, 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.
If any item in the bulk update array is missing an id field, this will produce a confusing error. Consider adding validation to provide a clear error message like 'Bulk update requires all items to have an id field'.
| case 'delete': | |
| const ids = data.map(item => (item as any).id).filter(Boolean); | |
| await this.client.data.deleteMany(resource, ids); | |
| return []; | |
| case 'update': | |
| // For update, we need to handle each record individually | |
| // or use the batch update if all records get the same changes | |
| const results = await Promise.all( | |
| data.map(item => | |
| this.client.data.update<T>(resource, String((item as any).id), item) | |
| ) | |
| ); | |
| return results; | |
| case 'delete': { | |
| const hasMissingId = data.some(item => (item as any).id == null); | |
| if (hasMissingId) { | |
| throw new Error('Bulk update requires all items to have an id field'); | |
| } | |
| const ids = data.map(item => (item as any).id); | |
| await this.client.data.deleteMany(resource, ids); | |
| return []; | |
| } | |
| case 'update': { | |
| const hasMissingId = data.some(item => (item as any).id == null); | |
| if (hasMissingId) { | |
| throw new Error('Bulk update requires all items to have an id field'); | |
| } | |
| // For update, we need to handle each record individually | |
| // or use the batch update if all records get the same changes | |
| const results = await Promise.all( | |
| data.map(item => | |
| this.client.data.update<T>(resource, String((item as any).id), item) | |
| ) | |
| ); | |
| return results; | |
| } |
| return record; | ||
| } catch (error) { | ||
| // If record not found, return null instead of throwing | ||
| if ((error as any)?.status === 404) { |
Copilot
AI
Jan 21, 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.
Using (error as any) bypasses type safety. Consider defining a proper error type interface or checking for specific error properties in a type-safe manner.
| total: result.count, | ||
| page: params?.$skip ? Math.floor(params.$skip / (params.$top || 20)) + 1 : 1, | ||
| pageSize: params?.$top, | ||
| hasMore: result.value.length === params?.$top, |
Copilot
AI
Jan 21, 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 hasMore calculation is incorrect when params.$top is undefined. If no limit is specified, result.value.length === undefined will always be false. This should check if a limit was provided before comparing: hasMore: params?.$top ? result.value.length === params.$top : false
| hasMore: result.value.length === params?.$top, | |
| hasMore: params?.$top != null ? result.value.length === params.$top : false, |
📦 Bundle Size Report
Size Limits
|
|
✅ All checks passed!
|
Co-authored-by: huangyiirene <7665279+huangyiirene@users.noreply.github.com>
📦 Bundle Size Report
Size Limits
|
|
✅ All checks passed!
|
Integrates latest ObjectStack packages to establish protocol conformance and enable native ObjectStack backend connectivity.
Dependencies
@object-ui/types→@objectstack/spec@^0.1.2(protocol foundation)@object-ui/core→@objectstack/client@^0.1.1(runtime SDK)@object-ui/data-objectql→@objectstack/client@^0.1.1(data adapter)ObjectStack Adapter
New
ObjectStackAdapterimplements ObjectUI'sDataSourceinterface with full protocol support:Features:
find,findOne,create,update,deletecreateMany,updateMany,deleteManyBuild Configuration
Updated
vite-plugin-dtsto skip diagnostics for workspace dependencies, preventing TypeScript from traversing source files across package boundaries during builds.Backward Compatibility
Zero breaking changes. ObjectStack integration is additive—existing code and data sources continue to work unchanged.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.