fix: align DashboardWidgetSchema with @objectstack/spec, fix missing React keys#522
fix: align DashboardWidgetSchema with @objectstack/spec, fix missing React keys#522
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ng React keys - Update DashboardWidgetSchema interface and Zod schema to support both legacy component format and spec shorthand format (type/options) - Fix missing React keys for metric widgets without id or title - Remove unsafe (widget as any) casts in DashboardRenderer and DashboardGridLayout - Add tests for spec shorthand metric widgets and unique key generation Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Aligns ObjectUI’s dashboard widget typing/validation with the @objectstack/spec shorthand widget format and improves React reconciliation by generating non-undefined widget keys.
Changes:
- Updated
DashboardWidgetSchema(TypeScript + Zod) to support both legacy{ component }widgets and spec shorthand{ type, options }. - Updated dashboard renderers to use typed
widget.type/widget.optionsand added an index-based key fallback. - Added renderer tests for shorthand metric widgets and (intended) key uniqueness coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/zod/complex.zod.ts | Makes widget Zod fields optional and adds type/options for spec shorthand. |
| packages/types/src/complex.ts | Makes widget interface fields optional and adds type/options for spec shorthand. |
| packages/plugin-dashboard/src/DashboardRenderer.tsx | Removes as any for type/options, adds index-based key fallback, updates widget mapping. |
| packages/plugin-dashboard/src/DashboardGridLayout.tsx | Removes as any for type/options, aligns shorthand mapping for chart/table. |
| packages/plugin-dashboard/src/tests/DashboardRenderer.widgetData.test.tsx | Adds tests for shorthand metric rendering and intended key uniqueness verification. |
| it('should assign unique keys to widgets without id or title', () => { | ||
| const schema = { | ||
| type: 'dashboard' as const, | ||
| name: 'test', | ||
| title: 'Test', | ||
| widgets: [ | ||
| { | ||
| type: 'metric', | ||
| layout: { x: 0, y: 0, w: 1, h: 1 }, | ||
| options: { label: 'Metric A', value: '100' }, | ||
| }, | ||
| { | ||
| type: 'metric', | ||
| layout: { x: 1, y: 0, w: 1, h: 1 }, | ||
| options: { label: 'Metric B', value: '200' }, | ||
| }, | ||
| { | ||
| type: 'metric', | ||
| layout: { x: 2, y: 0, w: 1, h: 1 }, | ||
| options: { label: 'Metric C', value: '300' }, | ||
| }, | ||
| ], | ||
| } as any; | ||
|
|
||
| const { container } = render(<DashboardRenderer schema={schema} />); | ||
|
|
||
| // All three metrics should render without React key warnings | ||
| expect(container.textContent).toContain('Metric A'); | ||
| expect(container.textContent).toContain('Metric B'); | ||
| expect(container.textContent).toContain('Metric C'); | ||
| expect(container.textContent).toContain('100'); | ||
| expect(container.textContent).toContain('200'); | ||
| expect(container.textContent).toContain('300'); | ||
| }); |
There was a problem hiding this comment.
This test claims to verify unique React keys, but it never asserts that React emitted no key warnings (e.g., via a console.error/console.warn spy). Consider adding an explicit assertion that the "Each child in a list should have a unique "key" prop" warning is not logged, otherwise this test can pass even if keys are still duplicated.
| export const DashboardWidgetSchema = z.object({ | ||
| id: z.string().describe('Widget ID'), | ||
| id: z.string().optional().describe('Widget ID'), | ||
| title: z.string().optional().describe('Widget Title'), | ||
| component: SchemaNodeSchema.describe('Widget Component'), | ||
| component: SchemaNodeSchema.optional().describe('Widget Component (legacy format)'), | ||
| layout: DashboardWidgetLayoutSchema.optional().describe('Widget Layout'), | ||
| type: z.string().optional().describe('Widget visualization type (spec shorthand)'), | ||
| options: z.unknown().optional().describe('Widget specific configuration (spec shorthand)'), | ||
| }); |
There was a problem hiding this comment.
DashboardWidgetSchema is now fully optional (id/component/type/options), so an empty widget object would pass validation. Consider modeling this as a union (legacy: requires component; shorthand: requires type) or add a Zod refinement to require at least one of component or type so invalid widgets are caught early.
| export interface DashboardWidgetSchema { | ||
| id: string; | ||
| id?: string; | ||
| title?: string; | ||
| component: SchemaNode; | ||
| /** Component schema (legacy format) */ | ||
| component?: SchemaNode; | ||
| layout?: DashboardWidgetLayout; | ||
| /** Widget visualization type (spec shorthand format) */ | ||
| type?: string; | ||
| /** Widget-specific configuration (spec shorthand format) */ | ||
| options?: unknown; |
There was a problem hiding this comment.
The updated DashboardWidgetSchema interface makes both component and type optional, which allows invalid widget shapes at compile time and forces downstream consumers to handle impossible states. Consider expressing this as a discriminated/union type (legacy component format vs spec shorthand) so TypeScript enforces that at least one rendering strategy is present.
| const isSelfContained = widget.type === 'metric'; | ||
| const widgetKey = widget.id || widget.title || `widget-${index}`; | ||
|
|
||
| if (isSelfContained) { | ||
| return ( | ||
| <div | ||
| key={widget.id || widget.title} | ||
| key={widgetKey} |
There was a problem hiding this comment.
widgetKey can still be non-unique when multiple widgets share the same title and have no id (keys will collide on title). To guarantee unique sibling keys, consider incorporating index into the title fallback (or another deterministic unique suffix) whenever id is missing.
@object-ui/typesDashboardWidgetSchemarequiredidandcomponentfields, but@objectstack/specv3.0.4 defines widgets usingtype/optionsshorthand. This type mismatch forced unsafeas anycasts throughout the renderer and left metric widgets without proper React keys (key={undefined}for all 4 cards), causing reconciliation issues.Types (
packages/types)idandcomponentoptional onDashboardWidgetSchematype?: stringandoptions?: unknownto match the spec shorthand formatRenderer (
packages/plugin-dashboard)(widget as any).type/(widget as any).optionscasts — types now cover both formatswidget.id || widget.title || \widget-${index}`prevents duplicateundefined` keys on metric widgetsDashboardGridLayout.tsxTests
id/titleOriginal 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.