From b0479120fc6df192988307fe1abfc2b82576a0d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 05:55:48 +0000 Subject: [PATCH] fix(plugin-charts): the schema normalizer and chart registration are reachable through the published surface (#4529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- ...n-charts-publish-normalize-chart-schema.md | 9 ++++ packages/plugin-charts/src/index.tsx | 13 +++++ .../DatasetWidget.chartConfig.dom.test.tsx | 37 +++++++++++---- .../DatasetWidget.comboPresentation.test.tsx | 7 +-- packages/plugin-dashboard/tsconfig.test.json | 47 +++++++------------ 5 files changed, 69 insertions(+), 44 deletions(-) create mode 100644 .changeset/plugin-charts-publish-normalize-chart-schema.md diff --git a/.changeset/plugin-charts-publish-normalize-chart-schema.md b/.changeset/plugin-charts-publish-normalize-chart-schema.md new file mode 100644 index 0000000000..1a04e62d5e --- /dev/null +++ b/.changeset/plugin-charts-publish-normalize-chart-schema.md @@ -0,0 +1,9 @@ +--- +'@object-ui/plugin-charts': minor +--- + +Publish `normalizeChartSchema` from the package entry. + +`normalizeChartSchema` is the single place the author-facing chart schema is translated into the renderer's internal pipeline contract, and `ChartRenderer` calls it on every render. It was not reachable from the package's only entry point, so a consumer that wanted to assert what `AdvancedChartImpl` is actually handed had to restate the translation rather than run it. It is now exported from the entry, along with the `NormalizedChartSchema` type it returns. + +Additive only: nothing is removed or renamed, and the module was already in the entry's eager import graph via `ChartRenderer`, so this publishes a name rather than shipping new bytes. diff --git a/packages/plugin-charts/src/index.tsx b/packages/plugin-charts/src/index.tsx index c840f06516..abd65991d4 100644 --- a/packages/plugin-charts/src/index.tsx +++ b/packages/plugin-charts/src/index.tsx @@ -15,6 +15,19 @@ export type { BarChartSchema } from './types'; export { ChartBarRenderer, ChartRenderer }; export { ObjectChart, ObjectChartBlock } from './ObjectChart'; +// The ONE place the author-facing chart schema is translated into the +// renderer's internal pipeline contract (#2880 S1). Published from this entry +// — the package's only door — because a consumer that wants to know what +// `AdvancedChartImpl` is actually handed has to run the schema through the SAME +// translation the runtime applies; restating the translation instead would make +// the assertion a copy of the thing under test (objectui#4529, #4471). +// +// Costs nothing to ship: `ChartRenderer` above already imports this module +// statically, so it is in this entry's eager graph either way. This adds a +// name, not a byte. +export { normalizeChartSchema } from './normalizeChartSchema'; +export type { NormalizedChartSchema } from './normalizeChartSchema'; + // Standard Export Protocol - for manual integration export const chartComponents = { 'bar-chart': ChartBarRenderer, diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx index fdaf5b0d0b..9da9dd4673 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx @@ -26,16 +26,19 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, cleanup, screen, waitFor } from '@testing-library/react'; // Registers `chart` in the ComponentRegistry, which is what `SchemaRenderer` -// resolves the widget's `{ type: 'chart' }` schema through. +// resolves the widget's `{ type: 'chart' }` schema through. This is the +// package's whole published surface, and it is all this file needs: production +// reaches `AdvancedChartImpl` ONLY through the `React.lazy(() => +// import('./AdvancedChartImpl'))` factory inside `ChartRenderer`, so this test +// reaches it the same way — by rendering the real chain and awaiting the +// Suspense boundary (objectui#4529). It used to also side-effect import +// `@object-ui/plugin-charts/AdvancedChartImpl` to pre-warm that chunk, but the +// package publishes `"."` alone, so that specifier resolved only through this +// repo's vitest alias — the objectui#4325 packaging gap, whose ruling (PR +// #4460) is that the unpublished deep subpath goes rather than the surface +// growing to keep it alive. See `renderWidget` for how the wait it used to +// shorten is now budgeted instead. import '@object-ui/plugin-charts'; -// `ChartRenderer` renders its implementation behind -// `React.lazy(() => import('./AdvancedChartImpl'))`, and every assertion below -// lives inside that Suspense boundary. Loading it is unbounded work — under full -// parallelism a first import of the recharts graph can outlast RTL's 1000ms -// `waitFor` window — so pay it in the import phase, which no test or hook -// timeout applies to (AGENTS.md §测试纪律). The alias maps this specifier to the -// very file the lazy factory imports, so the ESM cache already holds it. -import '@object-ui/plugin-charts/AdvancedChartImpl'; import { DatasetWidget } from '../DatasetWidget'; afterEach(cleanup); @@ -61,7 +64,21 @@ const renderWidget = async (chartConfig?: Record) => { ); // The chart container only exists once the dataset resolves AND the lazy chart // chunk has mounted — i.e. once the whole dashboard chart path really ran. - await waitFor(() => expect(view.container.querySelector('[data-slot="chart"]')).not.toBeNull()); + // + // Every witness in this file is post-boundary DOM by design (that is the point + // of the file — see the header), so unlike objectui#4325's case the race + // cannot be removed by choosing a witness that survives every state of the + // boundary. It is BUDGETED instead. Measured on an idle container: 359 ms from + // render to chart mount with the lazy chunk cold, against 91 ms when it had + // been eagerly pre-imported — so dropping the pre-import moves ~270 ms inside + // this wait. That fits RTL's 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. The budget below is generous on purpose — + // `waitFor` polls and returns as soon as the node appears, so a large timeout + // costs nothing when the chunk is warm and only spends time it actually needs. + await waitFor(() => expect(view.container.querySelector('[data-slot="chart"]')).not.toBeNull(), { + timeout: 15000, + }); return view; }; diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx index d685b4c326..ca193a32b6 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx @@ -47,9 +47,10 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, cleanup, waitFor } from '@testing-library/react'; // The renderer's own translation layer, imported read-only so these assertions // are made against what `AdvancedChartImpl` receives rather than a restatement -// of it. `plugin-charts` is a devDependency of this package and vitest aliases -// it to source. -import { normalizeChartSchema } from '@object-ui/plugin-charts/normalizeChartSchema'; +// of it. `plugin-charts` is a devDependency of this package, and this is its +// PUBLISHED root entry — the same module `ChartRenderer` calls, reached the way +// any consumer outside this repo would reach it (objectui#4529). +import { normalizeChartSchema } from '@object-ui/plugin-charts'; let lastChartSchema: any = null; diff --git a/packages/plugin-dashboard/tsconfig.test.json b/packages/plugin-dashboard/tsconfig.test.json index 4e606b64d7..bec3c207aa 100644 --- a/packages/plugin-dashboard/tsconfig.test.json +++ b/packages/plugin-dashboard/tsconfig.test.json @@ -39,38 +39,23 @@ // // `paths` drops the root tsconfig's source-tree mappings so `@object-ui/*` // and `@objectstack/spec` resolve through each workspace dependency's built - // `.d.ts` rather than pulling sibling sources in as program inputs (TS6059) - // — with ONE deliberate exception, which is not a workaround but a - // restatement of what the runtime already does: + // `.d.ts` rather than pulling sibling sources in as program inputs (TS6059). // - // Two suites import `@object-ui/plugin-charts` SUBPATHS, and the repo's - // vitest config (`vitest.config.mts`) aliases the whole package to its - // source, which is the only reason they resolve when the tests RUN: - // - `DatasetWidget.chartConfig.dom.test.tsx` side-effect imports - // `.../AdvancedChartImpl` at module scope to pre-load the chunk - // `ChartRenderer` reaches through `React.lazy`, so the unbounded first - // import is paid in the import phase instead of inside RTL's 1000 ms - // `waitFor` (AGENTS.md §测试纪律). - // - `DatasetWidget.comboPresentation.test.tsx` imports - // `.../normalizeChartSchema` as a VALUE and runs the emitted schema - // through it, so what it pins is what `AdvancedChartImpl` receives - // rather than a restatement of it. - // Neither subpath is published: `@object-ui/plugin-charts`' `exports` map - // declares `"."` alone, and neither name is on the barrel either, so both - // specifiers exist only inside this repo — the same packaging gap - // objectui#4325 found behind `@object-ui/fields/widgets/*`. Filed as - // objectui#4529 rather than fixed here: what this package publishes is a - // product call, and #4325's own answer ("drop the import") does not - // transfer to `normalizeChartSchema`, which is a VALUE the assertions run - // the emitted schema through. The mapping below points `tsc` at the - // very files vitest loads, so the compiler checks the real modules; it is - // the honest description of today's state, and it should be DELETED the - // moment either the subpaths are published or the imports are rebuilt on - // a witness that does not need them. Values are relative to the inherited - // `baseUrl` (the repo root), not to this file. - "paths": { - "@object-ui/plugin-charts/*": ["packages/plugin-charts/src/*"] - } + // Empty, like every other wired-up package's. It briefly carried one + // exception — `@object-ui/plugin-charts/*` mapped to that package's source — + // because two suites imported subpaths the package does not publish (its + // `exports` map declares `"."` alone), so they resolved only through the + // repo's vitest alias. objectui#4529 ruled both, and both imports now go + // through the published root entry, so the mapping is gone rather than + // maintained: + // - `normalizeChartSchema` is a genuine cross-package contract — the + // assertions run the emitted schema through the SAME translation layer + // the runtime applies — so plugin-charts publishes it from its entry. + // - `AdvancedChartImpl` was an eager pre-warm of a chunk production only + // ever reaches through `React.lazy`; the surface does not grow to keep + // an eager deep import alive (objectui#4325 / PR #4460), so the import + // is gone and the wait it shortened is budgeted explicitly instead. + "paths": {} }, // `src/**/*.d.ts` is pulled in explicitly: the build program gets ambient // declarations for free from `"include": ["src"]`, but an ambient declaration