Skip to content

Commit ae62c7f

Browse files
committed
refactor(engine): extract pure helpers from compile.ts into compile/ subfolder
Pulls three side-effect-light computations out of compileChart into their own files in packages/engine/src/compile/: - data-clip.ts: filterClippedDomains - drops data rows outside clipped scale domains on the x/y channels. - color-scale-range.ts: applyColorScaleRange - applies the theme palette to the color scale range when the encoding declares no explicit range. Still mutates the scale in place; documented at the callsite. - watermark-obstacle.ts: computeWatermarkObstacle - computes the rect the brand watermark occupies so annotations can avoid it. The orchestration ordering in compile.ts is unchanged. A comment block explicitly names the three load-bearing invariants (double legend pass, in-place computeGridlines mutation, post-hoc defaultColor assignment) so future edits don't silently regress layout. The compile-snapshot oracle (previous commit) matches exactly after the extraction, with no snapshot updates required.
1 parent 60f9a04 commit ae62c7f

7 files changed

Lines changed: 368 additions & 49 deletions

File tree

packages/engine/src/compile.ts

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import type {
3232
} from '@opendata-ai/openchart-core';
3333
import {
3434
adaptTheme,
35-
BRAND_RESERVE_WIDTH,
3635
computeLabelBounds,
3736
generateAltText,
3837
generateDataTable,
@@ -57,6 +56,9 @@ import { ruleRenderer } from './charts/rule';
5756
import { scatterRenderer } from './charts/scatter';
5857
import { textRenderer } from './charts/text';
5958
import { tickRenderer } from './charts/tick';
59+
import { applyColorScaleRange } from './compile/color-scale-range';
60+
import { filterClippedDomains } from './compile/data-clip';
61+
import { computeWatermarkObstacle } from './compile/watermark-obstacle';
6062
import { compile as compileSpec, flattenLayers } from './compiler/index';
6163

6264
// Register all built-in chart renderers under the new Vega-Lite mark type names.
@@ -307,6 +309,11 @@ export function compileChart(spec: unknown, options: CompileOptions): ChartLayou
307309
theme = adaptTheme(theme);
308310
}
309311

312+
// ORCHESTRATION INVARIANTS — do not reorder without care:
313+
// 1. Legend is computed twice (preliminary + refined) to break a dims/legend dependency cycle.
314+
// 2. computeGridlines mutates `axes` in place.
315+
// 3. scales.defaultColor is set post-computeScales because the resolution needs theme context.
316+
310317
// Compute legend first (needs to reserve space)
311318
const preliminaryArea: Rect = {
312319
x: 0,
@@ -358,19 +365,7 @@ export function compileChart(spec: unknown, options: CompileOptions): ChartLayou
358365
}
359366

360367
// Filter clipped scale domains: when scale.clip is true, exclude rows outside the domain
361-
for (const channel of ['x', 'y'] as const) {
362-
const enc = chartSpec.encoding[channel];
363-
if (!enc?.scale?.clip || !enc.scale.domain) continue;
364-
const domain = enc.scale.domain;
365-
const field = enc.field;
366-
if (Array.isArray(domain) && domain.length === 2 && typeof domain[0] === 'number') {
367-
const [lo, hi] = domain as [number, number];
368-
renderData = renderData.filter((row) => {
369-
const v = Number(row[field]);
370-
return Number.isFinite(v) && v >= lo && v <= hi;
371-
});
372-
}
373-
}
368+
renderData = filterClippedDomains(renderData, chartSpec.encoding);
374369

375370
// Build a filtered spec for scales and marks, keeping all other properties intact
376371
const renderSpec = renderData !== chartSpec.data ? { ...chartSpec, data: renderData } : chartSpec;
@@ -379,29 +374,7 @@ export function compileChart(spec: unknown, options: CompileOptions): ChartLayou
379374
const scales = computeScales(renderSpec, chartArea, renderSpec.data);
380375

381376
// Update color scale to use theme palette (only when user hasn't provided an explicit range)
382-
if (scales.color) {
383-
const hasExplicitRange = !!(
384-
renderSpec.encoding.color &&
385-
'field' in renderSpec.encoding.color &&
386-
(renderSpec.encoding.color.scale?.range as string[] | undefined)?.length
387-
);
388-
if (scales.color.type === 'sequential') {
389-
// Sequential: use first sequential palette (or fall back to categorical endpoints)
390-
if (!hasExplicitRange) {
391-
const seqStops = Object.values(theme.colors.sequential)[0] ?? theme.colors.categorical;
392-
(scales.color.scale as unknown as import('d3-scale').ScaleLinear<string, string>).range([
393-
seqStops[0],
394-
seqStops[seqStops.length - 1],
395-
]);
396-
}
397-
} else {
398-
if (!hasExplicitRange) {
399-
(scales.color.scale as import('d3-scale').ScaleOrdinal<string, string>).range(
400-
theme.colors.categorical,
401-
);
402-
}
403-
}
404-
}
377+
applyColorScaleRange(scales, renderSpec.encoding, theme);
405378

