Copilot commented on code in PR #21739:
URL: https://github.com/apache/echarts/pull/21739#discussion_r3804079527
##########
src/component/tooltip/seriesFormatTooltip.ts:
##########
@@ -142,14 +149,23 @@ function formatTooltipArrayValue(
markerColor: colorStr,
name: dimInfo.displayName,
value: val,
- valueType: dimInfo.type
+ valueType: dimInfo.type,
+ timeZone: getDimensionTimeZone(series, dimInfo)
}));
}
else {
inlineValues.push(val);
inlineValueTypes.push(dimInfo.type);
+ inlineTimeZones.push(getDimensionTimeZone(series, dimInfo));
}
}
- return { inlineValues, inlineValueTypes, blocks };
+ return { inlineValues, inlineValueTypes, inlineTimeZones, blocks };
+}
+
+function getDimensionTimeZone(series: SeriesModel, dimInfo: {coordDim?:
string}): string {
+ const axis = series.coordinateSystem?.getAxis?.(dimInfo.coordDim);
+ return axis && isTimeScale(axis.scale)
+ ? axis.scale.getTimeZone()
+ : series.ecModel.getTimeZone();
}
Review Comment:
`dimInfo.coordDim` is optional, but it’s passed directly into `getAxis`. If
`coordDim` is `undefined`, `getAxis(undefined)` may throw (or return an
unintended axis) depending on the coordinate system implementation, which can
break tooltip formatting. Guard before calling `getAxis` (e.g., only call when
`dimInfo.coordDim != null`), and fall back to `series.ecModel.getTimeZone()`
when there is no coordinate dimension.
##########
src/util/time.ts:
##########
@@ -325,16 +344,53 @@ export function format(
.replace(/{ss}/g, pad(s, 2))
.replace(/{s}/g, s + '')
.replace(/{SSS}/g, pad(S, 3))
- .replace(/{S}/g, S + '');
+ .replace(/{S}/g, S + '')
+ .replace(/{ZZ}/g, ZZ)
+ .replace(/{Z}/g, Z);
+}
+
+function formatTimeZoneOffset(offsetMinutes: number, padded: boolean): string {
+ if (!offsetMinutes) {
+ return 'Z';
+ }
+
+ const sign = offsetMinutes < 0 ? '-' : '+';
+ const absoluteOffset = Math.abs(offsetMinutes);
+ const hours = Math.floor(absoluteOffset / 60);
+ const minutes = absoluteOffset % 60;
+ return sign
+ + (padded ? pad(hours, 2) : hours)
+ + (padded || minutes ? ':' + pad(minutes, 2) : '');
}
Review Comment:
`{Z}`/`{ZZ}` currently render `'Z'` for any zero offset. For non-UTC IANA
zones that happen to be at UTC+00:00 (e.g., `Africa/Abidjan`), this can
misleadingly imply “UTC” rather than “+00:00”. If these tokens are intended to
mean “numeric offset,” consider emitting `+0` / `+00:00` for zero offsets (and
reserving `'Z'` for `timeZone === 'UTC'`), or clarify the token semantics in
documentation/tests.
##########
src/model/Global.ts:
##########
@@ -314,6 +316,18 @@ class GlobalModel extends Model<ECUnitOption> {
opt: InnerSetOptionOpts
): void {
const option = this.option;
+ const timeZone = newOption.timeZone != null
+ ? newOption.timeZone
+ : option.timeZone;
+ if (timeZone != null) {
+ this._timeZone = validateTimeZone(timeZone);
+ }
+ else if (newOption.useUTC != null) {
+ this._timeZone = resolveTimeZone(newOption);
+ }
+ else if (this._timeZone == null) {
+ this._timeZone = resolveTimeZone(option);
+ }
Review Comment:
This logic can’t “unset” a previously configured global `timeZone` via
`setOption({ timeZone: null })` (because `null` is treated as “not provided”
and `option.timeZone` is reused). If the option-merging semantics allow nulling
to clear a setting, you’ll want to distinguish “property absent” from “property
present with null.” A common approach is checking whether `'timeZone' in
newOption` (or `hasOwnProperty`) and treating an explicit `null` as a request
to clear and recompute from `useUTC`/system defaults.
##########
src/util/time.ts:
##########
@@ -409,117 +480,545 @@ export function getUnitFromValue(
}
}
-// export function getUnitValue(
-// value: number | Date,
-// unit: TimeUnit,
-// isUTC: boolean
-// ) : number {
-// const date = zrUtil.isNumber(value)
-// ? numberUtil.parseDate(value)
-// : value;
-// unit = unit || getUnitFromValue(value, isUTC);
-
-// switch (unit) {
-// case 'year':
-// return date[fullYearGetterName(isUTC)]();
-// case 'half-year':
-// return date[monthGetterName(isUTC)]() >= 6 ? 1 : 0;
-// case 'quarter':
-// return Math.floor((date[monthGetterName(isUTC)]() + 1) / 4);
-// case 'month':
-// return date[monthGetterName(isUTC)]();
-// case 'day':
-// return date[dateGetterName(isUTC)]();
-// case 'half-day':
-// return date[hoursGetterName(isUTC)]() / 24;
-// case 'hour':
-// return date[hoursGetterName(isUTC)]();
-// case 'minute':
-// return date[minutesGetterName(isUTC)]();
-// case 'second':
-// return date[secondsGetterName(isUTC)]();
-// case 'millisecond':
-// return date[millisecondsGetterName(isUTC)]();
-// }
-// }
-
/**
* e.g.,
* If timeUnit is 'year', return the Jan 1st 00:00:00 000 of that year.
* If timeUnit is 'day', return the 00:00:00 000 of that day.
*
* @return The input date.
*/
-export function roundTime(date: Date, timeUnit: PrimaryTimeUnit, isUTC:
boolean): Date {
- switch (timeUnit) {
- case 'year':
- date[monthSetterName(isUTC)](0);
- case 'month':
- date[dateSetterName(isUTC)](1);
- case 'day':
- date[hoursSetterName(isUTC)](0);
- case 'hour':
- date[minutesSetterName(isUTC)](0);
- case 'minute':
- date[secondsSetterName(isUTC)](0);
- case 'second':
- date[millisecondsSetterName(isUTC)](0);
+export function roundTime(
+ date: Date,
+ timeUnit: PrimaryTimeUnit,
+ timeZone: string
+): Date;
+/**
+ * @deprecated Pass a time zone string instead of the legacy `isUTC` boolean.
+ */
+export function roundTime(
+ date: Date,
+ timeUnit: PrimaryTimeUnit,
+ isUTC: boolean
+): Date;
+export function roundTime(
+ date: Date,
+ timeUnit: PrimaryTimeUnit,
+ timeZoneOrUTC: string | boolean
+): Date {
+ if (__DEV__ && typeof timeZoneOrUTC === 'boolean') {
+ deprecateReplaceLog('isUTC boolean parameter', 'timeZone string
parameter', 'echarts.time.roundTime');
}
+ date.setTime(roundTimeInTimeZone(
+ date.getTime(), timeUnit, normalizeTimeZone(timeZoneOrUTC)
+ ));
return date;
}
+function normalizeTimeZone(timeZoneOrUTC: string | boolean): string {
+ return typeof timeZoneOrUTC === 'string'
+ ? timeZoneOrUTC
+ : timeZoneOrUTC ? 'UTC' : getSystemTimeZone();
Review Comment:
When `timeZoneOrUTC` is a string, it’s returned without validation. Call
sites like `format(...)`, `roundTime(...)`, and `getUnitFromValue(...)` will
then throw a native `RangeError` from `Intl.DateTimeFormat` on invalid time
zones, leading to inconsistent/unclear error messaging compared to
`validateTimeZone(...)` (“Invalid time zone: …”). Consider
validating/canonicalizing string inputs at the boundary (e.g., via
`validateTimeZone` in this normalization path, or providing an exported
“normalize+validate” helper used by public APIs) so invalid zones consistently
produce the same error.
##########
src/util/time.ts:
##########
@@ -409,117 +480,545 @@ export function getUnitFromValue(
}
}
-// export function getUnitValue(
-// value: number | Date,
-// unit: TimeUnit,
-// isUTC: boolean
-// ) : number {
-// const date = zrUtil.isNumber(value)
-// ? numberUtil.parseDate(value)
-// : value;
-// unit = unit || getUnitFromValue(value, isUTC);
-
-// switch (unit) {
-// case 'year':
-// return date[fullYearGetterName(isUTC)]();
-// case 'half-year':
-// return date[monthGetterName(isUTC)]() >= 6 ? 1 : 0;
-// case 'quarter':
-// return Math.floor((date[monthGetterName(isUTC)]() + 1) / 4);
-// case 'month':
-// return date[monthGetterName(isUTC)]();
-// case 'day':
-// return date[dateGetterName(isUTC)]();
-// case 'half-day':
-// return date[hoursGetterName(isUTC)]() / 24;
-// case 'hour':
-// return date[hoursGetterName(isUTC)]();
-// case 'minute':
-// return date[minutesGetterName(isUTC)]();
-// case 'second':
-// return date[secondsGetterName(isUTC)]();
-// case 'millisecond':
-// return date[millisecondsGetterName(isUTC)]();
-// }
-// }
-
/**
* e.g.,
* If timeUnit is 'year', return the Jan 1st 00:00:00 000 of that year.
* If timeUnit is 'day', return the 00:00:00 000 of that day.
*
* @return The input date.
*/
-export function roundTime(date: Date, timeUnit: PrimaryTimeUnit, isUTC:
boolean): Date {
- switch (timeUnit) {
- case 'year':
- date[monthSetterName(isUTC)](0);
- case 'month':
- date[dateSetterName(isUTC)](1);
- case 'day':
- date[hoursSetterName(isUTC)](0);
- case 'hour':
- date[minutesSetterName(isUTC)](0);
- case 'minute':
- date[secondsSetterName(isUTC)](0);
- case 'second':
- date[millisecondsSetterName(isUTC)](0);
+export function roundTime(
+ date: Date,
+ timeUnit: PrimaryTimeUnit,
+ timeZone: string
+): Date;
+/**
+ * @deprecated Pass a time zone string instead of the legacy `isUTC` boolean.
+ */
+export function roundTime(
+ date: Date,
+ timeUnit: PrimaryTimeUnit,
+ isUTC: boolean
+): Date;
+export function roundTime(
+ date: Date,
+ timeUnit: PrimaryTimeUnit,
+ timeZoneOrUTC: string | boolean
+): Date {
+ if (__DEV__ && typeof timeZoneOrUTC === 'boolean') {
+ deprecateReplaceLog('isUTC boolean parameter', 'timeZone string
parameter', 'echarts.time.roundTime');
}
+ date.setTime(roundTimeInTimeZone(
+ date.getTime(), timeUnit, normalizeTimeZone(timeZoneOrUTC)
+ ));
return date;
}
+function normalizeTimeZone(timeZoneOrUTC: string | boolean): string {
+ return typeof timeZoneOrUTC === 'string'
+ ? timeZoneOrUTC
+ : timeZoneOrUTC ? 'UTC' : getSystemTimeZone();
+}
+
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function fullYearGetterName(isUTC: boolean) {
return isUTC ? 'getUTCFullYear' : 'getFullYear';
}
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function monthGetterName(isUTC: boolean) {
return isUTC ? 'getUTCMonth' : 'getMonth';
}
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function dateGetterName(isUTC: boolean) {
return isUTC ? 'getUTCDate' : 'getDate';
}
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function hoursGetterName(isUTC: boolean) {
return isUTC ? 'getUTCHours' : 'getHours';
}
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function minutesGetterName(isUTC: boolean) {
return isUTC ? 'getUTCMinutes' : 'getMinutes';
}
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function secondsGetterName(isUTC: boolean) {
return isUTC ? 'getUTCSeconds' : 'getSeconds';
}
+/**
+ * @deprecated Use `getTimeZoneParts` to read values in a specific time zone.
+ */
export function millisecondsGetterName(isUTC: boolean) {
return isUTC ? 'getUTCMilliseconds' : 'getMilliseconds';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function fullYearSetterName(isUTC: boolean) {
return isUTC ? 'setUTCFullYear' : 'setFullYear';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function monthSetterName(isUTC: boolean) {
return isUTC ? 'setUTCMonth' : 'setMonth';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function dateSetterName(isUTC: boolean) {
return isUTC ? 'setUTCDate' : 'setDate';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function hoursSetterName(isUTC: boolean) {
return isUTC ? 'setUTCHours' : 'setHours';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function minutesSetterName(isUTC: boolean) {
return isUTC ? 'setUTCMinutes' : 'setMinutes';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function secondsSetterName(isUTC: boolean) {
return isUTC ? 'setUTCSeconds' : 'setSeconds';
}
+/**
+ * @deprecated Use time-zone-aware utilities instead of selecting a local/UTC
`Date` setter.
+ */
export function millisecondsSetterName(isUTC: boolean) {
return isUTC ? 'setUTCMilliseconds' : 'setMilliseconds';
}
+
+interface TimeZoneDateParts {
+ year: number;
+ // Calendar month, from 1 (January) to 12 (December), matching
Intl/Temporal.
+ month: number;
+ day: number;
+ dayOfWeek: number;
+ hours: number;
+ minutes: number;
+ seconds: number;
+ milliseconds: number;
+ // Same sign as an ISO offset: UTC-05:00 is -300 and UTC+05:30 is 330.
+ offsetMinutes: number;
+}
+
+type TimeZoneWallTimeParts = Omit<TimeZoneDateParts, 'dayOfWeek' |
'offsetMinutes'>;
+
+interface TimeZoneDayInfo {
+ offsetBefore: number;
+ transitionTimestamp?: number;
+ offsetAfter: number;
+}
+
+interface TimeZoneDayCache {
+ dayStartOffsets: zrUtil.HashMap<number, number>;
+ days: zrUtil.HashMap<TimeZoneDayInfo, number>;
+}
+
+// Required for IANA time zones. Legacy environments can provide an Intl
polyfill.
+// eslint-disable-next-line no-restricted-globals
+const intl = Intl;
+type TimeZoneFormatter = ReturnType<typeof intl.DateTimeFormat>;
+type TimeZoneFormatterOptions = NonNullable<Parameters<typeof
intl.DateTimeFormat>[1]>;
+
+const MINUTES_PER_DAY = ONE_DAY / ONE_MINUTE;
+const formatterCache = zrUtil.createHashMap<TimeZoneFormatter, string>();
+const timeZoneDayCaches = zrUtil.createHashMap<TimeZoneDayCache, string>();
Review Comment:
These caches are unbounded and keyed by arbitrary time-zone strings. In
long-lived apps that dynamically construct many distinct `timeZone` values (or
accept user input), this can lead to steady memory growth. If that’s a
realistic usage pattern for this library, consider adding a bounded cache
strategy (LRU / size cap) or a way to clear caches (at least in test
environments) to avoid unbounded retention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
