You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/30 04:22:09 UTC
(superset) branch fix/getxaxisdomain-non-numeric-temporal created (now 9933360da55)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch fix/getxaxisdomain-non-numeric-temporal
in repository https://gitbox.apache.org/repos/asf/superset.git
at 9933360da55 fix(echarts): recognize Date and ISO-string temporal x-axis values in getXAxisDomain
This branch includes the following new commits:
new 9933360da55 fix(echarts): recognize Date and ISO-string temporal x-axis values in getXAxisDomain
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(echarts): recognize Date and ISO-string temporal x-axis values in getXAxisDomain
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/getxaxisdomain-non-numeric-temporal
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9933360da55388f855fa986bc0a89aab9e5f601e
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Tue Sep 29 21:21:54 2026 -0700
fix(echarts): recognize Date and ISO-string temporal x-axis values in getXAxisDomain
getXAxisDomain (used by getGrainBarMaxWidth and the x-axis label spacing
formatter) only recognized `typeof value === 'number'` x-axis values. A
temporal column whose rows are Date objects or ISO strings instead of raw
epoch numbers found no domain bounds at all, so the grain-aware bar-width
cap from #44628 silently fell back to the old flat 100px cap, and the
label-spacing formatter lost its collision-avoidance data.
Extracts the number/Date/string coercion `getTemporalTickValues` already
used (added by #43669) into a shared `coerceTemporalMs` helper in
formatters.ts, and uses it in getXAxisDomain instead of a bare `typeof`
check. getTemporalTickValues now calls the same shared helper instead of
carrying its own copy of the same logic.
Follow-up to the codeant findings on #44628 (getXAxisDomain/
getGrainBarMaxWidth numeric-only handling), which predate that PR and
originate in #43669.
Co-Authored-By: Evan Rusackas <ev...@rusackas.com>
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../plugin-chart-echarts/src/utils/formatters.ts | 40 ++++++-
.../plugin-chart-echarts/src/utils/series.ts | 28 +----
.../test/utils/formatters.test.ts | 118 +++++++++++++++++++++
.../plugin-chart-echarts/test/utils/series.test.ts | 96 ++++++++++++++++-
4 files changed, 253 insertions(+), 29 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
index 264a35ba994..e7080c3c65e 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
@@ -349,6 +349,42 @@ export function createSpacedXAxisFormatter(
return wrapper;
}
+// `new Date('2024-04-06')` parses as UTC, but ECharts' own date parser treats
+// zone-less strings as local time — mismatch would offset the pinned tick.
+const DATE_ONLY_RE = /^(\d{4})(?:-(\d{1,2})(?:-(\d{1,2}))?)?$/;
+
+function parseTemporalString(value: string): number {
+ const dateOnly = DATE_ONLY_RE.exec(value);
+ if (dateOnly) {
+ const [, year, month, day] = dateOnly;
+ return new Date(
+ Number(year),
+ Number(month || 1) - 1,
+ Number(day || 1),
+ ).getTime();
+ }
+ return new Date(value).getTime();
+}
+
+/**
+ * Coerces a data record's raw x-axis value to epoch milliseconds. A temporal
+ * column can carry any of the shapes the query/data pipeline produces for it
+ * (a raw epoch number, a `Date` from client-side parsing, or an ISO string
+ * straight off the wire) — every caller comparing or bucketing temporal
+ * values needs to recognize all three the same way, so this is the single
+ * place that does it. Returns `NaN` for a value that isn't temporal at all
+ * (nullish, an unparseable string, ...); callers filter with
+ * `Number.isFinite`.
+ */
+export function coerceTemporalMs(value: unknown): number {
+ // eslint-disable-next-line no-nested-ternary
+ return value instanceof Date
+ ? value.getTime()
+ : typeof value === 'string'
+ ? parseTemporalString(value)
+ : Number(value ?? NaN);
+}
+
/**
* Computes the [min, max] of a temporal x-axis column across one or more
* data record arrays, for use with createSpacedXAxisFormatter.
@@ -361,8 +397,8 @@ export function getXAxisDomain(
let domainMax: number | undefined;
dataRecordArrays.forEach(records => {
records.forEach(record => {
- const value = record[xAxisCol];
- if (typeof value === 'number') {
+ const value = coerceTemporalMs(record[xAxisCol]);
+ if (Number.isFinite(value)) {
if (domainMin === undefined || value < domainMin) domainMin = value;
if (domainMax === undefined || value > domainMax) domainMax = value;
}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index fa83470fa1e..58fd0804dda 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -55,7 +55,7 @@ import {
StackType,
} from '../types';
import { defaultLegendPadding } from '../defaults';
-import { getXAxisDomain } from './formatters';
+import { coerceTemporalMs, getXAxisDomain } from './formatters';
function isDefined<T>(value: T | undefined | null): boolean {
return value !== undefined && value !== null;
@@ -1059,23 +1059,6 @@ export function getAxisType(
return AxisType.Category;
}
-// `new Date('2024-04-06')` parses as UTC, but ECharts' own date parser treats
-// zone-less strings as local time — mismatch would offset the pinned tick.
-const DATE_ONLY_RE = /^(\d{4})(?:-(\d{1,2})(?:-(\d{1,2}))?)?$/;
-
-function parseTemporalString(value: string): number {
- const dateOnly = DATE_ONLY_RE.exec(value);
- if (dateOnly) {
- const [, year, month, day] = dateOnly;
- return new Date(
- Number(year),
- Number(month || 1) - 1,
- Number(day || 1),
- ).getTime();
- }
- return new Date(value).getTime();
-}
-
/**
* Bucket timestamps a temporal axis should tick on, or undefined to let ECharts
* choose.
@@ -1100,14 +1083,7 @@ export function getTemporalTickValues(
}
const values = new Set<number>();
data.forEach(row => {
- const value = row[xAxisLabel];
- const timestamp =
- // eslint-disable-next-line no-nested-ternary
- value instanceof Date
- ? value.getTime()
- : typeof value === 'string'
- ? parseTemporalString(value)
- : Number(value ?? NaN);
+ const timestamp = coerceTemporalMs(row[xAxisLabel]);
if (Number.isFinite(timestamp)) {
values.add(timestamp);
}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts
index 9efde35ad5a..b424f77d023 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts
@@ -27,9 +27,11 @@ import {
TimeGranularity,
} from '@superset-ui/core';
import {
+ coerceTemporalMs,
createSpacedXAxisFormatter,
getPercentFormatter,
getTooltipTimeFormatter,
+ getXAxisDomain,
getXAxisFormatter,
} from '../../src/utils/formatters';
@@ -386,3 +388,119 @@ test('createSpacedXAxisFormatter still dedupes identical consecutive labels when
expect(labels).toEqual(['Jan', '', '']);
});
+
+describe('coerceTemporalMs', () => {
+ test('passes a number through unchanged', () => {
+ expect(coerceTemporalMs(1712361600000)).toBe(1712361600000);
+ });
+
+ test('reads a Date object via getTime', () => {
+ const date = new Date(Date.UTC(2024, 3, 6));
+ expect(coerceTemporalMs(date)).toBe(date.getTime());
+ });
+
+ test('parses a zoned ISO string as the instant it names', () => {
+ expect(coerceTemporalMs('2026-04-06T00:00:00.000Z')).toBe(
+ Date.UTC(2026, 3, 6),
+ );
+ });
+
+ test('parses a zone-less datetime string as local time, matching ECharts', () => {
+ expect(coerceTemporalMs('2026-04-06T00:00:00')).toBe(
+ new Date(2026, 3, 6, 0, 0, 0).getTime(),
+ );
+ });
+
+ test('parses a bare date string as local midnight, matching ECharts rather than native Date', () => {
+ // `new Date('2026-04-06')` is UTC, but ECharts parses it as local time.
+ // jest.config.js fixes the test TZ to America/New_York, so they disagree.
+ const localMidnight = new Date(2026, 3, 6).getTime();
+ expect(localMidnight).not.toEqual(new Date('2026-04-06').getTime());
+ expect(coerceTemporalMs('2026-04-06')).toBe(localMidnight);
+ });
+
+ test('returns NaN for an unparseable string or a nullish value', () => {
+ expect(coerceTemporalMs('not-a-date')).toBeNaN();
+ expect(coerceTemporalMs(null)).toBeNaN();
+ expect(coerceTemporalMs(undefined)).toBeNaN();
+ });
+});
+
+describe('getXAxisDomain', () => {
+ const xAxisCol = '__timestamp';
+ const t0 = Date.UTC(2024, 0, 1);
+ const t1 = Date.UTC(2024, 0, 2);
+
+ test('finds the min/max of a purely numeric column', () => {
+ expect(
+ getXAxisDomain([[{ [xAxisCol]: t0 }, { [xAxisCol]: t1 }]], xAxisCol),
+ ).toEqual([t0, t1]);
+ });
+
+ test('finds the min/max when every value is a Date object', () => {
+ // Regression: getXAxisDomain used to only recognize `typeof === 'number'`,
+ // so a column of Date objects (as MixedTimeseries/Timeseries transformProps
+ // pass through in some data-fetch paths) found no bounds at all and the
+ // grain-aware bar-width cap silently fell back to the flat 100px default.
+ expect(
+ getXAxisDomain(
+ [[{ [xAxisCol]: new Date(t0) }, { [xAxisCol]: new Date(t1) }]],
+ xAxisCol,
+ ),
+ ).toEqual([t0, t1]);
+ });
+
+ test('finds the min/max when every value is an ISO string', () => {
+ expect(
+ getXAxisDomain(
+ [
+ [
+ { [xAxisCol]: '2024-01-01T00:00:00.000Z' },
+ { [xAxisCol]: '2024-01-02T00:00:00.000Z' },
+ ],
+ ],
+ xAxisCol,
+ ),
+ ).toEqual([t0, t1]);
+ });
+
+ test('finds the min/max across a mix of numbers, Dates and ISO strings', () => {
+ // The two query result sets MixedTimeseries combines don't have to agree
+ // on representation; every row must still count toward the same domain.
+ expect(
+ getXAxisDomain(
+ [
+ [{ [xAxisCol]: t0 }],
+ [
+ { [xAxisCol]: new Date(t1) },
+ { [xAxisCol]: '2024-01-01T12:00:00.000Z' },
+ ],
+ ],
+ xAxisCol,
+ ),
+ ).toEqual([t0, t1]);
+ });
+
+ test('ignores unparseable or nullish values without affecting the real bounds', () => {
+ expect(
+ getXAxisDomain(
+ [
+ [
+ { [xAxisCol]: t0 },
+ { [xAxisCol]: 'not-a-date' },
+ { [xAxisCol]: null },
+ { [xAxisCol]: t1 },
+ ],
+ ],
+ xAxisCol,
+ ),
+ ).toEqual([t0, t1]);
+ });
+
+ test('returns [undefined, undefined] when the column has no temporal values', () => {
+ expect(getXAxisDomain([[{ other: 1 }]], xAxisCol)).toEqual([
+ undefined,
+ undefined,
+ ]);
+ });
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index c5b0efc4fa3..bbf1c56dbb7 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -41,6 +41,7 @@ import {
getLegendProps,
getLegendScrollDataIndex,
getOverMaxHiddenFormatter,
+ getGrainBarMaxWidth,
getMinAndMaxFromBounds,
capTickMarks,
getTemporalTickValues,
@@ -56,7 +57,12 @@ import {
LegendType,
} from '../../src/types';
import { defaultLegendPadding } from '../../src/defaults';
-import { NULL_STRING, StackControlsValue } from '../../src/constants';
+import {
+ NULL_STRING,
+ ONE_DAY_MS,
+ StackControlsValue,
+ TIMEGRAIN_TO_TIMESTAMP,
+} from '../../src/constants';
const {
getHorizontalLegendAvailableWidth,
@@ -1936,6 +1942,94 @@ describe('getTemporalTickValues', () => {
});
});
+describe('getGrainBarMaxWidth', () => {
+ const xAxisCol = '__timestamp';
+ const plotLengthPx = 600;
+
+ test('returns undefined for a non-time axis', () => {
+ expect(
+ getGrainBarMaxWidth(
+ AxisType.Category,
+ TimeGranularity.HOUR,
+ [[{ [xAxisCol]: 0 }]],
+ xAxisCol,
+ plotLengthPx,
+ ),
+ ).toBeUndefined();
+ });
+
+ test('returns undefined when there is no resolved time grain', () => {
+ expect(
+ getGrainBarMaxWidth(
+ AxisType.Time,
+ undefined,
+ [[{ [xAxisCol]: 0 }]],
+ xAxisCol,
+ plotLengthPx,
+ ),
+ ).toBeUndefined();
+ });
+
+ test('computes the same grain-aware width whether the x column is numbers, Dates or ISO strings', () => {
+ // Regression: getGrainBarMaxWidth delegates to getXAxisDomain, which used
+ // to only recognize `typeof === 'number'`. A Date- or ISO-string-valued
+ // temporal column found no domain bounds and this returned undefined,
+ // silently falling back to the flat 100px sparse-bar cap instead of the
+ // grain-aware one.
+ const hour = TIMEGRAIN_TO_TIMESTAMP[TimeGranularity.HOUR];
+ const t0 = Date.UTC(2024, 0, 1, 0);
+ const t1 = Date.UTC(2024, 0, 1, 3);
+ const expected = (hour / (t1 - t0)) * plotLengthPx;
+
+ const numeric = getGrainBarMaxWidth(
+ AxisType.Time,
+ TimeGranularity.HOUR,
+ [[{ [xAxisCol]: t0 }, { [xAxisCol]: t1 }]],
+ xAxisCol,
+ plotLengthPx,
+ );
+ const dates = getGrainBarMaxWidth(
+ AxisType.Time,
+ TimeGranularity.HOUR,
+ [[{ [xAxisCol]: new Date(t0) }, { [xAxisCol]: new Date(t1) }]],
+ xAxisCol,
+ plotLengthPx,
+ );
+ const isoStrings = getGrainBarMaxWidth(
+ AxisType.Time,
+ TimeGranularity.HOUR,
+ [
+ [
+ { [xAxisCol]: '2024-01-01T00:00:00.000Z' },
+ { [xAxisCol]: '2024-01-01T03:00:00.000Z' },
+ ],
+ ],
+ xAxisCol,
+ plotLengthPx,
+ );
+
+ expect(numeric).toBeCloseTo(expected);
+ expect(dates).toBeCloseTo(expected);
+ expect(isoStrings).toBeCloseTo(expected);
+ });
+
+ test('falls back to the 2-day degenerate-domain span for a single distinct Date value', () => {
+ const hour = TIMEGRAIN_TO_TIMESTAMP[TimeGranularity.HOUR];
+ const t0 = new Date(Date.UTC(2024, 0, 1));
+ const expected = (hour / (2 * ONE_DAY_MS)) * plotLengthPx;
+
+ expect(
+ getGrainBarMaxWidth(
+ AxisType.Time,
+ TimeGranularity.HOUR,
+ [[{ [xAxisCol]: t0 }, { [xAxisCol]: new Date(t0.getTime()) }]],
+ xAxisCol,
+ plotLengthPx,
+ ),
+ ).toBeCloseTo(expected);
+ });
+});
+
describe('capTickMarks', () => {
test('returns values unchanged when within the cap', () => {
const values = [1, 2, 3];