docs(plugin-charts): mark normalizeSeries' chartType limb internal-only and pin the split - #7815
Merged
Merged
Conversation
…only and pin the split (objectui#7744) `normalizeSeries` resolves the per-series family as `str(raw.chartType) ?? str(raw.type)` — the INTERNAL spelling first. That limb is correct and lit: `chartType` is the carrier of the renderer's internal `dataKey` series shape, which `ChartRenderer`'s `series?` union declares on that arm and on no other, and which `DashboardRenderer` / `ObjectView` / the dataset path write. But the same function normalizes authored spec-shape series too, and by the time `raw` arrives it is a bag of keys with nothing on it naming its producer. So an author who skips validation still gets `chartType` honoured at runtime — over the declared `type`, read second — while the authoring face refuses that same key by name (`ChartDataSeriesSchema`, objectui#7694 / PR #7737). The two faces disagree, and nothing said so anywhere. This annotates and pins that split rather than retiring the limb: retiring it would change what the internal producers render, a reader-side decision of its own. No behaviour changes — `normalizeSeries` returns byte-identical output for 336 input cases before and after, and the probe is sensitive (a reader reorder moves 49 of them). - the `normalizeSeries` docblock names which face owns which spelling, DELEGATES the refusal's rule and remedy to `ChartDataSeriesSchema` rather than copying them, and records why a green ablation on the first limb is not a licence to delete it; - `chartType-internal-only-7744.test.ts` pins the RELATIONSHIP the two existing files each held only half of: the key the validator refuses is the key the reader prefers. It reads one fixture writing BOTH spellings with DIFFERENT values, so a reorder is observable and not just a deletion; and it carries a compile-time half over `ChartRenderer`'s `series?` union arms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
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
|
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 #7744
Option B of the card's three-way disposition, per the PM ruling on the claim comment: the
chartTypelimb is annotated as internal-only and the split is pinned. Not A (leave the twofaces disagreeing with nothing written down), and not C — retiring the limb at the reader
would change what the internal producers render, which is a reader-side decision of its own.
No behaviour changes.
normalizeSeriesreturns byte-identical output for the same input;the diff is a docblock, a two-line comment beside the limb, one new test file, and an
empty-frontmatter changeset.
ChartRenderer.tsxandpackages/types/are untouched.What the split is
normalizeSeriesresolves the per-series family asstr(raw.chartType) ?? str(raw.type)— theinternal spelling first. That limb is correct:
chartTypeis the carrier of the renderer'sinternal
dataKeyseries shape.ChartRenderer'sseries?prop is a union of exactly the twoshapes, and it declares
chartType?on thedataKeyarm and on no other — the authored armcarries
type?instead.DashboardRenderer,ObjectViewand the dataset path write that arm.But the one function normalizes both shapes, and by the time
rawarrives it is a bag of keyswith nothing on it naming its producer. So an author who skips validation still gets
chartTypehonoured at runtime — over the declared
type, read second — while the authoring face refusesthat same key by name. Two faces, one disagreement, and until now nothing anywhere said so.
1. The annotation, and the refusal it delegates to
The
normalizeSeriesdocblock now names which face owns which spelling, records that the limb islit (and that a green ablation on it is not a licence to delete it), and states that the split is
deliberate and pinned.
It delegates rather than summarises: the refusal is named as
ChartDataSeriesSchema.chartTypeinpackages/types/src/zod/data-display.zod.ts(objectui#7694 / PR #7737), together with its existing pin
packages/types/src/__tests__/chart-series-chart-type-alias-refusal-7694.test.ts. The rule, itsmessage and its remedy are not copied into plugin-charts — per objectui#7765's landing record,
a one-line summary in a second file rots, because only one of the two files is ever edited.
2. The pin, and why this shape catches drift
packages/plugin-charts/src/__tests__/chartType-internal-only-7744.test.ts.The refusal already had a pin, and the reader already had pins. Neither file pinned the pair —
which is exactly how the split survived to be found in the contract review of PR #7737. What this
case pins is the relationship: the key the validator refuses is the key the reader prefers.
Shape choices that make it catch drift rather than describe it:
{ name: 'margin', type: 'area', chartType: 'line' }—so the two faces are measured against the same input, not against two fixtures free to drift.
observable at all. With only
chartTypewritten, a reordered reader returns the identical answerand a pin on the value alone stays green straight through the change it exists to catch.
['chartType']— so the fixture beingrefused for some other reason fails too, rather than passing as a phantom. An accept-control on
the same document with the key dropped is the other half of that.
chartTypeis honoured somehow"; each one failsunder exactly one of the two behaviours, and the messages name objectui#7744 and say which side
moved (
"area"means REORDERED,undefinedmeans DELETED).ChartRenderer'sseries?union arms pins the same split inTypeScript. Confirmed compiled, not phantom:
tsc -p tsconfig.test.json --listFilesresolves thefile (1 hit, not 0).
3. Proof of no behaviour change (same input, same output)
A probe imports the module and runs 336 input cases through
normalizeChartSchema— the onlypublic entry into
normalizeSeries— pluscomboBaseFamily,effectiveChartFamily,formatterFor,ticksFor,domainForand the exported sets. 288 of those cases are thechartType/typelimb exhaustively: every combination of present/absent across 12 values perspelling, including disagreeing pairs, empty strings, wrong-typed values, and the same matrix again
reached through
categories.The pre-edit blob is
05a66dfb5b9c9a322761e1e1a694ac7a57af1e54— the same blob the card names atboth ends of PR #7737, so this is measured against exactly the reader the reviewer measured.
The probe was itself shown sensitive, otherwise "identical" would be vacuous: reordering the
limb in a scratch copy (never in the repo) moves 49 of the 336 cases, e.g.
{ name: 'margin', chartType: 'bar', type: 'line' }goes fromchartType: 'bar'to'line'.The stronger reading agrees: the package's pre-existing suite was 45 files / 410 tests, all
passing before the edit and 46 files / 416 tests, all passing after — the delta is exactly
this PR's one new file and its six cases; no pre-existing test changed verdict.
4. Lit controls — the pin goes red from either side, and names it
Both mutations ran under
trap … EXIT INT TERMwith absolute paths, the mutation confirmed on diskby anchored text counts and a changed blob hash before anything was read (an empty hash reads as
failure), and restoration proved by blob-hash equality plus an empty
git diff HEAD— state,not whether the trap fired. Both were run after the implementation was committed, so
HEADisthe correct restore reference.
str(raw.chartType) ?? str(raw.type)reorderedchartType…" and "THE SPLIT…"ChartDataSeriesSchema.chartTypemade to acceptchartType…" and "THE SPLIT…"The relationship case is red in both directions; the side-specific cases name which face
moved. Sample diagnostic, verbatim from the reader mutation:
Restoration, both controls:
5. Verification
Union run after the final commit, at
1120ee392, uncached (--force), through turbo:pnpm turbo run test type-check --filter=@object-ui/plugin-charts --forcepnpm --filter @object-ui/plugin-charts lintDerived gate family for the changed paths (
.changeset/*.md,packages/plugin-charts/src/**), allexit 0:
check-control-bytes·check-changeset-no-major·check-changeset-overwrite·check-changeset-fixed·check-lint-coverage·check-type-check-coverage·check-unreferenced-sources. Also self-scanned for control bytes beyond the gate(
grep -naPover both touched files, no hits).Declared narrowing: the repo-wide
pnpm lintwas not run locally; eslint was run on theaffected package only. Evidence the narrowing excludes nothing: the population is read from
eslint's own config, not guessed (57 files resolved in this package, from
--format json); thisrepo has no type-aware linting configured (no
projectService/parserOptions.projectineslint.config.*), so a comment-only edit cannot move any untouched file's verdict. CI runs thefull farm regardless.
6. Changeset gate — verdict verbatim
node scripts/check-changeset-presence.mjs, exit 0:The empty-frontmatter form is the honest one here: B ships no published behaviour. Never
major.No
skip-changesetlabel was applied — in this repo that label reads nothing and exempts nothing;the empty-frontmatter declaration is the mechanism, and the gate's verdict line is the authority.
Clause-② remains
noNothing here makes
normalizeSeriesproduce a different output for the same input — that ismeasured above, not asserted. The limb is untouched (still 1 occurrence, unchanged);
ChartRenderer.tsxandpackages/types/carry no edit in this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Generated by Claude Code