From 5153db6f7154af4cf2e0040cf5c25b42d8779184 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 11 Jun 2026 18:31:39 +0200 Subject: [PATCH 1/2] perf(appkit-ui): enable tree-shaking with sideEffects flag and modular echarts imports Importing any single component from the @databricks/appkit-ui/react barrel (e.g. Button) previously forced bundlers to retain every re-exported module, including the full echarts bundle (~350KB gz), because the package declared no sideEffects field and charts/base.tsx imported the monolithic echarts entry via echarts-for-react's default export. - Declare "sideEffects": ["**/*.css"] so bundlers can drop unused modules while keeping the dist/styles.css side-effect import alive. No JS module in the package relies on import-time side effects from another module; the ECharts registration lives in the same module as BaseChart, so it is always retained when charts are used. - Switch base.tsx to echarts-for-react/lib/core + echarts/core, registering only what the option builders use: LineChart, BarChart, PieChart, ScatterChart, HeatmapChart, RadarChart; Title/Tooltip/Legend/Grid/ VisualMap components; LegacyGridContainLabel (grid.containLabel) and CanvasRenderer. Keep ECharts instance type as a type-only import. - Add mount tests for every chart family that fail on ECharts missing-registration errors (which are logged, not thrown). Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit-ui/package.json | 3 + .../src/react/charts/__tests__/base.test.tsx | 208 ++++++++++++++++++ packages/appkit-ui/src/react/charts/base.tsx | 59 ++++- 3 files changed, 267 insertions(+), 3 deletions(-) create mode 100644 packages/appkit-ui/src/react/charts/__tests__/base.test.tsx diff --git a/packages/appkit-ui/package.json b/packages/appkit-ui/package.json index a1d62e0a8..815a630b7 100644 --- a/packages/appkit-ui/package.json +++ b/packages/appkit-ui/package.json @@ -3,6 +3,9 @@ "type": "module", "version": "0.41.2", "license": "Apache-2.0", + "sideEffects": [ + "**/*.css" + ], "repository": { "type": "git", "url": "git+https://github.com/databricks/appkit.git" diff --git a/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx b/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx new file mode 100644 index 000000000..51fdcd388 --- /dev/null +++ b/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx @@ -0,0 +1,208 @@ +/** + * Mount tests for BaseChart with the modular ECharts build. + * + * `base.tsx` imports from `echarts/core` and registers only the chart types, + * components, and renderer that the option builders use. ECharts does NOT + * throw when a series type or component is missing from the registration — + * it logs an error like "Series heatmap is used but not imported" and renders + * nothing. These tests mount every chart family and assert no such error is + * emitted, so a missing registration fails the suite instead of silently + * producing blank charts. + */ +import { cleanup, render, waitFor } from "@testing-library/react"; +import { + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, + vi, +} from "vitest"; +import { BaseChart } from "../base"; +import type { ChartType } from "../types"; + +// ---------------------------------------------------------------------------- +// jsdom canvas stub: ECharts' CanvasRenderer needs a 2D context, which jsdom +// does not implement. A Proxy that no-ops every method (and returns a metrics +// object for measureText) is enough for ECharts to lay out and "paint". +// ---------------------------------------------------------------------------- +beforeAll(() => { + // jsdom doesn't implement window.matchMedia, which the chart theme hook + // reads to track color-scheme changes. + if (!window.matchMedia) { + Object.defineProperty(window, "matchMedia", { + writable: true, + value: (query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + dispatchEvent: vi.fn(), + }), + }); + } + + const contextStub = new Proxy( + {}, + { + get(target: Record, prop: string) { + if (prop === "measureText") { + return () => ({ width: 10 }); + } + if ( + prop === "createLinearGradient" || + prop === "createRadialGradient" + ) { + return () => ({ addColorStop: () => {} }); + } + if (!(prop in target)) { + target[prop] = vi.fn(); + } + return target[prop]; + }, + set() { + return true; + }, + }, + ); + + Object.defineProperty(HTMLCanvasElement.prototype, "getContext", { + writable: true, + value: () => contextStub, + }); +}); + +const registrationErrors: string[] = []; + +function captureRegistrationIssues(...args: unknown[]) { + const message = args.map(String).join(" "); + // Matches ECharts messages such as: + // "Series heatmap is used but not imported." + // "Component visualMap is used but not imported." + // "Renderer 'canvas' is not imported." + // "Specified `grid.containLabel` but no `use(LegacyGridContainLabel)`" + if (/not (imported|exists|registered)|no `?use\(/i.test(message)) { + registrationErrors.push(message); + } +} + +beforeEach(() => { + registrationErrors.length = 0; + vi.spyOn(console, "error").mockImplementation(captureRegistrationIssues); + vi.spyOn(console, "warn").mockImplementation(captureRegistrationIssues); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const cartesianData = [ + { month: "Jan", revenue: 100, cost: 60 }, + { month: "Feb", revenue: 120, cost: 70 }, + { month: "Mar", revenue: 90, cost: 50 }, +]; + +const heatmapData = [ + { day: "Mon", hour: "9am", value: 3 }, + { day: "Mon", hour: "10am", value: 7 }, + { day: "Tue", hour: "9am", value: 5 }, + { day: "Tue", hour: "10am", value: 1 }, +]; + +describe("BaseChart ECharts registration", () => { + const cartesianTypes: ChartType[] = ["line", "area", "bar", "scatter"]; + + test.each(cartesianTypes)( + "renders %s chart without missing-registration errors", + async (chartType) => { + const { container } = render( + , + ); + + await waitFor(() => + expect(container.querySelector("canvas")).not.toBeNull(), + ); + expect(registrationErrors).toEqual([]); + }, + ); + + test("renders horizontal bar chart without missing-registration errors", async () => { + const { container } = render( + , + ); + + await waitFor(() => + expect(container.querySelector("canvas")).not.toBeNull(), + ); + expect(registrationErrors).toEqual([]); + }); + + test.each(["pie", "donut"] as ChartType[])( + "renders %s chart without missing-registration errors", + async (chartType) => { + const { container } = render( + , + ); + + await waitFor(() => + expect(container.querySelector("canvas")).not.toBeNull(), + ); + expect(registrationErrors).toEqual([]); + }, + ); + + test("renders radar chart without missing-registration errors", async () => { + const { container } = render( + , + ); + + await waitFor(() => + expect(container.querySelector("canvas")).not.toBeNull(), + ); + expect(registrationErrors).toEqual([]); + }); + + test("renders heatmap (visualMap component) without missing-registration errors", async () => { + const { container } = render( + , + ); + + await waitFor(() => + expect(container.querySelector("canvas")).not.toBeNull(), + ); + expect(registrationErrors).toEqual([]); + }); +}); diff --git a/packages/appkit-ui/src/react/charts/base.tsx b/packages/appkit-ui/src/react/charts/base.tsx index 6a623eb4e..f5dfb1ec5 100644 --- a/packages/appkit-ui/src/react/charts/base.tsx +++ b/packages/appkit-ui/src/react/charts/base.tsx @@ -1,5 +1,25 @@ +// Type-only import: erased at compile time, does not pull the full echarts +// bundle in. echarts-for-react's API is typed against this `ECharts` class. import type { ECharts } from "echarts"; -import ReactECharts from "echarts-for-react"; +import { + BarChart, + HeatmapChart, + LineChart, + PieChart, + RadarChart, + ScatterChart, +} from "echarts/charts"; +import { + GridComponent, + LegendComponent, + TitleComponent, + TooltipComponent, + VisualMapComponent, +} from "echarts/components"; +import * as echarts from "echarts/core"; +import { LegacyGridContainLabel } from "echarts/features"; +import { CanvasRenderer } from "echarts/renderers"; +import ReactEChartsCore from "echarts-for-react/lib/core"; import { useCallback, useMemo, useRef } from "react"; import { normalizeChartData, normalizeHeatmapData } from "./normalize"; import { @@ -18,6 +38,38 @@ import type { Orientation, } from "./types"; +// ============================================================================ +// ECharts Registration (modular imports for tree-shaking) +// ============================================================================ +// Only the chart types and components used by the option builders in +// `options.ts` are registered. Importing from `echarts/core` instead of the +// full `echarts` entry keeps unused chart types (graph, sankey, gauge, ...) +// out of consumer bundles. +// +// If you add a new chart type or use a new ECharts feature (e.g. dataZoom, +// toolbox, markLine), register it here. Consumers passing custom `options` +// that need extra features can register them in their own app via +// `import { use } from "echarts/core"`. +echarts.use([ + // Series types used by the option builders + LineChart, // line + area charts (area = line with areaStyle) + BarChart, // bar + horizontal bar charts + PieChart, // pie + donut charts + ScatterChart, + HeatmapChart, + RadarChart, + // Components referenced by built options + TitleComponent, // `title` + TooltipComponent, // `tooltip` + LegendComponent, // `legend` + GridComponent, // `grid` / `xAxis` / `yAxis` + VisualMapComponent, // `visualMap` (heatmap color scale) + // Features + LegacyGridContainLabel, // `grid.containLabel` (cartesian option builder) + // Renderer (BaseChart always renders with `renderer: "canvas"`) + CanvasRenderer, +]); + // ============================================================================ // Palette Selection // ============================================================================ @@ -137,7 +189,7 @@ export function BaseChart({ // Callback ref pattern: captures the ECharts instance when ReactECharts mounts // This ensures we always have a stable reference to the actual instance - const chartRefCallback = useCallback((node: ReactECharts | null) => { + const chartRefCallback = useCallback((node: ReactEChartsCore | null) => { // Dispose previous instance if component is being replaced if ( echartsInstanceRef.current && @@ -281,8 +333,9 @@ export function BaseChart({ } return ( - Date: Thu, 11 Jun 2026 19:08:00 +0200 Subject: [PATCH 2/2] docs(appkit-ui): document echarts registration contract for custom options - Document on the `options` prop that only the built-in feature set is registered; extra ECharts features (dataZoom, toolbox, markLine, ...) must be registered by the consumer via `use` from "echarts/core", which requires resolving the same echarts module instance/version. - Note in the registration block that tooltip-driven axisPointer ships with TooltipComponent, and that `use()` must stay co-located with BaseChart because package.json#sideEffects declares JS modules pure. - Fix the type-only import comment: the ECharts type is used for the stored instance ref. - Add tests: custom `options` with a registered component logs no registration errors; empty data renders the "No data" fallback. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../src/react/charts/__tests__/base.test.tsx | 27 +++++++++++++++++++ packages/appkit-ui/src/react/charts/base.tsx | 22 +++++++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx b/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx index 51fdcd388..140502f43 100644 --- a/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx +++ b/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx @@ -205,4 +205,31 @@ describe("BaseChart ECharts registration", () => { ); expect(registrationErrors).toEqual([]); }); + + test("renders custom `options` using a registered component without registration errors", async () => { + const { container } = render( + , + ); + + await waitFor(() => + expect(container.querySelector("canvas")).not.toBeNull(), + ); + expect(registrationErrors).toEqual([]); + }); + + test("renders the no-data fallback for empty data without mounting ECharts", () => { + const { container, getByText } = render( + , + ); + + expect(getByText("No data")).toBeTruthy(); + expect(container.querySelector("canvas")).toBeNull(); + expect(registrationErrors).toEqual([]); + }); }); diff --git a/packages/appkit-ui/src/react/charts/base.tsx b/packages/appkit-ui/src/react/charts/base.tsx index f5dfb1ec5..99a931d28 100644 --- a/packages/appkit-ui/src/react/charts/base.tsx +++ b/packages/appkit-ui/src/react/charts/base.tsx @@ -1,5 +1,5 @@ // Type-only import: erased at compile time, does not pull the full echarts -// bundle in. echarts-for-react's API is typed against this `ECharts` class. +// bundle in. The `ECharts` type is used for the stored ECharts instance ref. import type { ECharts } from "echarts"; import { BarChart, @@ -50,6 +50,14 @@ import type { // toolbox, markLine), register it here. Consumers passing custom `options` // that need extra features can register them in their own app via // `import { use } from "echarts/core"`. +// +// Note: tooltip-driven axisPointer is bundled with `TooltipComponent`, so no +// explicit AxisPointerComponent registration is needed. +// +// This `use()` call must stay co-located in the same module as `BaseChart`: +// `package.json#sideEffects` declares JS modules side-effect free, so moving +// registration to a separate import-for-side-effect module would let bundlers +// drop it during tree-shaking. echarts.use([ // Series types used by the option builders LineChart, // line + area charts (area = line with areaStyle) @@ -140,7 +148,17 @@ export interface BaseChartProps { min?: number; /** Max value for heatmap color scale */ max?: number; - /** Additional ECharts options to merge */ + /** + * Additional ECharts options to merge. + * + * Only the built-in feature set is registered by this package, so options + * referencing extra ECharts features (`dataZoom`, `toolbox`, `markLine`, + * `markArea`, `graphic`, `dataset`, top-level `axisPointer`, ...) require + * registering them in your app via `import { use } from "echarts/core"`. + * This only works when your `echarts` resolves to the same module + * instance/version as this package's (the registry is a singleton; + * duplicate echarts copies won't share registrations). + */ options?: Record; /** Additional CSS classes */ className?: string;