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..140502f43 --- /dev/null +++ b/packages/appkit-ui/src/react/charts/__tests__/base.test.tsx @@ -0,0 +1,235 @@ +/** + * 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([]); + }); + + 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 6a623eb4e..99a931d28 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. The `ECharts` type is used for the stored ECharts instance ref. 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,46 @@ 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"`. +// +// 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) + 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 // ============================================================================ @@ -88,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; @@ -137,7 +207,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 +351,9 @@ export function BaseChart({ } return ( -