Do not generate `Record` for unsupported key types

[tpetrucciani]

Map[NonBlankString, NonBlankString] is converted to Record<NonBlankString, NonBlankString>, but Record doesn't support NonBlankString as key type.

final case class RuleConfig(
  ruleId: Id[Rule],
  params: Map[NonBlankString, NonBlankString],
)

↓

export interface RuleConfig {
  ruleId: Id<Rule>,
  params: Record<NonBlankString, NonBlankString>
}

(From https://github.com/buildo/retro/issues/107)

[Cardu]

UPDATE the desired output imho should be a ts Map

export interface RuleConfig {
  ruleId: Id<Rule>,
  params: Map<NonBlankString, NonBlankString>
}

the only difficult point is that there is no map in io-ts but we could easily implement it

[giogonzo]

Also: I believe that, Scala-side, we do both of the following:

1. use Map[A, B] with a non-finite A type (or implying that some keys could not be set anyway)
2. use Map[A, B] with finite domain, and implying that for all values in A, a key is set and points to a value of type B

Is this correct? We should somehow differentiate the two cases above and act differently based on the situation. THe best possible outcome, TS-side, would be:

1. if we are dealing with a non-finite domain

   1. if A is a primitive value (let's even stick to string and number for simplicity) generate a Record<A, B | undefined> (note: the | undefined is debatable here.. if accessed with e.g. fp-ts helpers, any access will be an Option<B> anyway, If accessed with primitive syntax though, this is not the case)
   2. is A is a non-primitive, generate a Map<A, B> <-- Map allows for any type as key, plus encodes the fact that (if accessed with fp-ts helpers) accessing it returns Option<B>

2. if the domain is finite and Scala-side we are implying that all keys are set, Record<A, B> is 👍

[tpetrucciani]

I agree it would be nice to distinguish total and partial maps. And yes, in Scala we represent both with Map and I don't think we distinguish them in any way.

By the way, we already translate maps differently depending on whether the domain is an enum:

function partialRecordCombinator(
  keyType: gen.TypeReference,
  valueType: gen.TypeReference
): Reader<Ctx, gen.Combinator> {
  return ask<Ctx>().map(({ models }) => {
    const model = models.find(m => keyType.kind === 'Identifier' && m.name === keyType.name);
    if (model && model._type === 'CaseEnum') {
      return gen.partialCombinator(model.values.map(k => gen.property(k.name, valueType)));
    }
    return gen.recordCombinator(keyType, valueType);
  });
}

Why are we using the partial combinator in that case?

(Enum domains sometimes correspond to total maps I suppose, but not always.)

[Cardu]

Also: I believe that, Scala-side, we do both of the following:

1. use Map[A, B] with a non-finite A type (or implying that some keys could not be set anyway)
2. use Map[A, B] with finite domain, and implying that for all values in A, a key is set and points to a value of type B

Is this correct? We should somehow differentiate the two cases above and act differently based on the situation. THe best possible outcome, TS-side, would be:

1. if we are dealing with a non-finite domain

   1. if A is a primitive value (let's even stick to string and number for simplicity) generate a Record<A, B | undefined> (note: the | undefined is debatable here.. if accessed with e.g. fp-ts helpers, any access will be an Option<B> anyway, If accessed with primitive syntax though, this is not the case)
   2. is A is a non-primitive, generate a Map<A, B> <-- Map allows for any type as key, plus encodes the fact that (if accessed with fp-ts helpers) accessing it returns Option<B>

Why not use always the option ii?

2. if the domain is finite and Scala-side we are implying that all keys are set, Record<A, B> is 👍

I think this case should not be represented as a Map in the backend: if there is a finite domain and there should be a value for every key, we could just use a case class

[giogonzo]

Why are we using the partial combinator in that case?

@tpetrucciani I believe because the example at hand when we implemented that code was of a non-total map.. so it's just a heuristic, and not a good one 😄

I think this case should not be represented as a Map in the backend: if there is a finite domain and there should be a value for every key, we could just use a case class

@Cardu provided this "code convention" is accepted Scala-side, this could allow for a much better heuristic

[tpetrucciani]

I think this case should not be represented as a Map in the backend: if there is a finite domain and there should be a value for every key, we could just use a case class

@Cardu Yes, that makes sense.

The case where we might like a total map is when there is a preexisting enumeration type (say UserType = User | Admin | SuperAdmin) and we want to expose in the model a value for each element of the enum. So we want a total Map[UserType, String] for example.

I suppose your point is we should just declare

case class UserTypeData(user: String, admin: String, superAdmin: String)

which indeed sounds better overall (the only problem is we lose a bit the correlation with the UserInfo type).

BTW I've had a look at a project where I thought we had some "supposed-to-be-total" maps, but actually now I look at them they're probably all meant to be potentially partial.