aeson-typescript icon indicating copy to clipboard operation
aeson-typescript copied to clipboard

Prohibit `undefined` in TypeScript Index Signatures for `Data.Map` when key is `String`

Open benrbray opened this issue 1 year ago • 3 comments

Thanks for aeson-typescript, it's great!

Example

I've noticed that when using Data.Map as follows:

newtype Example = Example (Map String Int)
$(deriveTypeScript defaultOptions ''Foo)
result = tt @Example

(helper functions)

tsFormatOpts :: FormattingOptions
tsFormatOpts = defaultFormattingOptions {exportMode = ExportEach, typeAlternativesFormat = EnumWithType}
ts :: (TypeScript a) => Proxy a -> String
ts = formatTSDeclarations' tsFormatOpts . getTypeScriptDeclarations
tt :: forall a. (TypeScript a) => String
tt = ts (Proxy @a)

The generated types look like this:

export type Example = IExample;
// actual generated type
export type IExample = {[ k in string] ?: number};

It's a bit difficult to use these types, since indexing IExample will give a number | undefined value, even though the ToJSON instance for Data.Map should never include undefined values.

const foo: Example = { bar : undefined } // no error

function example(data: Example) {
  const bar: { [k : string] : number } = data;
  // Type 'IExample' is not assignable to type '{ [k: string]: number; }'.
  // 'string' index signatures are incompatible.
  //   Type 'number | undefined' is not assignable to type 'number'.
  //     Type 'undefined' is not assignable to type 'number'.
}

Note that these errors still happen with exactOptionalPropertyTypes enabled.

Desired Behavior

Instead, I would have expected the generated type to look more like this:

// desired type
export type IExample = { [k : string] : number };

const foo: Example = { bar : undefined }
// Type 'undefined' is not assignable to type 'number'.

function example(data: Example) {
  const bar: { [k : string] : number } = data;
  // Type 'IExample' is not assignable to type '{ [k: string]: number; }'.
  // 'string' index signatures are incompatible.
  //   Type 'number | undefined' is not assignable to type 'number'.
  //     Type 'undefined' is not assignable to type 'number'.
}

Questions

  • was there a particular reason for this choice of types?
  • would it be possible to change the behavior when the key is String or an alias for String?

Related

microsoft/TypeScript#46969

benrbray avatar Jul 21 '23 03:07 benrbray

Hmm, the question mark was introduced in this commit: https://github.com/codedownio/aeson-typescript/commit/64c03f3b8ffe15180f60d6cc72795f9ac8639798

It's been a while so I don't remember the context well. It makes sense that : was changed to in but I'm not sure why the question mark was needed. I can't think of any situation where such a map would contain undefined.

I tried removing it locally and the tests passed ¯\(ツ)/¯. Trying it in CI here.

Before merging such a change I'd just want to think really hard and see if there's any way aeson can encode a Map or HashMap to have undefined values, in a way that isn't due to the value itself encoding to undefined...

thomasjm avatar Jul 27 '23 11:07 thomasjm

(Is there some situation where k: string and k in string differ in TypeScript?)

thomasjm avatar Jul 27 '23 11:07 thomasjm

The commit comment says:

This allows to have unions as keys, which often happens when encoding variants made out of nullary constructors.

It sounds like string keys shouldn't ever cause this. I agree that it'd be great to not have the ? for string keys. Or maybe more generally, for types where there's no possibility of undefined.

wraithm avatar Apr 19 '24 00:04 wraithm