unsplash / intlc

Compile ICU messages into code. Supports TypeScript and JSX. No runtime.
MIT License
57 stars 3 forks source link

# does not respect literal union types #93

Closed samhh closed 2 years ago

samhh commented 2 years ago

In plurals. Related to #79 in that # is generally buggy.

Given:

{
  "f": {
    "message": "{n, plural, =42 {#}}"
  }
}

Expected (see input type):

export const f: (x: { n: 42 }) => string = x => `${(() => { switch (x.n) { case 42: return `${new Intl.NumberFormat('en-XA').format(x.n)}`; } })()}`

Actual:

export const f: (x: { n: number & 42 }) => string = x => `${(() => { switch (x.n) { case 42: return `${new Intl.NumberFormat('en-XA').format(x.n)}`; } })()}`
samhh commented 2 years ago

At present the parser and ICU types are ignorant of the prospect of unionisation and it's handled later on in the compilers. On the other hand, # support is handled via the parser, where it's given a Number type. This mismatch is at the heart of this bug.

To my mind it probably makes sense to add a new ICU type to represent #, have it hold a reference to the name of the relevant plural arg, and have compilers take care of deciding what that means. Alternatively it could instead hold a copy of the full Arg, and we could just compile down the type like usual in the TypeScript compiler. :thinking:

samhh commented 2 years ago

The above suggestion might not work because n in {n, plural, =42 {#}} isn't fully resolved yet by the time we reach #. There's no resolved value to reference and trying to recursively resolve it will never halt.

It still needs some sort of type-level distinction but it doesn't need to affect the output aside from the runtime variable reference - a # reference to n should act as a no-op on n in terms of its computed type anyway.