fix(plugin-dashboard): the editable grid renders dataset-bound widgets — and says so visibly when it cannot (#4614) - #4619
Merged
Conversation
…s — and says so visibly when it cannot (#4614) DashboardGridLayout had no dataset path at all: it never read `widget.dataset`, never imported DatasetWidget, and took no `dataSource` prop. A widget authored in the current ADR-0021 shape fell through to the static-data branch and rendered nothing — measured three ways: `{type:'chart',data:[]}` for a chart, `{type:'metric',value:'—'}` for a metric, `{type:'data-table',data:[]}` for a table. No data, no diagnostic, no path to fix. The cure is the sibling's own mechanics, not a second dispatch idiom: the `datasetBound` predicate decided per widget, DatasetWidget picked at the render site (DashboardRenderer.tsx:524 / :849-851), and a dataset-bound metric taking the shared Card wrapper (:777-782). A dataset-bound widget with NO dataSource renders DatasetWidget's own no-capability alert rather than a blank tile — measured visible before choosing it, so no third diagnostic surface is declared. That is the `dashboard-grid` SDUI path, which passes no adapter. The #4612 legacy sentinel keeps its position and verdict: the two conditions are mutually exclusive by construction (legacyRetiredWidget.ts:107). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…id SDUI registration (#4614) The component-level suite renders DashboardGridLayout directly. This one renders it the way a schema-driven host does — as the registered `dashboard-grid` component type resolved by SchemaRenderer — because that registration is the surface #4614 was filed against. Two directions, neither visible from the component-level suite: - no `dataSource` passed (what the registration actually declares: `title` and `className` only) still yields the visible dataset diagnostic inside a rendered grid, so adding an optional prop did not break schema-driven usage; - a `dataSource` handed to SchemaRenderer reaches DatasetWidget, which works only because SchemaRenderer forwards unread props to the resolved component (`...props`, spread last) and strips the spec's same-named per-element binding before that spread (objectstack#5576). Nothing else pinned that interaction for this component. Reverse-verified: with the fix reverted, the two positives go red and the static-data control stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4614
DashboardGridLayouthad no dataset path at all — it never readwidget.dataset, never importedDatasetWidget, and took nodataSourceprop — so a widget authored in the CURRENT ADR-0021 shape fell through to the static-data branch and rendered nothing.The defect, measured three ways (not one)
The issue reported a blank chart. Spying on the node the grid handed
SchemaRendereronorigin/main(8640cec) showed the silence has one cause and three faces, one per dispatch family:{ type: 'bar', dataset, values }{ type: 'chart', chartType: 'bar', data: [], … }{ type: 'metric', dataset, values }{ type: 'metric', label: 'metric', value: '—' }{ type: 'table', dataset, … }{ type: 'data-table', data: [], … }The chart node verbatim, pre-fix:
{ "type": "chart", "chartType": "bar", "data": [], "xAxisKey": "name", "series": [ { "dataKey": "value" } ], "colors": [ "hsl(var(--chart-1))", "…", "…", "…", "hsl(var(--chart-5))" ], "isAnimationActive": false, "className": "h-full" }All three are #4612's defect one level up: that card was the RETIRED authoring shape falling through this same surface, this one is the shape that is current.
Mirror design — the sibling's mechanics, with lines cited
DashboardRenderer.tsx:524const datasetBound = !!(widget as any).dataset;:849-851{datasetBound ? < DatasetWidget … / > : < SchemaRenderer … / >}:777-782isSelfContained = widget.type === 'metric' && !datasetBound:198dataSource?: anydataSourceprop (typedunknown— see below)Three deliberate departures, each stated rather than silently copied:
widgetis passed whole, not wrapped aseffectiveWidget. The sibling buildseffectiveWidget(:774-776) only to merge the dashboard FILTER BAR'sscopedFilter. This surface has no filter bar and noscopedFilter, so re-creating the wrapper would state a dependency that does not exist.DatasetWidgetreadswidget.filteritself and forwards it asruntimeFilter, so an authored per-widget filter still applies.:820-822) which is unreachable there, becauseisSelfContainedalready requires!datasetBound. The reachable mechanics are mirrored; the stranded limb is not. Filed as an observation-class finding: finding(plugin-dashboard): DashboardRenderer's self-contained branch carries a DatasetWidget fork that cannot be reached #4620.unknowninstead ofany, in both places. The sibling'sdataSource?: anyis explicitly historical ("that is precisely what it resolved to before", via an index signature that used to answer for it). This is a NEW declaration with no prior resolution to preserve, andunknownis what the consumer itself declares (DatasetWidget.tsx:655,dataSource: unknown) — the value is forwarded to exactly the type that receives it, and no caller is held to anything new since every value is assignable tounknown. Likewise(widget as { dataset?: unknown }).datasetrather thanas any, following this package's own precedent for keys the bundled spec type lacks:legacyRetiredWidget.ts:64-80names them in a shape of its own, which is "what keepsas anyout of both call sites". This also keeps eslint net-zero (below).Case 2 — measured first, then chosen
The ruling asked whether
DatasetWidget's own no-capability rendering is visible here before inventing a placeholder. Measured withdataSource={undefined}:DatasetWidget.tsx:766-771sets the error state BEFORE the measures check, and:929-934renders it as arole="alert"box. Visible in this context ⇒ routing throughDatasetWidgetunconditionally is the fix for both halves, and no new placeholder is declared. One condition, one wording, no third diagnostic surface. This is not a hypothetical branch:dashboard-grid's SDUI registration declares onlytitle/classNameinputs, so schema-driven hosts render this component with no adapter at all — that path IS case 2, and it is pinned twice (component level, and through the registry).Red-first, predictions written before the run
Predictions were recorded before any run. Pre-fix,
origin/mainsource, new suite: 8 positives red, 5 negatives green (13 total) — verbatim:Post-fix: 13 passed (13).
One prediction was wrong, and fixing it is the most load-bearing change in the suite. I predicted a test asserting
findByRole('alert')would be red pre-fix. It passed pre-fix — becausechartwas not registered in the test environment, soSchemaRendererrendered its own red "Unknown component type" box, which also carriesrole="alert". A control passing for the wrong reason. Two consequences, both applied:@object-ui/componentsAND@object-ui/plugin-charts(both declared deps; the latter is wherechartis registered, and it is a devDependency) at module scope. With the registries populated, the pre-fix tile for a dataset-bound widget is exactly what the issue claims — empty: no alert, no chart, no text, just an injectedstyletag.message.closest('[role="alert"]')), soSchemaRenderer's own error box cannot satisfy it.The SDUI registration, pinned separately
DashboardGridLayout.sduiDatasetPath.test.tsxrenders the component the way a schema-driven host does — as the registereddashboard-gridtype resolved bySchemaRenderer— because that registration is the surface the card was filed against, and nothing else in the repo pinned it for this component. Both directions:dataSource(what the registration actually declares) still yields the visible diagnostic inside a rendered grid, so schema-driven usage is intact rather than merely failing loudly;dataSourcehanded toSchemaRendererreachesDatasetWidget— which works only becauseSchemaRendererforwards unread props to the resolved component (...props, spread last,:632) and strips the spec's same-named per-element BINDING before that spread (:564-575, objectstack#5576).Reverse-verified on its own: with the fix reverted, its two positives go red and its static-data control stays green.
Reverse verification
Fix removed with
git checkout origin/main -- DashboardGridLayout.tsx(nevergit stash), everything else identical: new suite 8 failed | 5 passed, andDashboardGridLayout.legacyRetired.test.tsx9/9 still green — the #4613 sentinel is independent of this change in both directions.Must-not-change proofs
getComponentSchemaBEFORE the dispatch branches. The dataset check sits at the RENDER SITE instead, and the two conditions are mutually exclusive by construction:isLegacyRetiredWidgetreturns false the moment a widget carriesdataset(legacyRetiredWidget.ts:107,if (w.dataset) return false). Neither can capture the other's widget, so their relative order cannot change a verdict — which is exactly why that module states step 1 on its own terms rather than inheriting it from a caller's fork (legacyRetiredWidget.ts:97-102). Pinned both ways: a legacy widget still shows the placeholder and runs no dataset query, even when a workingdataSourceIS supplied.dashboard-gridSDUI registration with no dataSource — that is positive case (b), pinned across chart / metric / table / pivot at component level and again through the registry.index.tsx:271-289is unchanged.legacyRetiredWidget.ts— untouched, not even a comment.DashboardRenderer's suites — untouched and green (whole package: 55 files / 440 tests).SchemaRendererprop channel — the new prop cannot be shadowed by the spec's per-elementdataSourceBINDING, which is stripped before the props spread (SchemaRenderer.tsx:564-575); an explicit ReactdataSourcearrives via...props, spread last (:632).Verification
pnpm --filter '@object-ui/plugin-dashboard^...' --filter '@object-ui/plugin-dashboard' build— green (run before judging anything).pnpm exec vitest run --maxWorkers=2 packages/plugin-dashboard— 55 files / 440 tests passed.app-shell/widget-dom-leak-sweep,components/inline-locale-label-read-sites,console/preview-samples-registry-resolvable— 3 files / 49 tests passed.turbo run type-check --filter='...@object-ui/plugin-dashboard' --force— 41/41 tasks, uncached. Scope:app-shell,console,example-byo-backend-console,example-console-starter,plugin-dashboard,site..d.tsboth ways,dist+tsbuildinfocleared and rebuilt on each side. The entire diff:export interface DashboardGridLayoutProps { schema: DashboardComponentSchema; className?: string; + dataSource?: unknown; onLayoutChange?: (layout: RGLLayout[]) => void;One added OPTIONAL property ⇒ additive ⇒ minor by position analysis, plus the behavior move. Never major. One changeset.
origin/maincompare worktree: baseerrors=0 warnings=327→ brancherrors=0 warnings=327(both new test files contribute zero).check-control-bytes(4395 files),check-phantom-dependencies,check-changeset-presence,check-changeset-no-major,check-changeset-fixed,check-type-check-coverage(41/41),check-lint-coverage(46/46),check-doc-links. Plus a control-byte self-scan over the touched files including untracked ones — clean.Out-of-scope findings
finding) — the sibling's unreachableDatasetWidgetfork described above.tablewidget with inline static data crashes into the error boundary — "Maximum update depth exceeded" #4618 — already open, filed by another seat: a static-datatablewidget crashes into the error boundary with "Maximum update depth exceeded" (data-table.tsx:788). Observed in this card's pre-fix stderr and left alone; this change does not touch that path (post-fix, dataset-bound tables no longer build adata: []data-table).Surface respected
Touches
packages/plugin-dashboard/src/**+ two tests + one changeset, nothing else.examples/schema-catalog/**,content/docs/**, the plugin README andapps/site/**belong to the in-flight #4600-d1 seat and are untouched;content/docs/releases/**never.Generated by Claude Code