Uh oh!
There was an error while loading. Please reload this page.
refactor(components)!: remove the duplicate chart primitives, plugin-charts is the one implementation - #7626
Merged
os-warren merged 2 commits intoSep 4, 2026
Conversation
…i#7397) `packages/components/src/ui/chart.tsx` was a second copy of the primitives in `packages/plugin-charts/src/ChartContainerImpl.tsx`, and it carried the label-resolution hole that objectui#7248 fixed in the plugin-charts one. `@object-ui/plugin-charts` is now the single implementation, per the maintainer ruling of 2026-09-04 (decision batch #28, option (a)). The copy was NOT unreachable, contrary to the finding that opened the card: `src/index.ts` re-exported `./ui`, `ui/index.ts` re-exported `./chart`, and `chart.tsx` ended in a trailing `export { ... }` block — two `export *` hops, so `ChartContainer`, `ChartTooltip`, `ChartTooltipContent`, `ChartLegend`, `ChartLegendContent`, `ChartStyle` and the `ChartConfig` type were all on the published surface of `@object-ui/components`, and an external consumer importing `ChartLegendContent` got the unfixed copy. That makes this a breaking change, shipped as `minor` per this repo's release model. `shadcn-components.json` moves `chart` out of `components` and into `customComponents` with `movedToPlugin: "@object-ui/plugin-charts"` — entries there are never fetched, so `pnpm shadcn:update-all` can no longer resurrect the file. A pin test holds the barrel, the file's absence and that manifest record together, because "two copies of one primitive is how a fixed bug returns" is a claim that has to outlive this diff. Consumer census before the deletion, with a lit control: zero in-repo importers across apps/**, examples/** and packages/**. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
…ove-duplicate-chart-primitives
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-warren
commented
Sep 4, 2026
Collaborator
Contract review PASS at Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
os-warren
deleted the
claude/issue-7397-remove-duplicate-chart-primitives
branch
September 4, 2026 09:23
This was referenced Sep 4, 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 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#7397
Implements the maintainer ruling of 2026-09-04 (decision batch #28, option (a), verbatim 「同意」, recorded on the card as comment 5536046538): the chart primitives are removed from
@object-ui/components, andpackages/plugin-charts/src/ChartContainerImpl.tsxis the single implementation.⛔ Not (b) keep-and-port: "two copies of one primitive is how a fixed bug returns."
⛔ Not (c) re-export:
plugin-chartsdepends on@object-ui/components(workspace:*), so the dependency direction forbids it.The card's body was wrong, and this PR corrects it
The finding recorded the copy as "not re-exported from the
packages/componentsbarrel" and therefore "currently unreachable". Both were false, re-verified here onorigin/main0d8fd7ce:Two
export *hops, so all six primitives plus theChartConfigtype alias were on the published surface, and an external consumer importingChartLegendContentfrom@object-ui/componentsgot the copy carrying the legend hole objectui#7248 had already repaired in the other implementation. That makes this a breaking change (Clause-②: yes), not a tidy-up. The card body has been corrected in the same stroke, as the ruling requires.The body's zero came from the probe
^export (const|function|type|interface) ChartLegend...run with no lit control: shadcn exports through a trailingexport { ... }block, so the query shape was wrong and the zero was never a reading. Every zero below therefore carries a control that fires.Consumer census, before the deletion
apps/**,examples/**,packages/**SchemaRenderer→ 3004 hits;Buttonfrom@object-ui/components→ 272 files;./ChartContainerImpl→ 5org:objectstack-aiobjectstack-ai/objectuiobjectstack-ai/hotcrm,objectstack-ai/cloudThe census matched symbol names anywhere in a file, not just import statements, so namespace access (
import * as C ...; C.ChartContainer) is covered too.apps/holds 2 projects andexamples/4, so the populations were non-empty.hotcrmandcloudreturned HTTP 403 to this seat (GET /repos/...on both), and REST code search is unavailable to this session entirely (/search/code→ 403, "sessions are bound to their configured repositories") — its control did not fire, so no zero is claimed from it. Code search only indexes what the caller can read, so the org-wide search's silence about those two repos is not evidence of absence. This gap is open for the reviewer to close. No real external importer was found in anything that could be read, so the changeset carries the migration line without a further migration note.What changed
packages/components/src/ui/chart.tsx— deleted (384 lines).packages/components/src/ui/index.ts— theexport * from './chart';line removed.packages/components/shadcn-components.json—chartmoves out ofcomponentsand intocustomComponentswithmovedToPlugin: "@object-ui/plugin-charts". This is the anti-resurrection record the ruling asks for:scripts/shadcn-sync.jsiteratesObject.keys(manifest.components), andupdateComponentrefuses a name that is not there, sopnpm shadcn:update-allandpnpm shadcn:update chartcan no longer rewrite the file.movedToPluginis the vocabulary this manifest already uses forcalendar-view,chatbotandtimeline— no new mechanism, andREADME_SHADCN_SYNC.mdalready documents its meaning, so the README needs no edit (its## Component Categoriesprose names no component, whichreadme-shadcn-sync-categories.test.tsenforces)..changeset/7397-remove-duplicate-chart-primitives.md—@object-ui/components: minor, BREAKING-as-minor per this repo's policy, migration text "import the chart primitives from@object-ui/plugin-charts".Declared file-surface increment
The dispatch declared the surface as
ui/chart.tsx,ui/index.ts, the sync exclusion record and.changeset/. This PR adds one file beyond it, named here rather than slipped in:packages/components/src/__tests__/chart-primitives-removed-7397.test.ts. The ruling's reason ("two copies of one primitive is how a fixed bug returns") is a claim that has to outlive this diff, and the manifest record only blocks resurrection by sync — nothing blocked it by hand. The pin holds the barrel surface, the file's absence, and the manifest record together. It conflicts with no open PR.packages/components/package.jsonwas not touched, and that is deliberate:rechartsis now a declared-but-unused dependency there, but that file is outside the declared surface and is held by an open claim (dependabot #7058). Filed instead as objectui#7625.Verification
All measurements on the final head
b53a02ad(after mergingorigin/maind28d87bf). Exit codes captured before any pipe; verdicts quoted from each tool's own line.Gates derived from the actual diff (this repo has no
dispatch-gates.mjs; objectstack's is repo-bound and would answer about the wrong tree):The ratchet was worth measuring rather than assuming.
@object-ui/componentsissideEffects: true, so the deleted file could not be tree-shaken and the hypothesis was that dropping it removesrechartsfrom the console's eager closure — which would leave the ceiling sitting more than one regression above the payload and turncheck:eager-closurered for going blind. A real console build disproves it:vendor-charts(155.2 KB gzipped) is still eagerly reachable through the plugins, the aggregate measures 3180.1 KB against its 3191.4 KB ceiling (headroom 0.13x the 89 KB regression), and no ceiling needs to move.The pin was ablated, in both directions
No rebuild leg was needed: the pin resolves
../indexto the source barrel, neverdist, so no built artefact takes part in the reading. The mutation script carried atrap ... EXIT INT TERMwith absolute paths, and its restore assertion caught a real failure on the first run —git checkout HEAD -- pathcannot restore a path HEAD deletes, so the tree stayed mutated and only the explicitgit diff HEADcheck revealed it. The restore isgit rm -f --ignore-unmatchfor the deleted path; the numbers above are from the corrected run.Landing
Clause-②: yes— public exports removed.needs:contract-reviewis on both carriers. ⛔ This PR is parked as a draft awaiting contract review atclaude-fable-5-1, which the dispatching seat cannot commission. Do not flip it ready, do not enqueue it, and do not strip the label.🤖 Generated with Claude Code
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Generated by Claude Code