[TS] collection.ensureIndex: replace overload signature with union

[akomm]

There are currently two problems with the signature of ensureIndex

1. A set of types, like EnsurePersistentIndexOptions and EnsureGeoIndexOptions exist, but no union type that combines the options. For example: type EnsureIndexOptions = EnsurePersistentIndexOptions | EnsureGeoIndexOptions | ...
2. Using overloading on ensureIndex is unnecessary because the options are fit to be a tagged union.

Because you use overloading (2.) instead of using a union (which would also solve the problem 1.), this is not possible:

// need to combine it when I want to wrap ensureIndex into some logic
type EnsureIndexOptions =
  | EnsurePersistentIndexOptions
  | EnsureGeoIndexOptions
  | EnsureTtlIndexOptions
  | EnsureZkdIndexOptions
  | EnsureInvertedIndexOptions

function takesEnsureIndexOptions(opts: EnsureIndexOptions) {
  // collection API
  collection.ensureIndex(opts) // this will fail, because overload signatures are not compatible with unions
}

You need to always consider your API is not used inline and the type is inferred, but also wrapped by some consumer logic. The current design is very limited in this way.

Example of the consequences and what is needed because of overloading:

await Promise.all(
  indecies.map(indexOpts => {
    switch (indexOpts.type) {
      case "persistent":
        return col.ensureIndex(indexOpts)
      case "geo":
        return col.ensureIndex(indexOpts)
      case "inverted":
        return col.ensureIndex(indexOpts)
      case "ttl":
        return col.ensureIndex(indexOpts)
      case "zkd":
        return col.ensureIndex(indexOpts)
    }
  })

Its also possible to map the Options to the Return type using conditionals. I assume that was the initial consideration when the overload API was picked? Maybe it was not possible at that time?

[akomm]

Example:

import {ArangoApiResponse} from "arangojs/connection"
import {
  EnsureGeoIndexOptions,
  EnsureInvertedIndexOptions,
  EnsurePersistentIndexOptions,
  EnsureTtlIndexOptions,
  EnsureZkdIndexOptions,
  Index
} from "arangojs/indexes"

type TaggedLookup<TType extends {type: string}> = {
  [TTag in TType["type"]]: TType extends {type: TTag} ? TType : never
}

type EnsureIndexOptions =
  | EnsurePersistentIndexOptions
  | EnsureGeoIndexOptions
  | EnsureTtlIndexOptions
  | EnsureZkdIndexOptions
  | EnsureInvertedIndexOptions

type EnsureIndexResult = TaggedLookup<Index>

declare function ensureIndex<TOptions extends EnsureIndexOptions>(
  options: TOptions
): Promise<
  ArangoApiResponse<
    EnsureIndexResult[TOptions["type"]] & {isNewlyCreated: boolean}
  >
>

declare const input0: EnsureIndexOptions // union
declare const input1: EnsurePersistentIndexOptions
declare const input2: EnsureGeoIndexOptions
declare const input3: EnsureTtlIndexOptions
declare const input4: EnsureZkdIndexOptions
declare const input5: EnsureInvertedIndexOptions

const index1 = ensureIndex(input1) // knows result is persistent index
const index2 = ensureIndex(input2) // knows result is geo index
const index3 = ensureIndex(input3) // knows result is ttl index
const index4 = ensureIndex(input4) // knows result is zkd index
const index5 = ensureIndex(input5) // knows result is inverted index

function takesEnsureIndexOptions<TOptions extends EnsureIndexOptions>(opts: TOptions) {
  return ensureIndex(opts)
}

const index6 = takesEnsureIndexOptions(input0) // knows result is one of persistent, geo, ttl, zkd or inverted index
const index7 = takesEnsureIndexOptions(input1) // knows result is persistent index

[akomm]

Is there any chance for the fix to get into the repository, if there was a PR?

Here example: TS Playground

import type {ArangoApiResponse} from "arangojs/connection"
import type {
  EnsureGeoIndexOptions,
  EnsureInvertedIndexOptions,
  EnsurePersistentIndexOptions,
  EnsureTtlIndexOptions,
  EnsureZkdIndexOptions,
  EnsureFulltextIndexOptions,
  PersistentIndex,
  TtlIndex,
  GeoIndex,
  InvertedIndex,
  ZkdIndex
  FulltextIndex
} from "arangojs/indexes"

// 1. add type here if needed
type EnsureIndexOptions =
    EnsurePersistentIndexOptions |
    EnsureTtlIndexOptions |
    EnsureGeoIndexOptions |
    EnsureInvertedIndexOptions |
    EnsureZkdIndexOptions |
    EnsureFulltextIndexOptions // deprecated, but for completness

// 2. add result for ensureIndex here
type EnsureIndexResult = {
    persistent: Promise<ArangoApiResponse<PersistentIndex & {isNewlyCreated: boolean}>>
    ttl: Promise<ArangoApiResponse<TtlIndex & {isNewlyCreated: boolean}>>,
    geo: Promise<ArangoApiResponse<GeoIndex & {isNewlyCreated: boolean}>>,
    inverted: Promise<ArangoApiResponse<InvertedIndex & {isNewlyCreated: boolean}>>,
    zkd: Promise<ArangoApiResponse<ZkdIndex & {isNewlyCreated: boolean}>>
    fulltext: Promise<ArangoApiResponse<FulltextIndex & {isNewlyCreated: boolean}>>
}

// 3. no need to change this one any longer
// You can't pass in what is not defined in 1.
// You get type error on *return type*, when you forget to sync 1. & 2.
declare function ensureIndex<TOptions extends EnsureIndexOptions>(details: TOptions): EnsureIndexResult[TOptions['type']]

The doc can either be merged on ensureIndex with each type example, or moved to the type property, as its what you would typically start with if you don't know the exact shape or options yet.

[akomm]

oh, wow