Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-dashboard): PivotTable keeps "no rows" stable so the pivot memo holds - #5584
Merged
Merged
Conversation
… memo holds `PivotTable` spelled the empty array twice — as the destructuring default for `schema.data` and as the `Array.isArray` fallback that keeps a provider-config object out of iteration. Both feed the same cross-tabulation memo, so a schema with no `data` key, or one whose `data` is a provider config, handed the memo a fresh array identity every render and rebuilt the ordered key sets, the bucket map, the aggregated matrix and the row/column/grand totals over nothing. Both spellings now resolve to one module-scope `Object.freeze([])`, matching `data-table.tsx` (objectui#4618) and `ObjectPivotTable` (objectui#4629). This closes the direct-use path those two did not cover. The test asserts on the memo's recompute count and the identity of its `data` dependency, not on the rendered output — nothing renders wrong today, so a rendering assertion is green against the broken code. Each of the two literals gets its own case. Fixes#5562 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 21, 2026 15:34
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#5562
PivotTablespelled the empty array twice, and both spellings feed the same cross-tabulation memo:So a schema that declares no
datakey, or one whosedatais a provider-config object rather than rows, handed the memo a fresh array identity on every render. The memo is not trivial — two ordered key sets, abucket[row][col]number-array map, the aggregated matrix and the row/column/grand totals — and all of it was rebuilt on every render, over nothing. Thedata.length === 0early return sits after the memo (hooks must stay unconditional), so the wasted matrix is computed and then discarded.Both spellings now resolve to one module-scope
Object.freeze([]). Same fix, same reason asdata-table.tsx'sEMPTY_COLUMNS/EMPTY_ROWS(#4618) andObjectPivotTable's (#4629); this closes the direct-use path those two did not cover, whereDashboardRenderer/DashboardGridLayoutconstruct pivot schemas withoutObjectPivotTablein the chain.Severity is unchanged from the card: wasted work only. The churn feeds a memo rather than a
setState, andPivotTableholds no prop-to-state sync, so nothing rendered wrong and no render loop was possible.The test asserts on the memo, not on the render
Nothing renders wrong today, so a "the pivot renders correctly" assertion is green against the broken code and proves nothing.
PivotTable.stableEmptyRows.test.tsxwrapsuseMemoand counts how many times the cross-tabulation memo's factory actually runs, identifying that memo by its exact five-entry dependency list. It also asserts the identity of the memo'sdatadependency across renders, mirroringObjectPivotTable.stableEmptyRows.test.tsx.The two literals get one case each, because they are independent churn sources on independent lines and a single test cannot tell you which half is live.
Reverse-verification
Three mutations, each restored by a
trap ... EXIT INT TERM, each proved on disk with anchoredgrep -cin both directions (real text 1 to 0, mutant 0 to 1) plusgit diff --stat;git status --porcelainempty after every leg. No build step is involved: the root vitest config aliases every@object-ui/*specifier to that package'ssrc/, and the test imports../PivotTablerelatively, so the mutated source is what executes.data-key case:146reverted:176revertedThe two one-sided legs are the load-bearing half: they confirm the card's warning that fixing one literal leaves the other live, and that this test says which. The headline failure is
expected 4 to be 1— four renders, four distinct empty-array identities, four full recomputes.ESLint tracks the same thing independently. Reverting
:176alone reproduces the warning the issue quotes verbatim, and it is absent on the fixed file:The one remaining warning is the pre-existing
react-refresh/only-export-componentsonexport const PIVOT_AGGREGATIONS, shifted from line 109 to 133 by the new doc comment. Not introduced here.Gates
All run locally, exit codes captured before any pipe, each gate's own verdict line quoted.
pnpm --filter @object-ui/plugin-dashboard type-checktsc --noEmit && tsc -p tsconfig.test.json, both echoed)pnpm --filter @object-ui/plugin-dashboard lint365 problems (0 errors, 365 warnings)— every one pre-existingpnpm exec vitest run packages/plugin-dashboard/Test Files 73 passed (73)/Tests 657 passed (657)pnpm exec eslint .(repo-wide, 3486 files)node scripts/check-control-bytes.mjscheck-control-bytes: OK (scanned 4635 tracked text file(s); skipped 85 binary).node scripts/check-changeset-presence.mjs2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjsNo changeset declares a major bump.node scripts/check-changeset-fixed.mjsAll workspace packages are in the changeset fixed group.node scripts/check-lint-coverage.mjslint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).node scripts/check-type-check-coverage.mjs45/46 via type-check/41/41 packages compile their testsnode scripts/check-phantom-dependencies.mjsEvery in-scope import is declared by the package that publishes it.node scripts/check-package-self-import.mjsNo package names itself inside its own src/.Repo-wide lint was not narrowed — the full
eslint .farm ran green. The dependency closure had to be built first (pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-dashboard^...' build); without ittscresolves workspace packages throughdist/and reportsCannot find module '@object-ui/components'on files this PR never touches.One gate is declared not run:
node scripts/check-eager-closure-budget.mjsrefuses withoutapps/console/dist/eager-closure.json, which only a consolevite buildemits — its own verdict line calls that "a broken gauge, not a passing budget", so it reports nothing about this change either way. It cannot be implicated regardless: that gate measures which modules land in the eager closure, andgit diffshows this PR adds and removes zero import statements insrc/.The 6
@typescript-eslint/no-explicit-anywarnings on the new test areas anycasts on SDUI schema fixtures — the established pattern in this package's tests (the siblingObjectPivotTable.stableEmptyRows.test.tsxcarries 8 of the same,PivotTable.drill.test.tsx1).Gate union was run against the final commit,
34db0672.Scope
packages/plugin-dashboard/src/PivotTable.tsx, its new test, and one changeset.ObjectPivotTable.tsxandObjectDataTable.tsxwere not touched — both already fixed by #4629. No drive-by tidying, and no out-of-scope findings.Generated by Claude Code