-
Notifications
You must be signed in to change notification settings - Fork 0
Fix console form infinite loading, duplicate data seeding, and non-grid view rendering #397
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.
|
… issues - ObjectForm: handle null schema from getObjectSchema by throwing error - ObjectForm: handle missing objectName/dataSource by clearing loading state - browser.ts: remove duplicate data seeding (AppPlugin already seeds during bootstrap) - ObjectView: fix layout to use flex column with proper height chain for non-grid views Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…grid views Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot fix ci build and test |
Remove unused imports (beforeAll, afterAll, within, fireEvent) that caused TS6133 errors during the console app build. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in f23c7c5. The build failure was caused by unused imports ( The 3 test failures ( |
📦 Bundle Size Report
Size Limits
|
|
@copilot fix all ci test errors |
…ui/plugin-report Add two missing module resolution aliases in vitest.config.mts: - @object-ui/types/zod → packages/types/src/zod/index.zod.ts - @object-ui/plugin-report → packages/plugin-report/src This fixes 3 test file failures: - packages/plugin-grid/src/ListColumnSchema.test.ts (could not resolve @object-ui/types/zod) - apps/console/src/__tests__/BrowserSimulation.test.tsx (could not resolve @object-ui/plugin-report) - apps/console/src/__tests__/ConsoleApp.test.tsx (could not resolve @object-ui/plugin-report) All 117 test files now pass (2472 tests). Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in 6af4860. The 3 test failures were caused by missing vitest module resolution aliases in
All 117 test files now pass (2472 tests, 0 failures). |
📦 Bundle Size Report
Size Limits
|
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
Fixes several console/runtime regressions around form schema fetching, MSW mock initialization, and non-grid view layout sizing, plus adds targeted regression coverage and test/build config updates.
Changes:
- Update
ObjectFormto handle falsy schema fetch results and clear loading on schema-fetch errors. - Prevent duplicate manifest data seeding in the console MSW browser mock startup.
- Fix
ObjectViewheight/scroll chain for non-grid view types and add Vitest aliases + regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.config.mts |
Adds missing module aliases needed for tests to resolve subpath/package imports. |
packages/plugin-view/src/ObjectView.tsx |
Enforces a flex column + height chain and wraps content to allow non-grid views to render/scroll correctly. |
packages/plugin-form/src/ObjectForm.tsx |
Improves schema-fetch failure handling to avoid infinite loading and adds an early loading-clear fallback. |
apps/console/src/mocks/browser.ts |
Removes redundant manifest seeding to avoid duplicating records in browser MSW mode. |
apps/console/src/__tests__/ConsoleIssues.test.tsx |
Adds regression tests covering form loading, initial data fetch, and non-grid view rendering paths. |
| it('should not stay in loading when dataSource is missing', async () => { | ||
| render( | ||
| <ObjectForm | ||
| schema={{ | ||
| type: 'object-form', | ||
| objectName: 'contact', | ||
| mode: 'create', | ||
| fields: ['name'], | ||
| }} | ||
| />, | ||
| ); | ||
|
|
||
| // Should not show loading forever — either shows empty form or no-loading state | ||
| await waitFor( | ||
| () => { | ||
| expect(screen.queryByText(/Loading form/i)).not.toBeInTheDocument(); | ||
| }, | ||
| { timeout: 5000 }, | ||
| ); | ||
| }); |
Copilot
AI
Feb 8, 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 test currently only asserts that the loading indicator disappears when dataSource is missing. Given the form can’t function without a DataSource (and the component has a specific error UI), it would be stronger and less prone to false positives to assert the error state/message is rendered (e.g., “Error loading form” / “DataSource is required …”) rather than just “not loading”.
| function createMockDataSource(overrides?: Partial<DataSource>): DataSource { | ||
| return { | ||
| async getObjectSchema(_objectName: string) { | ||
| return { | ||
| name: 'contact', |
Copilot
AI
Feb 8, 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.
createMockDataSource() ends with } as DataSource, which disables compile-time checking of the mock’s method signatures (e.g. delete should take (resource, id) per DataSource). Consider using satisfies DataSource (or const base: DataSource = { ... }) so signature mismatches are caught by TypeScript, and only use a targeted cast in the one test that intentionally returns null from getObjectSchema.
| } else if (schema.objectName && dataSource) { | ||
| fetchObjectSchema(); | ||
| } else if (!hasInlineFields) { | ||
| // No objectName or dataSource and no inline fields — cannot proceed |
Copilot
AI
Feb 8, 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.
In the schema-fetching effect, the fetchObjectSchema() function contains a !dataSource error path, but it’s never reached because fetchObjectSchema() is only called when schema.objectName && dataSource. When objectName is set and inline fields are not provided, but dataSource is missing, the code currently just setLoading(false) and renders an empty form (no error). Consider setting an explicit error in this branch (or calling fetchObjectSchema() whenever schema.objectName is present so the existing error handling runs) so the UI shows a clear failure state instead of a blank form.
| } else if (schema.objectName && dataSource) { | |
| fetchObjectSchema(); | |
| } else if (!hasInlineFields) { | |
| // No objectName or dataSource and no inline fields — cannot proceed | |
| } else if (schema.objectName) { | |
| // Fetch schema using objectName; fetchObjectSchema will handle missing dataSource | |
| fetchObjectSchema(); | |
| } else if (!hasInlineFields) { | |
| // No objectName and no inline fields — cannot proceed | |
| setError(new Error('Object name is required when inline fields are not provided')); |
plugin-form/src/ObjectForm.tsxConsoleIssues.test.tsx(TS6133)@object-ui/types/zodand@object-ui/plugin-report💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.