406379
// Set default color for single-series charts. If the user set a fill on the mark def
407380
// (string or gradient), that takes priority over the theme's first categorical color.
@@ -444,18 +417,8 @@ export function compileChart(spec: unknown, options: CompileOptions): ChartLayou
444417
}
445418

446419
// Add brand watermark as an obstacle so annotations avoid overlapping it.
447-
// The brand is right-aligned on the same baseline as the first bottom chrome element,
448-
// offset below the chart area by x-axis extent (tick labels + axis title).
449-
if (watermark) {
450-
const brandPadding = theme.spacing.padding;
451-
const brandX = dims.total.width - brandPadding - BRAND_RESERVE_WIDTH;
452-
const xAxisExtent = axes.x?.label ? 48 : axes.x ? 26 : 0;
453-
const firstBottomChrome = dims.chrome.source ?? dims.chrome.byline ?? dims.chrome.footer;
454-
const brandY = firstBottomChrome
455-
? chartArea.y + chartArea.height + xAxisExtent + firstBottomChrome.y
456-
: chartArea.y + chartArea.height + xAxisExtent + theme.spacing.chartToFooter;
457-
obstacles.push({ x: brandX, y: brandY, width: BRAND_RESERVE_WIDTH, height: 30 });
458-
}
420+
const watermarkRect = computeWatermarkObstacle(dims, watermark, axes, theme);
421+
if (watermarkRect) obstacles.push(watermarkRect);
459422
const annotations: ResolvedAnnotation[] = computeAnnotations(
460423
chartSpec,
461424
scales,
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import type { Encoding, ResolvedTheme } from '@opendata-ai/openchart-core';
2+
import { resolveTheme } from '@opendata-ai/openchart-core';
3+
import { scaleLinear, scaleOrdinal } from 'd3-scale';
4+
import { describe, expect, it } from 'vitest';
5+
import type { ResolvedScales } from '../../layout/scales';
6+
import { applyColorScaleRange } from '../color-scale-range';
7+
8+
const theme: ResolvedTheme = resolveTheme();
9+
10+
describe('applyColorScaleRange', () => {
11+
it('is a no-op when no color scale is present', () => {
12+
const scales: ResolvedScales = {};
13+
const encoding: Encoding = {
14+
x: { field: 'x', type: 'quantitative' },
15+
y: { field: 'y', type: 'quantitative' },
16+
};
17+
expect(() => applyColorScaleRange(scales, encoding, theme)).not.toThrow();
18+
expect(scales.color).toBeUndefined();
19+
});
20+
21+
it('does not overwrite the range when the encoding declares an explicit palette', () => {
22+
// computeScales has already applied the explicit palette to the scale.
23+
// The helper must leave it untouched (not replace it with the theme palette).
24+
const explicit = ['#111111', '#222222', '#333333'];
25+
const ordinal = scaleOrdinal<string, string>().domain(['a', 'b', 'c']).range(explicit);
26+
const scales: ResolvedScales = {
27+
color: { scale: ordinal, type: 'ordinal', channel: 'color' },
28+
};
29+
const encoding: Encoding = {
30+
x: { field: 'x', type: 'nominal' },
31+
y: { field: 'y', type: 'quantitative' },
32+
color: {
33+
field: 'c',
34+
type: 'nominal',
35+
scale: { range: explicit },
36+
},
37+
};
38+
applyColorScaleRange(scales, encoding, theme);
39+
expect(ordinal.range()).toEqual(explicit);
40+
expect(ordinal.range()).not.toEqual(theme.colors.categorical);
41+
});
42+
43+
it('assigns the theme categorical palette when no range is set', () => {
44+
const ordinal = scaleOrdinal<string, string>().domain(['a', 'b', 'c']);
45+
const scales: ResolvedScales = {
46+
color: { scale: ordinal, type: 'ordinal', channel: 'color' },
47+
};
48+
const encoding: Encoding = {
49+
x: { field: 'x', type: 'nominal' },
50+
y: { field: 'y', type: 'quantitative' },
51+
color: { field: 'c', type: 'nominal' },
52+
};
53+
applyColorScaleRange(scales, encoding, theme);
54+
expect(ordinal.range()).toEqual(theme.colors.categorical);
55+
});
56+
57+
it('uses the first sequential palette endpoints for sequential color scales', () => {
58+
const linear = scaleLinear<string, string>().domain([0, 100]);
59+
const scales: ResolvedScales = {
60+
color: {
61+
scale: linear as unknown as ResolvedScales['color'] extends infer T
62+
? T extends { scale: infer S }
63+
? S
64+
: never
65+
: never,
66+
type: 'sequential',
67+
channel: 'color',
68+
} as NonNullable<ResolvedScales['color']>,
69+
};
70+
const encoding: Encoding = {
71+
x: { field: 'x', type: 'quantitative' },
72+
y: { field: 'y', type: 'quantitative' },
73+
color: { field: 'v', type: 'quantitative' },
74+
};
75+
applyColorScaleRange(scales, encoding, theme);
76+
const firstSeq = Object.values(theme.colors.sequential)[0] ?? theme.colors.categorical;
77+
expect(linear.range()).toEqual([firstSeq[0], firstSeq[firstSeq.length - 1]]);
78+
});
79+
});
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import type { Encoding } from '@opendata-ai/openchart-core';
2+
import { describe, expect, it } from 'vitest';
3+
import { filterClippedDomains } from '../data-clip';
4+
5+
describe('filterClippedDomains', () => {
6+
const data = [
7+
{ x: -5, y: 10 },
8+
{ x: 10, y: 20 },
9+
{ x: 25, y: 80 },
10+
{ x: 50, y: 110 },
11+
{ x: 80, y: 200 },
12+
];
13+
14+
it('returns data unchanged when no channel declares scale.clip', () => {
15+
const encoding: Encoding = {
16+
x: { field: 'x', type: 'quantitative' },
17+
y: { field: 'y', type: 'quantitative' },
18+
};
19+
const result = filterClippedDomains(data, encoding);
20+
expect(result).toBe(data);
21+
expect(result).toHaveLength(5);
22+
});
23+
24+
it('filters rows outside the x-axis clipped domain', () => {
25+
const encoding: Encoding = {
26+
x: {
27+
field: 'x',
28+
type: 'quantitative',
29+
scale: { domain: [0, 30], clip: true },
30+
},
31+
y: { field: 'y', type: 'quantitative' },
32+
};
33+
const result = filterClippedDomains(data, encoding);
34+
expect(result).toEqual([
35+
{ x: 10, y: 20 },
36+
{ x: 25, y: 80 },
37+
]);
38+
});
39+
40+
it('filters rows outside both x- and y-axis clipped domains', () => {
41+
const encoding: Encoding = {
42+
x: {
43+
field: 'x',
44+
type: 'quantitative',
45+
scale: { domain: [0, 60], clip: true },
46+
},
47+
y: {
48+
field: 'y',
49+
type: 'quantitative',
50+
scale: { domain: [0, 100], clip: true },
51+
},
52+
};
53+
const result = filterClippedDomains(data, encoding);
54+
expect(result).toEqual([
55+
{ x: 10, y: 20 },
56+
{ x: 25, y: 80 },
57+
]);
58+
});
59+
});
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import type { AxisLayout, Rect, ResolvedChrome, ResolvedTheme } from '@opendata-ai/openchart-core';
2+
import { BRAND_RESERVE_WIDTH, resolveTheme } from '@opendata-ai/openchart-core';
3+
import { describe, expect, it } from 'vitest';
4+
import type { AxesResult } from '../../layout/axes';
5+
import type { LayoutDimensions } from '../../layout/dimensions';
6+
import { computeWatermarkObstacle } from '../watermark-obstacle';
7+
8+
const theme: ResolvedTheme = resolveTheme();
9+
10+
function makeDims(overrides: Partial<LayoutDimensions> = {}): LayoutDimensions {
11+
const total: Rect = { x: 0, y: 0, width: 800, height: 500 };
12+
const chartArea: Rect = { x: 60, y: 80, width: 700, height: 360 };
13+
const chrome: ResolvedChrome = {
14+
topHeight: 80,
15+
bottomHeight: 40,
16+
} as ResolvedChrome;
17+
return {
18+
total,
19+
chartArea,
20+
chrome,
21+
margins: { top: 80, right: 40, bottom: 40, left: 60 },
22+
theme,
23+
...overrides,
24+
};
25+
}
26+
27+
function makeAxis(label?: string): AxisLayout {
28+
return {
29+
ticks: [],
30+
gridlines: [],
31+
label,
32+
tickLabelStyle: {
33+
fontFamily: theme.fonts.family,
34+
fontSize: 12,
35+
fontWeight: 400,
36+
fill: '#000',
37+
lineHeight: 1.2,
38+
},
39+
start: { x: 0, y: 0 },
40+
end: { x: 100, y: 0 },
41+
};
42+
}
43+
44+
describe('computeWatermarkObstacle', () => {
45+
it('returns null when watermark is disabled', () => {
46+
const dims = makeDims();
47+
const axes: AxesResult = { x: makeAxis(), y: makeAxis() };
48+
expect(computeWatermarkObstacle(dims, false, axes, theme)).toBeNull();
49+
});
50+
51+
it('sits below the x-axis when no bottom chrome is present', () => {
52+
const dims = makeDims();
53+
const axes: AxesResult = { x: makeAxis('value'), y: makeAxis() };
54+
const rect = computeWatermarkObstacle(dims, true, axes, theme);
55+
expect(rect).not.toBeNull();
56+
// Right-aligned: total.width - padding - BRAND_RESERVE_WIDTH
57+
expect(rect!.x).toBe(dims.total.width - theme.spacing.padding - BRAND_RESERVE_WIDTH);
58+
expect(rect!.width).toBe(BRAND_RESERVE_WIDTH);
59+
expect(rect!.height).toBe(30);
60+
// Watermark sits below chart area, offset by x-axis extent + chartToFooter
61+
const expectedY = dims.chartArea.y + dims.chartArea.height + 48 + theme.spacing.chartToFooter;
62+
expect(rect!.y).toBe(expectedY);
63+
});
64+
65+
it('aligns with bottom chrome when a source element is present', () => {
66+
const sourceY = 20;
67+
const dims = makeDims({
68+
chrome: {
69+
topHeight: 80,
70+
bottomHeight: 40,
71+
source: {
72+
text: 'Source: Test',
73+
x: 10,
74+
y: sourceY,
75+
maxWidth: 500,
76+
style: {
77+
fontFamily: theme.fonts.family,
78+
fontSize: 10,
79+
fontWeight: 400,
80+
fill: '#666',
81+
lineHeight: 1.2,
82+
},
83+
},
84+
} as ResolvedChrome,
85+
});
86+
const axes: AxesResult = { x: makeAxis(), y: makeAxis() };
87+
const rect = computeWatermarkObstacle(dims, true, axes, theme);
88+
expect(rect).not.toBeNull();
89+
// With axis present but no label, extent is 26.
90+
// Y is chartArea.y + chartArea.height + 26 + sourceY
91+
expect(rect!.y).toBe(dims.chartArea.y + dims.chartArea.height + 26 + sourceY);
92+
});
93+
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Apply the theme palette as the color scale range when no explicit range was provided.
3+
*
4+
* Sequential scales take the first/last stops of the first sequential palette
5+
* (or the categorical endpoints as a fallback). Categorical scales get the
6+
* full categorical palette. A user-provided `encoding.color.scale.range`
7+
* always wins.
8+
*/
9+
10+
import type { Encoding, ResolvedTheme } from '@opendata-ai/openchart-core';
11+
import type { ScaleLinear, ScaleOrdinal } from 'd3-scale';
12+
import type { ResolvedScales } from '../layout/scales';
13+
14+
/** Mutates `scales.color.scale.range` in place when no explicit palette was set. */
15+
export function applyColorScaleRange(
16+
scales: ResolvedScales,
17+
encoding: Encoding,
18+
theme: ResolvedTheme,
19+
): void {
20+
if (!scales.color) return;
21+
22+
const hasExplicitRange = !!(
23+
encoding.color &&
24+
'field' in encoding.color &&
25+
(encoding.color.scale?.range as string[] | undefined)?.length
26+
);
27+
if (hasExplicitRange) return;
28+
29+
if (scales.color.type === 'sequential') {
30+
const seqStops = Object.values(theme.colors.sequential)[0] ?? theme.colors.categorical;
31+
(scales.color.scale as unknown as ScaleLinear<string, string>).range([
32+
seqStops[0],
33+
seqStops[seqStops.length - 1],
34+
]);
35+
} else {
36+
(scales.color.scale as ScaleOrdinal<string, string>).range(theme.colors.categorical);
37+
}
38+
}

0 commit comments

Comments
 (0)