Skip to content
This repository has been archived by the owner on Mar 31, 2024. It is now read-only.

Commit

Permalink
[@kbn/datemath] improve types (elastic#24671)
Browse files Browse the repository at this point in the history
* [kbn-datemath][parseEsInterval] improve types slightly

* [kbn-datemath][vis/leastCommonInterval] make types more precise

* [ui/leastCommonInterval] fix bug in finding same types

* add back valid test
  • Loading branch information
Spencer committed Oct 30, 2018
1 parent 1d8a0b9 commit f6889d8
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 26 deletions.
34 changes: 25 additions & 9 deletions packages/kbn-datemath/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,29 @@
* under the License.
*/

declare module '@kbn/datemath' {
const dateMath: {
parse: any;
unitsMap: any;
units: string[];
unitsAsc: string[];
unitsDesc: string[];
import moment from 'moment';
export type Unit = 'ms' | 's' | 'm' | 'h' | 'd' | 'w' | 'M' | 'y';

declare const datemath: {
unitsMap: {
[k in Unit]: {
weight: number;
type: 'calendar' | 'fixed' | 'mixed';
base: number;
}
};
export default dateMath;
}
units: Unit[];
unitsAsc: Unit[];
unitsDesc: Unit[];

parse(
input: string,
options?: {
roundUp?: boolean;
forceNow?: boolean;
momentInstance?: typeof moment;
}
): moment.Moment | undefined;
};

export default datemath;
6 changes: 3 additions & 3 deletions packages/kbn-datemath/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ const unitsMap = {
m: { weight: 3, type: 'mixed', base: 1000 * 60 },
h: { weight: 4, type: 'mixed', base: 1000 * 60 * 60 },
d: { weight: 5, type: 'mixed', base: 1000 * 60 * 60 * 24 },
w: { weight: 6, type: 'calendar' },
M: { weight: 7, type: 'calendar' },
w: { weight: 6, type: 'calendar', base: NaN },
M: { weight: 7, type: 'calendar', base: NaN },
// q: { weight: 8, type: 'calendar' }, // TODO: moment duration does not support quarter
y: { weight: 9, type: 'calendar' },
y: { weight: 9, type: 'calendar', base: NaN },
};
const units = Object.keys(unitsMap).sort((a, b) => unitsMap[b].weight - unitsMap[a].weight);
const unitsDesc = [...units];
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/utils/parse_es_interval/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
* under the License.
*/

export { parseEsInterval } from './parse_es_interval';
export { parseEsInterval, ParsedInterval } from './parse_es_interval';
export { InvalidEsCalendarIntervalError } from './invalid_es_calendar_interval_error';
export { InvalidEsIntervalFormatError } from './invalid_es_interval_format_error';
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
* under the License.
*/

import { Unit } from '@kbn/datemath';

export class InvalidEsCalendarIntervalError extends Error {
constructor(
public readonly interval: string,
public readonly value: number,
public readonly unit: string,
public readonly unit: Unit,
public readonly type: string
) {
super(`Invalid calendar interval: ${interval}, value must be 1`);
Expand Down
19 changes: 12 additions & 7 deletions src/ui/public/utils/parse_es_interval/parse_es_interval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import dateMath from '@kbn/datemath';
import dateMath, { Unit } from '@kbn/datemath';

import { InvalidEsCalendarIntervalError } from './invalid_es_calendar_interval_error';
import { InvalidEsIntervalFormatError } from './invalid_es_interval_format_error';
Expand All @@ -26,6 +26,8 @@ const ES_INTERVAL_STRING_REGEX = new RegExp(
'^([1-9][0-9]*)\\s*(' + dateMath.units.join('|') + ')$'
);

export type ParsedInterval = ReturnType<typeof parseEsInterval>;

/**
* Extracts interval properties from an ES interval string. Disallows unrecognized interval formats
* and fractional values. Converts some intervals from "calendar" to "fixed" when the number of
Expand All @@ -37,15 +39,15 @@ const ES_INTERVAL_STRING_REGEX = new RegExp(
* | -------- | ---------------- | ------------------- |
* | ms | fixed | fixed |
* | s | fixed | fixed |
* | m | fixed | fixed |
* | m | calendar | fixed |
* | h | calendar | fixed |
* | d | calendar | fixed |
* | w | calendar | N/A - disallowed |
* | M | calendar | N/A - disallowed |
* | y | calendar | N/A - disallowed |
*
*/
export function parseEsInterval(interval: string): { value: number; unit: string; type: string } {
export function parseEsInterval(interval: string) {
const matches = String(interval)
.trim()
.match(ES_INTERVAL_STRING_REGEX);
Expand All @@ -54,9 +56,9 @@ export function parseEsInterval(interval: string): { value: number; unit: string
throw new InvalidEsIntervalFormatError(interval);
}

const value = matches && parseFloat(matches[1]);
const unit = matches && matches[2];
const type = unit && dateMath.unitsMap[unit].type;
const value = parseFloat(matches[1]);
const unit = matches[2] as Unit;
const type = dateMath.unitsMap[unit].type;

if (type === 'calendar' && value !== 1) {
throw new InvalidEsCalendarIntervalError(interval, value, unit, type);
Expand All @@ -65,6 +67,9 @@ export function parseEsInterval(interval: string): { value: number; unit: string
return {
value,
unit,
type: (type === 'mixed' && value === 1) || type === 'calendar' ? 'calendar' : 'fixed',
type:
(type === 'mixed' && value === 1) || type === 'calendar'
? ('calendar' as 'calendar')
: ('fixed' as 'fixed'),
};
}
6 changes: 6 additions & 0 deletions src/ui/public/vis/lib/least_common_interval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ describe('leastCommonInterval', () => {
expect(() => {
leastCommonInterval('60 s', '1m');
}).toThrowError();
expect(() => {
leastCommonInterval('1m', '2m');
}).toThrowError();
expect(() => {
leastCommonInterval('1h', '2h');
}).toThrowError();
expect(() => {
leastCommonInterval('1d', '7d');
}).toThrowError();
Expand Down
13 changes: 8 additions & 5 deletions src/ui/public/vis/lib/least_common_interval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import { parseEsInterval } from '../../utils/parse_es_interval';
* Finds the lowest common interval between two given ES date histogram intervals
* in the format of (value)(unit)
*
* - `ms, s` units are fixed-length intervals
* - `m, h, d` units are fixed-length intervals when value > 1 (i.e. 2m, 24h, 7d),
* - `ms` units are fixed-length intervals
* - `s, m, h, d` units are fixed-length intervals when value > 1 (i.e. 2m, 24h, 7d),
* but calendar interval when value === 1
* - `w, M, q, y` units are calendar intervals and do not support multiple, aka
* value must === 1
Expand All @@ -51,7 +51,7 @@ export function leastCommonInterval(a: string, b: string): string {
}

// If intervals are calendar units, pick the larger one (calendar value is always 1)
if (aInt.type === 'calendar') {
if (aInt.type === 'calendar' || bInt.type === 'calendar') {
return aUnit.weight > bUnit.weight ? `${aInt.value}${aInt.unit}` : `${bInt.value}${bInt.unit}`;
}

Expand All @@ -68,8 +68,11 @@ export function leastCommonInterval(a: string, b: string): string {
return a.replace(/\s/g, '');
}

// Otherwise find the biggest unit that divides evenly
const lcmUnit = unitsDesc.find(unit => unitsMap[unit].base && lcmMs % unitsMap[unit].base === 0);
// Otherwise find the biggest non-calendar unit that divides evenly
const lcmUnit = unitsDesc.find(unit => {
const unitInfo = unitsMap[unit];
return !!(unitInfo.type !== 'calendar' && lcmMs % unitInfo.base === 0);
});

// Throw error in case we couldn't divide evenly, theoretically we never get here as everything is
// divisible by 1 millisecond
Expand Down

0 comments on commit f6889d8

Please sign in to comment.