Skip to content

Commit b0fdd19

Browse files
MarioCadenasMarioCadenas
authored
perf(appkit-ui): enable tree-shaking with sideEffects flag and modular echarts imports (#442)
* 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 <MarioCadenas@users.noreply.github.com> * 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 <MarioCadenas@users.noreply.github.com> * fix(appkit-ui): import echarts-for-react ESM core for correct default export echarts-for-react ships no exports map, so the /lib/core subpath resolves to the CommonJS build whose default export is module.exports. Under ESM default-import interop a consumer's bundler hands back the whole { default, __esModule } object rather than the component, so BaseChart rendered an object and React threw "Element type is invalid". Import echarts-for-react/esm/core (real ESM export default) instead, which keeps the modular tree-shakeable core and fixes interop. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> * perf(shared): declare package side-effect-free except CLI for tree-shaking The "." barrel re-exports several modules; without a sideEffects field a consumer importing a single symbol (e.g. sql from appkit-ui) forces bundlers to retain every sibling module and its runtime deps. Declare sideEffects: ["**/cli/**"] so bundlers can drop unused barrel modules, while preserving the CLI's import "dotenv/config" side effect (the . barrel never reaches ./cli). Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> --------- Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> Co-authored-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
1 parent 74b5b02 commit b0fdd19

4 files changed

Lines changed: 322 additions & 4 deletions

File tree

‎packages/appkit-ui/package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
"type": "module",
44
"version": "0.43.0",
55
"license": "Apache-2.0",
6+
"sideEffects": [
7+
"**/*.css"
8+
],
69
"repository": {
710
"type": "git",
811
"url": "git+https://github.com/databricks/appkit.git"

‎packages/appkit-ui/src/react/charts/__tests__/base.test.tsx‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
/**
2+
* Mount tests for BaseChart with the modular ECharts build.
3+
*
4+
* `base.tsx` imports from `echarts/core` and registers only the chart types,
5+
* components, and renderer that the option builders use. ECharts does NOT
6+
* throw when a series type or component is missing from the registration —
7+
* it logs an error like "Series heatmap is used but not imported" and renders
8+
* nothing. These tests mount every chart family and assert no such error is
9+
* emitted, so a missing registration fails the suite instead of silently
10+
* producing blank charts.
11+
*/
12+
import { cleanup, render, waitFor } from "@testing-library/react";
13+
import {
14+
afterEach,
15+
beforeAll,
16+
beforeEach,
17+
describe,
18+
expect,
19+
test,
20+
vi,
21+
} from "vitest";
22+
import { BaseChart } from "../base";
23+
import type { ChartType } from "../types";
24+
25+
// ----------------------------------------------------------------------------
26+
// jsdom canvas stub: ECharts' CanvasRenderer needs a 2D context, which jsdom
27+
// does not implement. A Proxy that no-ops every method (and returns a metrics
28+
// object for measureText) is enough for ECharts to lay out and "paint".
29+
// ----------------------------------------------------------------------------
30+
beforeAll(() => {
31+
// jsdom doesn't implement window.matchMedia, which the chart theme hook
32+
// reads to track color-scheme changes.
33+
if (!window.matchMedia) {
34+
Object.defineProperty(window, "matchMedia", {
35+
writable: true,
36+
value: (query: string) => ({
37+
matches: false,
38+
media: query,
39+
onchange: null,
40+
addEventListener: vi.fn(),
41+
removeEventListener: vi.fn(),
42+
addListener: vi.fn(),
43+
removeListener: vi.fn(),
44+
dispatchEvent: vi.fn(),
45+
}),
46+
});
47+
}
48+
49+
const contextStub = new Proxy(
50+
{},
51+
{
52+
get(target: Record<string, unknown>, prop: string) {
53+
if (prop === "measureText") {
54+
return () => ({ width: 10 });
55+
}
56+
if (
57+
prop === "createLinearGradient" ||
58+
prop === "createRadialGradient"
59+
) {
60+
return () => ({ addColorStop: () => {} });
61+
}
62+
if (!(prop in target)) {
63+
target[prop] = vi.fn();
64+
}
65+
return target[prop];
66+
},
67+
set() {
68+
return true;
69+
},
70+
},
71+
);
72+
73+
Object.defineProperty(HTMLCanvasElement.prototype, "getContext", {
74+
writable: true,
75+
value: () => contextStub,
76+
});
77+
});
78+
79+
const registrationErrors: string[] = [];
80+
81+
function captureRegistrationIssues(...args: unknown[]) {
82+
const message = args.map(String).join(" ");
83+
// Matches ECharts messages such as:
84+
// "Series heatmap is used but not imported."
85+
// "Component visualMap is used but not imported."
86+
// "Renderer 'canvas' is not imported."
87+
// "Specified `grid.containLabel` but no `use(LegacyGridContainLabel)`"
88+
if (/not (imported|exists|registered)|no `?use\(/i.test(message)) {
89+
registrationErrors.push(message);
90+
}
91+
}
92+
93+
beforeEach(() => {
94+
registrationErrors.length = 0;
95+
vi.spyOn(console, "error").mockImplementation(captureRegistrationIssues);
96+
vi.spyOn(console, "warn").mockImplementation(captureRegistrationIssues);
97+
});
98+
99+
afterEach(() => {
100+
cleanup();
101+
vi.restoreAllMocks();
102+
});
103+
104+
const cartesianData = [
105+
{ month: "Jan", revenue: 100, cost: 60 },
106+
{ month: "Feb", revenue: 120, cost: 70 },
107+
{ month: "Mar", revenue: 90, cost: 50 },
108+
];
109+
110+
const heatmapData = [
111+
{ day: "Mon", hour: "9am", value: 3 },
112+
{ day: "Mon", hour: "10am", value: 7 },
113+
{ day: "Tue", hour: "9am", value: 5 },
114+
{ day: "Tue", hour: "10am", value: 1 },
115+
];
116+
117+
describe("BaseChart ECharts registration", () => {
118+
const cartesianTypes: ChartType[] = ["line", "area", "bar", "scatter"];
119+
120+
test.each(cartesianTypes)(
121+
"renders %s chart without missing-registration errors",
122+
async (chartType) => {
123+
const { container } = render(
124+
<BaseChart
125+
data={cartesianData}
126+
chartType={chartType}
127+
xKey="month"
128+
yKey={["revenue", "cost"]}
129+
title="Test"
130+
/>,
131+
);
132+
133+
await waitFor(() =>
134+
expect(container.querySelector("canvas")).not.toBeNull(),
135+
);
136+
expect(registrationErrors).toEqual([]);
137+
},
138+
);
139+
140+
test("renders horizontal bar chart without missing-registration errors", async () => {
141+
const { container } = render(
142+
<BaseChart
143+
data={cartesianData}
144+
chartType="bar"
145+
xKey="month"
146+
yKey="revenue"
147+
orientation="horizontal"
148+
/>,
149+
);
150+
151+
await waitFor(() =>
152+
expect(container.querySelector("canvas")).not.toBeNull(),
153+
);
154+
expect(registrationErrors).toEqual([]);
155+
});
156+
157+
test.each(["pie", "donut"] as ChartType[])(
158+
"renders %s chart without missing-registration errors",
159+
async (chartType) => {
160+
const { container } = render(
161+
<BaseChart
162+
data={cartesianData}
163+
chartType={chartType}
164+
xKey="month"
165+
yKey="revenue"
166+
/>,
167+
);
168+
169+
await waitFor(() =>
170+
expect(container.querySelector("canvas")).not.toBeNull(),
171+
);
172+
expect(registrationErrors).toEqual([]);
173+
},
174+
);
175+
176+
test("renders radar chart without missing-registration errors", async () => {
177+
const { container } = render(
178+
<BaseChart
179+
data={cartesianData}
180+
chartType="radar"
181+
xKey="month"
182+
yKey={["revenue", "cost"]}
183+
/>,
184+
);
185+
186+
await waitFor(() =>
187+
expect(container.querySelector("canvas")).not.toBeNull(),
188+
);
189+
expect(registrationErrors).toEqual([]);
190+
});
191+
192+
test("renders heatmap (visualMap component) without missing-registration errors", async () => {
193+
const { container } = render(
194+
<BaseChart
195+
data={heatmapData}
196+
chartType="heatmap"
197+
xKey="day"
198+
yAxisKey="hour"
199+
yKey="value"
200+
/>,
201+
);
202+
203+
await waitFor(() =>
204+
expect(container.querySelector("canvas")).not.toBeNull(),
205+
);
206+
expect(registrationErrors).toEqual([]);
207+
});
208+
209+
test("renders custom `options` using a registered component without registration errors", async () => {
210+
const { container } = render(
211+
<BaseChart
212+
data={cartesianData}
213+
chartType="line"
214+
xKey="month"
215+
yKey="revenue"
216+
options={{ title: { subtext: "custom" } }}
217+
/>,
218+
);
219+
220+
await waitFor(() =>
221+
expect(container.querySelector("canvas")).not.toBeNull(),
222+
);
223+
expect(registrationErrors).toEqual([]);
224+
});
225+
226+
test("renders the no-data fallback for empty data without mounting ECharts", () => {
227+
const { container, getByText } = render(
228+
<BaseChart data={[]} chartType="line" />,
229+
);
230+
231+
expect(getByText("No data")).toBeTruthy();
232+
expect(container.querySelector("canvas")).toBeNull();
233+
expect(registrationErrors).toEqual([]);
234+
});
235+
});

‎packages/appkit-ui/src/react/charts/base.tsx‎

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,31 @@
1+
// Type-only import: erased at compile time, does not pull the full echarts
2+
// bundle in. The `ECharts` type is used for the stored ECharts instance ref.
13
import type { ECharts } from "echarts";
2-
import ReactECharts from "echarts-for-react";
4+
import {
5+
BarChart,
6+
HeatmapChart,
7+
LineChart,
8+
PieChart,
9+
RadarChart,
10+
ScatterChart,
11+
} from "echarts/charts";
12+
import {
13+
GridComponent,
14+
LegendComponent,
15+
TitleComponent,
16+
TooltipComponent,
17+
VisualMapComponent,
18+
} from "echarts/components";
19+
import * as echarts from "echarts/core";
20+
import { LegacyGridContainLabel } from "echarts/features";
21+
import { CanvasRenderer } from "echarts/renderers";
22+
// Import the ESM core build, not `echarts-for-react/lib/core` (CJS). The package
23+
// has no `exports` map, so `/lib/core` resolves to the literal CommonJS file
24+
// whose default export is `module.exports`; under ESM default-import interop a
25+
// consumer's bundler hands back the whole `{ default, __esModule }` object rather
26+
// than the component, so `<ReactEChartsCore/>` renders an object and React throws
27+
// "Element type is invalid". `esm/core` is real ESM with a true `export default`.
28+
import ReactEChartsCore from "echarts-for-react/esm/core";
329
import { useCallback, useMemo, useRef } from "react";
430
import { normalizeChartData, normalizeHeatmapData } from "./normalize";
531
import {
@@ -18,6 +44,46 @@ import type {
1844
Orientation,
1945
} from "./types";
2046

47+
// ============================================================================
48+
// ECharts Registration (modular imports for tree-shaking)
49+
// ============================================================================
50+
// Only the chart types and components used by the option builders in
51+
// `options.ts` are registered. Importing from `echarts/core` instead of the
52+
// full `echarts` entry keeps unused chart types (graph, sankey, gauge, ...)
53+
// out of consumer bundles.
54+
//
55+
// If you add a new chart type or use a new ECharts feature (e.g. dataZoom,
56+
// toolbox, markLine), register it here. Consumers passing custom `options`
57+
// that need extra features can register them in their own app via
58+
// `import { use } from "echarts/core"`.
59+
//
60+
// Note: tooltip-driven axisPointer is bundled with `TooltipComponent`, so no
61+
// explicit AxisPointerComponent registration is needed.
62+
//
63+
// This `use()` call must stay co-located in the same module as `BaseChart`:
64+
// `package.json#sideEffects` declares JS modules side-effect free, so moving
65+
// registration to a separate import-for-side-effect module would let bundlers
66+
// drop it during tree-shaking.
67+
echarts.use([
68+
// Series types used by the option builders
69+
LineChart, // line + area charts (area = line with areaStyle)
70+
BarChart, // bar + horizontal bar charts
71+
PieChart, // pie + donut charts
72+
ScatterChart,
73+
HeatmapChart,
74+
RadarChart,
75+
// Components referenced by built options
76+
TitleComponent, // `title`
77+
TooltipComponent, // `tooltip`
78+
LegendComponent, // `legend`
79+
GridComponent, // `grid` / `xAxis` / `yAxis`
80+
VisualMapComponent, // `visualMap` (heatmap color scale)
81+
// Features
82+
LegacyGridContainLabel, // `grid.containLabel` (cartesian option builder)
83+
// Renderer (BaseChart always renders with `renderer: "canvas"`)
84+
CanvasRenderer,
85+
]);
86+
2187
// ============================================================================
2288
// Palette Selection
2389
// ============================================================================
@@ -88,7 +154,17 @@ export interface BaseChartProps {
88154
min?: number;
89155
/** Max value for heatmap color scale */
90156
max?: number;
91-
/** Additional ECharts options to merge */
157+
/**
158+
* Additional ECharts options to merge.
159+
*
160+
* Only the built-in feature set is registered by this package, so options
161+
* referencing extra ECharts features (`dataZoom`, `toolbox`, `markLine`,
162+
* `markArea`, `graphic`, `dataset`, top-level `axisPointer`, ...) require
163+
* registering them in your app via `import { use } from "echarts/core"`.
164+
* This only works when your `echarts` resolves to the same module
165+
* instance/version as this package's (the registry is a singleton;
166+
* duplicate echarts copies won't share registrations).
167+
*/
92168
options?: Record<string, unknown>;
93169
/** Additional CSS classes */
94170
className?: string;
@@ -139,7 +215,7 @@ export function BaseChart({
139215

140216
// Callback ref pattern: captures the ECharts instance when ReactECharts mounts
141217
// This ensures we always have a stable reference to the actual instance
142-
const chartRefCallback = useCallback((node: ReactECharts | null) => {
218+
const chartRefCallback = useCallback((node: ReactEChartsCore | null) => {
143219
// Dispose previous instance if component is being replaced
144220
if (
145221
echartsInstanceRef.current &&
@@ -285,8 +361,9 @@ export function BaseChart({
285361
}
286362

287363
return (
288-
<ReactECharts
364+
<ReactEChartsCore
289365
ref={chartRefCallback}
366+
echarts={echarts}
290367
option={option}
291368
style={{ height }}
292369
className={className}

‎packages/shared/package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
"type": "module",
44
"version": "0.0.1",
55
"private": true,
6+
"sideEffects": [
7+
"**/cli/**"
8+
],
69
"exports": {
710
".": {
811
"development": "./src/index.ts",

0 commit comments

Comments
 (0)