Uh oh!
There was an error while loading. Please reload this page.
plugin-view: drop the unreferenced handleRefresh from ObjectView - #5568
Merged
Conversation
…tView `ObjectView` declared a `handleRefresh` callback that bumped `refreshKey` and was referenced by nothing — not passed to the toolbar, not exposed on a handle, not wired to any control. It advertised a refresh entry point the component does not have, which cost objectui#4549 a detour to rule out. Triage ruled deletion rather than wiring it to a new toolbar Refresh button: the button is a feature with no recorded pull, and the affordance already exists downstream in `plugin-list`'s `ListView`, which ObjectView reaches by delegating through `renderListView`. Reference surface verified empty on the merged ref before deleting: the identifier appears exactly once in `packages/plugin-view` (the declaration), is a component-local `const` that no barrel re-exports, and the package's only entry point is `src/index.tsx`. Nothing else fell out — `useCallback`, `setRefreshKey` and `refreshKey` all keep live readers. Adds `ObjectView.refreshSignal.test.tsx`, pinning the refresh paths that survive: a same-object mutation re-fetches, a different-object mutation does not, and a host-supplied list view suppresses the auto-subscribe. Part of #4568
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 13:36
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#4568
ObjectViewdeclared ahandleRefreshcallback that bumpedrefreshKeyand was referenced by nothing — not passed to the toolbar, not exposed on a handle, not wired to any control. It advertised a refresh entry point the component does not have, which cost #4549 a detour to rule out.Triage ruled deletion rather than wiring it to a new toolbar Refresh button: the button is a feature with no recorded pull, and the affordance already exists downstream. That ruling is not re-litigated here.
The reference surface, proven empty before deleting
A dead-code deletion is mechanical only after the reference surface is proven empty, so every zero-hit below is paired with a control probe on a term known to be present in the same scope — a bare zero-hit proves nothing about the search itself. All measured on the merged ref at claim time (
f24195aeb).1. Whole repo, all tracked files.
git grep -n "handleRefresh" -- .returns 17 hits across 7 files. Six of those files (app-shell/.../ExternalDatasourcePanel.tsx,plugin-ai/AIFormAssist.tsx,plugin-dashboard/DashboardGridLayout.tsx,plugin-dashboard/DashboardRenderer.tsx,plugin-report/ReportViewer.tsx,plugin-timeline/ObjectTimeline.tsx) declare their own module-localhandleRefreshand each wires it to something; none of them can see this one. Inpackages/plugin-viewthe identifier appears exactly once — the declaration.Control: the same command shape for
handleDeletereturns hits acrossapps/,packages/, tests and CHANGELOGs, so the scope is live and repo-wide. The sharpest contrast is in this very file:handleDeleteshows a declaration (:817) and a use (:1501);handleRefreshshowed a declaration and nothing.2. Untracked files too. A filesystem
grep -rnexcludingnode_modules/.git/dist/.turbogives the same 17 hits, 1 in plugin-view. Control:setRefreshKeyin the identical scope returns 6 hits inside the same file, so the scope reaches the file's interior rather than stopping at its name.3. Case-insensitive.
git grep -in "handlerefresh" -- packages/plugin-view/returns only the declaration. Control:git grep -in "HANDLEREFRESH"matches that same mixed-case line, proving-iwas actually in effect.4. Not exported — the public-surface flip condition does not trigger.
handleRefreshwas aconstinside theObjectViewcomponent body (component opens at:509), so it is lexically unreachable from outside. The package's only entry point is.→dist/index.*, built fromsrc/index.tsx, and that barrel re-exportsObjectView,ViewSwitcher,FilterUI,SortUI,SharedViewLink,ViewTabBar,ManageViewsDialog, therecordSurfacehelpers and theview-config-utilshelpers — nohandleRefresh. This is an internal cleanup, not a public-surface removal.Where the deletion stopped, and what did not fall out
Nothing was orphaned.
useCallbackstill has 13 uses in the file,setRefreshKeystill has 5, andrefreshKeyitself keeps four live readers (the non-grid fetch effect's dep array:709, the child-view remount key:1356, and therefreshTrigger/refreshKeyprops forwarded to a host list view). No import became unused; no helper became unreferenced. The diff is exactly the five deleted lines plus a test and a changeset.Where the real refresh button lives
Not in this component.
packages/plugin-list/src/ListView.tsxrenders a toolbar Refresh button gated ontoolbarFlags.showRefresh(derived fromuserActions.refresh,:796) driving ListView's own counter (:3080).ObjectViewreaches it by delegating throughrenderListView, and deliberately skips its own mutation auto-subscribe in that mode. That delegation is why the deleted callback never had a toolbar to attach to.Test
packages/plugin-view/src/__tests__/ObjectView.refreshSignal.test.tsxis new. It pins the paths that survive rather than asserting the absence of the one that went — the package previously had zero coverage of refresh oronMutationat all:event.resource === schema.objectNamegate);renderListViewsuppresses the auto-subscribe entirely.Reverse-verified. Direction predicted before running: test 1 only should go red. Ablating the
setRefreshKeycall inside theonMutationhandler (mutation confirmed on disk by grepping the injected marker — 1 — and the anchor text,setRefreshKey5→4) gaveTests 1 failed | 2 passed, exit 1 — exactly the predicted direction. The restore leg is proven byte-identical to the commit (marker absent,setRefreshKeyback to 5, emptygit diffagainstHEAD). The mutation and restore legs needed no rebuild: the test imports../ObjectViewby relative path, so it resolves to source and never readsdist/.Gates
All run on the final commit
f882f07a9, working tree byte-identical to it; each verdict quoted from the gate's own output, with exit codes captured before any pipe.vitest run packages/plugin-view/Test Files 18 passed (18)/Tests 183 passed (183)turbo run type-check --filter @object-ui/plugin-view16 successful, 16 total(includestsconfig.test.json)turbo run lint --filter @object-ui/plugin-view2 successful, 2 total;224 problems (0 errors, 224 warnings)— all warnings pre-existing except 2no-explicit-anyin the new test, matching the siblingObjectView.kanbanConditionalFormatting.test.tsxconvention for test doublescheck:control-bytesOK (scanned 4625 tracked text file(s))check-changeset-presence2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-majorNo changeset declares a major bump.check-changeset-fixedAll workspace packages are in the changeset fixed group.check:self-importNo package names itself inside its own src/.check:phantom-depsEvery in-scope import is declared by the package that publishes it.check:published-distNo published package's build output carries tooling material.(full 39-package build, 221s — independently proves plugin-view builds clean after the deletion)The changeset has empty frontmatter: this is a purely internal deletion with no released behaviour change, and the gate confirms that form is "the explicit exemption and a complete answer to this gate."
Declared narrowing. Repo-wide
pnpm lint(turbo run lintacross all 44 packages) was not run locally; CI runs the farm regardless. The narrowing is a measurement, not a gap: (1) the changed-file population is the completegit diff --name-onlyagainst the merge-base — three files, all inpackages/plugin-view/src/and.changeset/; (2) the file count comes from eslint's own--format jsonoutput, 33 files linted in@object-ui/plugin-view, 0 errors; (3)eslint.config.jshas zero matches forprojectService/parserOptions, so type-aware linting is not enabled and this diff cannot move a verdict in any file it does not itself contain.Out of scope
While proving the reference surface I measured a second, adjacent dead declaration and filed it separately as #5567 rather than fixing it here: the
object-viewregistry declaresshowRefreshas a designer input, defaults it totrue, and the docs describe it as a working toolbar toggle — butObjectViewnever reads it and does not forward it. That key lives inindex.tsx(outside this card's file surface) and is authorable surface, so its disposition is a decision rather than a cleanup. Not addressed here.Generated by Claude Code