-
Notifications
You must be signed in to change notification settings - Fork 0
Add ObjectQL 1.9.1 integration with ObjectTable and ObjectForm components #87
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
…ble and ObjectForm components Co-authored-by: hotlong <[email protected]>
Co-authored-by: hotlong <[email protected]>
Co-authored-by: hotlong <[email protected]>
Co-authored-by: hotlong <[email protected]>
|
为相关组件添加showcase |
|
Co-authored-by: hotlong <[email protected]>
Added comprehensive showcase pages for ObjectTable and ObjectForm components in commit 0f3fd52. What was added:
Each showcase page includes:
You can view the showcases by running: pnpm dev:showcaseThen navigate to the "ObjectQL" section in the sidebar menu. |
|
@copilot 两个软件包的区别是什么?data-object, data-objectql |
两个软件包的区别:
关系: 简单说:
|
|
我们其他的组件包名plugin开头,重新评估一下这个软件包的名称以后,所有对象相关的组件在这里开发合适吗?@copliot |
包名评估和建议: 查看了现有的 plugin 包(plugin-charts、plugin-kanban、plugin-editor、plugin-markdown),我认为应该重命名: 建议改名:
理由:
不需要改名:
架构清晰度: 是否需要我重命名
|
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 a new plugin package (@object-ui/plugin-object) that provides ObjectQL-specific UI components for automatic table and form generation based on ObjectQL object schemas. The plugin follows Object UI's schema-first architecture and integrates seamlessly with the ObjectQL backend through the MetadataApiClient.
Changes:
- Added
@object-ui/plugin-objectpackage withObjectTableandObjectFormcomponents that auto-generate UI from ObjectQL metadata - Enhanced
ObjectQLDataSourcewith schema fetching capability via MetadataApiClient - Updated ObjectQL dependencies to version 1.9.1 across affected packages
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/objectql.ts | New type definitions for ObjectQL components (ObjectTableSchema, ObjectFormSchema) |
| packages/types/src/index.ts | Exports new ObjectQL types and adds them to AnySchema union |
| packages/plugin-object/* | New plugin package with ObjectTable and ObjectForm components, configuration files, and documentation |
| packages/data-objectql/src/ObjectQLDataSource.ts | Added MetadataApiClient and getObjectSchema() method |
| packages/data-objectql/package.json | Updated ObjectQL SDK dependencies to 1.9.1 |
| examples/showcase/pages/objectql/* | Showcase pages demonstrating ObjectTable and ObjectForm usage |
| examples/showcase/app.json | Added ObjectQL section to showcase navigation menu |
| CHANGELOG.md | Documented changes in unreleased section |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
packages/plugin-object/README.md
Outdated
| ### ObjectTable | ||
|
|
||
| ```tsx | ||
| import { ObjectTable } from '@object-ui/data-object'; |
Copilot
AI
Jan 17, 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.
Incorrect import path. The package name should be '@object-ui/plugin-object', not '@object-ui/data-object'.
packages/plugin-object/README.md
Outdated
| ### ObjectForm | ||
|
|
||
| ```tsx | ||
| import { ObjectForm } from '@object-ui/data-object'; |
Copilot
AI
Jan 17, 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.
Incorrect import path. The package name should be '@object-ui/plugin-object', not '@object-ui/data-object'.
| }; | ||
|
|
||
| /** | ||
| * Map ObjectQL field type to form field type |
Copilot
AI
Jan 17, 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.
This helper function lacks JSDoc documentation. Add documentation explaining the mapping logic and its parameters.
| * Map ObjectQL field type to form field type | |
| * Maps an ObjectQL field type to the corresponding form field component type. | |
| * | |
| * This helper provides the translation layer between backend/ObjectQL field | |
| * definitions (e.g. `text`, `date`, `lookup`) and the generic form field | |
| * types understood by the schema renderer (e.g. `input`, `date-picker`, | |
| * `select`). If a field type is not explicitly mapped, the function falls | |
| * back to the generic `"input"` type. | |
| * | |
| * @param fieldType - The ObjectQL field type identifier to convert | |
| * (for example: `"text"`, `"number"`, `"date"`, `"lookup"`). | |
| * @returns The normalized form field type string used in the form schema | |
| * (for example: `"input"`, `"textarea"`, `"date-picker"`, `"select"`). |
packages/types/src/objectql.ts
Outdated
| * Form layout | ||
| * @default 'vertical' | ||
| */ | ||
| layout?: 'vertical' | 'horizontal' | 'inline' | 'grid'; | ||
|
|
||
| /** | ||
| * Grid columns (for grid layout) |
Copilot
AI
Jan 17, 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 documentation refers to 'grid layout', but in ObjectForm.tsx lines 217-219, 'grid' layout is mapped to 'vertical' because FormSchema doesn't support 'grid'. This inconsistency should be documented or the type definition should clarify supported layouts.
| * Form layout | |
| * @default 'vertical' | |
| */ | |
| layout?: 'vertical' | 'horizontal' | 'inline' | 'grid'; | |
| /** | |
| * Grid columns (for grid layout) | |
| * Form layout. | |
| * | |
| * Supported layouts: | |
| * - `vertical` – label above field (default) | |
| * - `horizontal` – label and field in a row | |
| * - `inline` – compact inline layout, typically used in toolbars | |
| * - `grid` – **experimental** grid layout | |
| * | |
| * Note: As of the current implementation, the underlying form renderer does not yet | |
| * support a native `grid` layout and will internally treat `layout: "grid"` as | |
| * `layout: "vertical"`. This value is exposed in the schema for forward compatibility, | |
| * and behavior may change once grid support is implemented. | |
| * | |
| * @default 'vertical' | |
| */ | |
| layout?: 'vertical' | 'horizontal' | 'inline' | 'grid'; | |
| /** | |
| * Grid columns (for grid layout). | |
| * | |
| * Intended number of columns when using a `grid` layout. Current renderers that do | |
| * not implement true grid support may ignore this value and fall back to a vertical | |
| * layout. When grid layout is supported, this value should control how many form | |
| * fields are placed per row. | |
| * |
📦 Bundle Size Report
Size Limits
|
|
@copilot fix all and test |
…t documentation Co-authored-by: huangyiirene <[email protected]>
Fixed all code review issues in commit ce1079b:
All changes have been built and tested successfully. The package builds without errors. |
📦 Bundle Size Report
Size Limits
|
Added missing color default value (#8884d8) to: - defaultProps object (required by test) - input definition defaultValue (for consistency) This fixes the failing test in packages/plugin-charts/src/index.test.ts that expected color to be '#8884d8' in defaultProps. Co-authored-by: huangyiirene <[email protected]>
📦 Bundle Size Report
Size Limits
|
|
✅ All checks passed!
|
Co-authored-by: huangyiirene <[email protected]>
Co-authored-by: hotlong <[email protected]>
…ue-another-one
📦 Bundle Size Report
Size Limits
|
| // Resolve the actual project root and schema file | ||
| let projectRoot = cwd; | ||
| let targetSchemaPath = schemaPath; | ||
| let _projectRoot = cwd; |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, to fix a “useless assignment to local variable” you either (a) remove the redundant initial assignment if the variable is never read before being overwritten, or (b) move the defaulting logic to the actual decision point so that the value is only set when needed. This preserves behavior while eliminating dead writes.
Here, _projectRoot is intended to represent the project root. The code already uses cwd as the base when computing absoluteSchemaPath and looking for a pages directory in cwd. The initial let _projectRoot = cwd; is not used before the discovery logic that may reset _projectRoot to fileDir. The simplest behavior-preserving fix is to declare _projectRoot without an initial value, and then explicitly ensure it has a value before any later use. Given the visible snippet, the logical default is cwd when no more specific project root is detected.
To do this safely while only changing shown code:
- Change the declaration on line 20 from
let _projectRoot = cwd;tolet _projectRoot: string;. - After the existing fallback pages-dir detection block (lines 51–59), set
_projectRootbased on the detection result. If a project structure was found atfileDir,_projectRootwas already set in line 36; otherwise, we should fall back tocwd. The clean way is to check if_projectRootis still undefined and set it tocwd. This preserves the previous effective behavior (root isfileDirif detected, otherwisecwd) while eliminating the unused initial assignment. - No new imports or external methods are required; we only adjust variable initialization logic inside
dev.
-
Copy modified line R20 -
Copy modified lines R61-R65
| @@ -17,7 +17,7 @@ | ||
| const cwd = process.cwd(); | ||
|
|
||
| // Resolve the actual project root and schema file | ||
| let _projectRoot = cwd; | ||
| let _projectRoot: string; | ||
| const targetSchemaPath = schemaPath; | ||
| let hasPagesDir = false; | ||
| let pagesDir = ''; | ||
| @@ -58,6 +58,11 @@ | ||
| } | ||
| } | ||
|
|
||
| // If no specific project root was detected, default to the current working directory | ||
| if (!_projectRoot) { | ||
| _projectRoot = cwd; | ||
| } | ||
|
|
||
| const require = createRequire(join(cwd, 'package.json')); | ||
|
|
||
| let routes: RouteInfo[] = []; |
| if (existsSync(potentialPagesDir)) { | ||
| console.log(chalk.blue(`📂 Detected project structure at ${fileDir}`)); | ||
| projectRoot = fileDir; | ||
| _projectRoot = fileDir; |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, a "useless assignment to local variable" is fixed either by removing the assignment (if the value is truly not needed) or by adding the missing logic that consumes the assigned value (if the assignment points to an incomplete implementation). Here, we must not infer or add new usage for _projectRoot in code we cannot see, so the least invasive and safest fix is to remove the redundant assignment that CodeQL flagged.
Concretely, in packages/cli/src/commands/dev.ts, inside the if (existsSync(absoluteSchemaPath) && statSync(absoluteSchemaPath).isFile()) branch where a pages directory is detected, we currently set _projectRoot = fileDir; on line 36. Since CodeQL reports that this value is never read, we will delete this line only. We will leave the declaration let _projectRoot = cwd; intact in case it is used later in the file; that way, any later logic that depends on _projectRoot continues to see the original cwd value and behavior does not change. No new imports, methods, or definitions are required to implement this change.
| @@ -33,7 +33,6 @@ | ||
|
|
||
| if (existsSync(potentialPagesDir)) { | ||
| console.log(chalk.blue(`📂 Detected project structure at ${fileDir}`)); | ||
| _projectRoot = fileDir; | ||
| hasPagesDir = true; | ||
| pagesDir = potentialPagesDir; | ||
|
|
| let projectRoot = cwd; | ||
| let targetSchemaPath = schemaPath; | ||
| let _projectRoot = cwd; | ||
| const targetSchemaPath = schemaPath; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, an unused local variable should be removed to avoid confusion and unnecessary computation. If the value it would hold is actually needed, then code should be updated to use that variable instead; otherwise, the declaration should be deleted.
Here, targetSchemaPath is simply assigned schemaPath and never read. The code already uses schemaPath and absoluteSchemaPath where needed, so removing the targetSchemaPath declaration is the simplest, safest change that does not alter behavior. No other lines depend on it.
Concretely, in packages/cli/src/commands/dev.ts, inside the dev function, delete the line:
const targetSchemaPath = schemaPath;No imports or other definitions are needed to implement this change.
| @@ -18,7 +18,6 @@ | ||
|
|
||
| // Resolve the actual project root and schema file | ||
| let _projectRoot = cwd; | ||
| const targetSchemaPath = schemaPath; | ||
| let hasPagesDir = false; | ||
| let pagesDir = ''; | ||
| let appConfig: unknown = null; |
|
✅ All checks passed!
|
Task: Update ObjectQL and Create Object Components Package
✅ All Tasks Completed Successfully!
Updates Made:
@object-ui/data-objectto@object-ui/plugin-objectmapFieldTypeToFormTypehelper functionCode Review Fixes:
gridlayout is experimental and currently falls back toverticallayoutPackage Naming:
Following the established pattern where UI component packages use the
plugin-prefix:@object-ui/plugin-object- ObjectQL UI components plugin (ObjectTable, ObjectForm)@object-ui/data-objectql- ObjectQL data adapter (unchanged)Architecture:
All issues identified in code review have been addressed and the package builds successfully.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.