fix(plugin-dashboard): GridLayout shows the retired-format placeholder instead of a silent blank chart (#4612) - #4613
Merged
Conversation
…r instead of a silent blank chart (#4612) framework#3320 retired the pre-ADR-0021 inline-analytics widget shape and gave DashboardRenderer a graceful fallback: a visible "This widget uses a retired data format. Edit it to bind a dataset." tile for the stored metadata that still carries it. DashboardGridLayout — separately exported, registered as the `dashboard-grid` SDUI component — had no such sentinel, so the identical widget fell through to its static-data branch with `data: []`: a silent blank chart with no diagnostic and no path to fix, the exact outcome the retirement's own test header says must not happen. The detector and the placeholder now live in one module (`legacyRetiredWidget.ts`) consumed by both surfaces, because one private copy is why the defect existed. DashboardRenderer's observable behaviour is unchanged. The nested `options.data = { provider: 'object', … }` config, dataset widgets and static-data widgets are untouched on both surfaces, pinned by four negative controls. Fixes #4612 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
|
yinlianghui
marked this pull request as ready for review
August 14, 2026 01:05
This was referenced Aug 14, 2026
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 #4612
The defect
framework#3320 retired the pre-ADR-0021 inline-analytics widget shape (top-level
object+categoryField/valueField/aggregate, pivotrowField/columnField) and shipped a graceful fallback for the stored metadata that still carries it — a visible tile reading "This widget uses a retired data format. Edit it to bind a dataset."That fallback was applied to
DashboardRendererand to nothing else.DashboardGridLayouthad no sentinel at all, so the identical stored widget fell through to its static-data branch withdata: []. Same metadata, same product, two outcomes: a rebind prompt on one surface, a silent blank chart on the other — worse for the author than the pre-retirement state, because it carries no chart, no diagnostic and no path to fix. That is verbatim the outcomeDashboardRenderer.legacyRetired.test.tsx's own header says must not happen.One correction to the issue's blast radius. The card says
DashboardGridLayoutis "used byDashboardWithConfig". Measured onorigin/main, it is not —DashboardWithConfig.tsx:210rendersDashboardRenderer, and repo-wide there is no in-repo importer ofDashboardGridLayoutoutside its own package. The real exposure is the two published surfaces: the named export from the package entry, and thedashboard-gridSDUI component type registered as "Dashboard Grid (Editable)" (index.tsx:271-289), whichapp-shell's own widget sweep renders. The defect and its severity are unchanged; only the named consumer was wrong.The design — one detector, two consumers
New internal module
packages/plugin-dashboard/src/legacyRetiredWidget.tsholdsLEGACY_RETIRED_WIDGET_SCHEMA(moved verbatim out ofDashboardRenderer) andisLegacyRetiredWidget(). Both surfaces import it; neither restates it. The condition is not hand-copied, because one private copy is precisely why the defect existed — two surfaces and one fix.The predicate is true when all three hold, in this order: no
dataset; no renderer-internal data (widget.dataoroptions.data,||not??, preserving the original byte for byte); and a top-levelobject.DashboardRenderer's observable behaviour is unchanged — its existinglegacyRetiredsuite is untouched and green on both sides of the change.The one refinement, and why it is inert on the existing surface
The shared predicate states the
datasetguard explicitly, which the inline condition did not. OnDashboardRendererthat costs nothing observable:datasetBoundpicksDatasetWidgetat the render site regardless of whatgetComponentSchemareturned, so the placeholder was already unreachable for a dataset-bound widget. Verified with a throwaway probe run on both sides of the revert (a widget carrying bothdatasetand a stale top-levelobjectrenders no placeholder, before and after). Naming the condition keeps the predicate true on its own terms for any surface that has no such render fork — which is exactlyDashboardGridLayout.Why the pivot family arm was NOT shared
DashboardRenderer.tsx:712returns the placeholder for the entire pivot family, unconditionally. Mirroring that arm onto the grid would have been a faithful copy and a regression: it is a statement about whatDashboardRenderercan draw — that surface emits no pivot block at all — not about the widget being legacy.DashboardGridLayoutdoes draw pivots, from static data and from theprovider: 'object'config, and the ruling's must-not-change note is binding on both surfaces. So the arm stays surface-local (with a comment saying why), and the legacy pivot shape is caught on both surfaces by the shared sentinel via the top-levelobjectit carries — which is what the card's pivot positive case actually exercises.Red-first — predictions written before the run
Recorded before the pre-fix run, then measured. Positives assert the placeholder absent pre-fix, present post-fix; negatives assert absent on both sides.
{type:'bar', object:'invoices', categoryField:'month', valueField:'amount', aggregate:'sum'}{type:'chart', data: []}{id:'invoices_by_status', title:'Invoices by Status', type:'bar', object:'invoices', categoryField:'status', aggregate:'count'}— byte-for-byteexamples/schema-catalog/src/schemas/plugin-dashboard/filtered-dashboard.jsonwidgets[0]{type:'pivot', object:'invoices', rowField:'region', valueField:'amount'}{type:'pivot', data: []}{type:'metric', object:'invoices', aggregate:'count'}— same entry'swidgets[2]{type:'metric', value:'—'}{type:'bar', dataset:'invoices', values:['count']}{type:'bar', options:{data:{provider:'object', object:'invoices', aggregate:{…}}}}{type:'bar', options:{data:[{name:'A', value:1}]}}{type:'pivot', options:{data:[…]}}Pre-fix run, verbatim:
The failure output confirms the predicted branch was taken, not merely that text was missing — the rendered node dumped
{"type": "chart", "chartType": "bar", "data": [], …}: the silent blank chart itself.Post-fix, same command:
Test Files 2 passed (2) / Tests 15 passed (15).Reverse verification
git diffto a patch file,git checkout origin/main --on both consumers, re-run,git applyto restore,sha256sum -con all four touched files (4/4 OK). Nogit stashat any point — the stash stack is shared across worktrees.Reverted (both consumers at
origin/main, new module and new suite present):The 5 reds are exactly the 5 positives; the 4 negatives,
DashboardRenderer's 6, and the dataset-guard probe stay green. The predicted direction is the ordinary one — new pins go red when the fix is removed — and it held.Must-not-change, proved on both live surfaces
options.dataprovider config — N2, plusDashboardRenderer.legacyRetired.test.tsx's own control. The nestedobjectis read offwidgetData, never off the widget top level; the detector requires the absence of any widget-level data before it looks atobject, so the two cannot be conflated.DashboardRendereras a whole — itslegacyRetiredsuite untouched, 6/6 green before and after.Verification
packages/plugin-dashboard,app-shell/widget-dom-leak-sweepwhich rendersplugin-dashboard:dashboard-grid,components/inline-locale-label-read-sites): 55 files / 470 tests passed.turbo run type-check --filter='...@object-ui/plugin-dashboard'(leading dots = the package's consumers): 41 successful, 41 total..d.ts,distand tsbuildinfo cleared, rebuilt: byte-identical, sha256172d66f9…both sides. The shared module is internal-only, not re-exported from the package entry.origin/mainbaseline 328 problems (0 errors, 328 warnings), this branch 327 (0 errors, 327 warnings) — net −1, theas anythe extraction removed fromDashboardRenderer.check-control-bytes,check-phantom-dependencies,check-changeset-presence,check-changeset-no-major,check-changeset-fixed,check-type-check-coverage,check-lint-coverage,check-doc-links— 8/8 PASS. Control-byte self-scan over all touched files incl. untracked: clean.Grading
minor, one changeset. By measurement the published type surface is unchanged (byte-identical
.d.ts, internal-only module) — that half is patch. The grade is set by the behaviour move on a published component:DashboardGridLayout/dashboard-gridnow renders a placeholder where it rendered a blank, which is the behaviour-move precedent (#4495 / #4271 / #4479). Never major.Scope
packages/plugin-dashboard/src/**plus one changeset, exactly. Untouched, per the mutual exclusion with the #4600-(d1) seat:examples/schema-catalog/**,content/docs/**,packages/plugin-dashboard/README.md,apps/site/**.content/docs/releases/**never.Generated by Claude Code