fix(plugin-charts): the schema normalizer and chart registration are reachable through the published surface (#4529) - #4537
Merged
Conversation
…reachable through the published surface (#4529) Two plugin-dashboard suites imported `@object-ui/plugin-charts` subpaths the package does not publish (its `exports` map declares "." alone). They resolved only through the repo's vitest alias, and PR #4530 had to restate that alias as a transitional `paths` entry so tsc could check what vitest loads. The two imports get two different dispositions, each measured: - `normalizeChartSchema` is a load-bearing VALUE — the assertions run the emitted schema through the same translation the runtime applies. It is now exported from the package ROOT entry. Measured: `ChartRenderer` already imports it statically, so it was in the entry's eager graph already; this publishes a name, not bytes. plugin-charts graded MINOR. - `AdvancedChartImpl` was an eager pre-warm of a chunk production only ever reaches through `React.lazy`, so #4325's ruling (PR #4460) transfers: the deep import is dropped rather than the surface growing to keep it alive. The test now reaches the registration exactly as production does. Measured: importing the published barrel does NOT preload the chunk (60.3 ms first dynamic import after the barrel, against 0.0 ms for a real cache hit), so the ~270 ms this moves inside the wait is budgeted explicitly instead of raced. The transitional `paths` entry is deleted; `tsc -p tsconfig.test.json` exits 0 with both imports resolving through the published surface. 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
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)
Auto-merge armed (squash) — landing will be verified per the new merge-queue discipline. Generated by Claude Code Generated by Claude Code |
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 #4529.
@object-ui/plugin-chartspublishes exactly one entry point, and twoplugin-dashboardsuites imported subpaths of it. Both resolved only through the repo's vitest alias; under Node's own resolution, or from any consumer outside this repo, both areERR_PACKAGE_PATH_NOT_EXPORTED. PR #4530 could not fix that from a test-surface card, so it restated the alias as one narrow, commentedpathsentry inpackages/plugin-dashboard/tsconfig.test.json, with the instruction to delete it once this card ruled.The two imports are different kinds of dependency and get two different dispositions, each measured rather than assumed.
1.
normalizeChartSchema— a load-bearing value, published from the root entryMeasured first, per the ruling: is it already reachable from the root entry? No.
packages/plugin-charts/src/index.tsxexportedChartBarRenderer,ChartRenderer,ObjectChart,ObjectChartBlock,chartComponentsand theBarChartSchematype, and nothing else.Is the exclusion deliberate — a lazy-loading or bundle-size constraint? No, and this is the measurement that decides it:
ChartRenderer.tsxline 5 importsnormalizeChartSchemastatically, andindex.tsximportsChartRendererstatically. The module is already in the entry's eager graph. Publishing it adds a name, not a byte. So the ruling's fallback (a subpath export for a deliberately-excluded module) does not apply, and the primary branch lands: it is exported from the package root entry, together with theNormalizedChartSchematype it returns so the published function's contract is nameable.This is a genuine cross-package contract, not a test convenience. The assertions run the widget's emitted schema through the same translation layer the runtime applies, which is exactly what a consumer does; restating that translation in the test instead would replace the test with a copy of the thing under test (#4471).
2.
AdvancedChartImpl— the #4325 precedent transfers, and the deep import goesMeasured: how does production load it? One way only.
ChartRenderer.tsx:There is no eager path anywhere in production. The test's side-effect import existed only to pre-warm that chunk so the unbounded first import was paid in the import phase rather than inside RTL's 1000 ms
waitFor.Measured: does the published barrel preload it? No — #4460's refutation of "preload through the public index" reproduces here rather than being cited. After importing
@object-ui/plugin-charts, a first dynamic import of the implementation costs 60.3 ms; a genuine cache hit immediately after costs 0.0 ms. The barrel evaluates the module that declares the lazy factory; the factory never runs.So the surface does not grow to keep an eager deep import alive (#4325, PR #4460). The import is deleted and the test reaches the registration exactly the way production does: render the real chain and await the Suspense boundary.
Measured: what does that cost, and how is it handled? Unlike #4325's case, the race cannot be removed by choosing a better witness — every witness in this file is post-boundary DOM by design, which is the entire point of the file. So it is budgeted instead. On an idle container, render to chart mount takes 359.1 ms with the chunk cold against 91.4 ms when eagerly pre-imported, so dropping the pre-import moves about 270 ms inside that one wait. That fits the 1000 ms default here and would still be a coin flip on a loaded machine — AGENTS.md records first-import latencies up to 976 ms under full parallelism — so the single
waitForthat spans the boundary is given an explicit 15 s budget.waitForpolls and returns as soon as the node appears, so the budget costs nothing when the chunk is warm.No test semantics changed. Same assertions, same witnesses, same
renderWidget, in both files. The deleted import was a preload, never a witness; the changed import inDatasetWidget.comboPresentation.test.tsxis the same function reached by its published spelling.3. The transitional
pathsentry is deletedpackages/plugin-dashboard/tsconfig.test.jsonnow carries"paths": {}, like every other wired-up package, with the comment rewritten to record what was ruled rather than what was owed.Red-first
Predicted in writing before running: deleting the
pathsentry with the imports unchanged reports exactly the two errors #4529 quoted, and nothing else.Exit 2, error count 2. After the fix,
tsc -p tsconfig.test.jsonexits 0 with"paths": {}— both imports resolving through the published surface.Reverse verification — direction predicted first, and it is not the obvious one
Reverting all four files returns to
origin/main, which was green (thepathsentry restated the alias), so that direction proves nothing. The load-bearing question is whether the new published surface is what carries the value import, so the limb removed waspackages/plugin-charts/src/index.tsxalone, with the tests' new spellings and the emptiedpathsleft in place. Three predictions, all confirmed:dist/index.d.tsreturns byte-identical to the pre-fix baseline05539ce9…both timesDatasetWidget.comboPresentation.test.tsx(53,10): error TS2305: Module '"@object-ui/plugin-charts"' has no exported member 'normalizeChartSchema'.— 1 errorchartConfig.dom.test.tsxstays silent, since it imports only the barrel for its side effectTaken out with
git checkout --and restored from a sha256-verified copy (2c6f2834…, matched after restore), nevergit stash. Rebuilt and re-checked green afterwards.The
.d.tsdiff, measured both waysdist/andtsconfig.tsbuildinfocleared between builds. The entire diff:Additive only — nothing removed, nothing renamed, no signature narrowed. Measured in the other direction too: removing the limb returns the file to the baseline hash exactly. Minor, never major.
Green, and what must not change
tsc -p tsconfig.test.json(plugin-dashboard tests)pnpm --filter @object-ui/plugin-dashboard type-check(both passes)pnpm --filter @object-ui/plugin-charts type-checkvitest run packages/plugin-dashboard/vitest run packages/plugin-dashboard/ packages/plugin-charts/node scripts/check-type-check-coverage.mjsnode scripts/check-control-bytes.mjsnode scripts/check-phantom-dependencies.mjscheck-changeset-presence/-no-major/-fixedeslinton all changed filesOne declared deviation
ESLint gains one new warning, not zero:
react-refresh/only-export-componentsatpackages/plugin-charts/src/index.tsx:28, on the newexport { normalizeChartSchema }line. Baseline for that file onorigin/mainis 1 warning of the same rule on the same grounds (its pre-existingchartComponentsexport); my tree has 2. It is an inherent consequence of adding a value export to a barrel that already trips this rule, and it is left un-suppressed rather than silenced with a disable comment that its identical neighbour does not carry. Zero errors, and the lint workflow deliberately does not set--max-warnings. The two test files are unchanged at 13 warnings / 0 errors, all pre-existingno-explicit-any.Changeset:
@object-ui/plugin-chartsminor — entry-reachable additive growth, as ruled.plugin-dashboard's changes are two test files and a test-only tsconfig, which release nothing.Generated by Claude Code