👍

Eelsewhere we have export type TimeMs = number; (that's for duration). So a S or Ms postfix may be useful.

It might be worth putting both this and the existing TimeMs into a common time type file (even outside /charts). The latter could perhaps be renamed DurationMs.

The type of the 1st and 2nd params of bisectLeft need to match, and we're typeguarding the 2nd param to number, so it's slightly tighter to say as number[]. Though it's not truthful, maybe we could brainstorm so I can learn about whether OrdinalDomain needs to be (number | string)[] and can't be number[] | string[] eg. by coercing numbers to strings if at least one element is a string. Though there's still the lack of type coherence between xValues and startValue - probably they correlate strongly (otherwise we can't do as any or as number[] here) but this seems to be lost on TS

If the type of bisectLeft is incorrect or too restrictive (we can't overrule it as it's beyond what the D3 TS API guarantees, but iirc these are 3rd party post hoc libs, not canonical Bostock contract) then maybe it'd be better to factor it out into our bisectLeft utility with the desired types, so we can easily change from D3 if needed. I think we could already use the also logarithmically bisecting monotonicHillClimb instead of d3.bisectLeft

There is one main reason for having (number | string)[] as OrdinalDomain: we like to preserve the original type, coming from the user data, to return the original values on the interaction callbacks (brushing or clicking)
We can probably force it to be number[] | string[] but I don't have strong opinion here

Some more info may strengthen the case for string[] | number[]:

D3 bisectLeft, and generally, logarithmic search requires total order, which we likely ensure by sorting the values beforehand(*).

Out of the properties of total order, transitivity is most important: if a <= b and b <= c then a <= c must be true (it's not necessarily true for partial orders, eg. when a number is defined non-comparable to a string).

(*) The mechanism doesn't matter, eg. [].sort also relies on a predicate plus array which together guarantee a total order, otherwise the order is ill defined: EcmaScript requires transitivity for predictable results: If a <CF b and b <CF c, then a <CF c (transitivity of <CF).

Sorting (number | string)[] with a < predicate doesn't lead to total order. Example:

a = '2'
b = 2.5
c = '10'

a < b: true
b < c: true
a < c: false

So a sensible option is to string-compare all elements if at least one element in the array is not a number, which is the default sort predicate (and number-compare for all-numeric arrays, which is not the default sort predicate). Instead of checking for this at the place of sorting, we may as well come clean and convert upfront. As you suggested Marco, the original value can still be retained. The effect of the conversion though is that a user-supplied [5, '5'] will map to ['5', '5'] so an equally good method is, documenting in the API, perhaps even enforcing via TS, that the domain values be homogeneous.

There can of course be other sorting rules, eg. all strings are moved to the end (tiebreaker rule for when a string and a number are compared), and within both the strings and numbers, there's full order

As it's quite a long switch, requiring up to 14 comparisons, it'd be a bit nicer if these were in a map, eg.

const esCalendarIntervalsToMoment = { 
  minute: 'minutes',
  m: 'minutes',
  hour: 'hour',
  h: 'hour',
  ...
}

...

const whatever = esCalendarIntervalsToMoment[interval.unit];

done in f471fd0

nit

I've update the logic to use a map instead of a switch as proposed by robert

I'm really not sure why this is needed as this is not configurable from outside of charts.

As it is we use moment across the charts code so are you thinking of being able to swap time libraries entirely? In that case we could have optional dependencies on moment and luxon so the user can switch between them.

If not is this to only be used as a utility to handle different time types?

This was done in the hope we can switch soon and easily to luxon or to Temporal https://tc39.es/proposal-temporal/docs/index.html
It really depends on Kibana right now, so extracting everything like that can help us migrating easily