unsplash / intlc

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

Compiling flattened JSON results in type error #111

Closed OliverJAsh closed 2 years ago

OliverJAsh commented 2 years ago

Given this JSON:

app/routes/Users/components/UserStatsSubRoute/components/StatsGraph/lang/en-US.translations.json:

{
  "infoIconTooltip": {
    "message": "Part of your {title, select, Views {views} Downloads {downloads}} come from being {title, select, Views {viewed in} Downloads {downloaded from}} the following applications."
  }
}

If we compile, it generates valid TS:

$ intlc compile --locale en-US app/routes/Users/components/UserStatsSubRoute/components/StatsGraph/lang/en-US.translations.json | prettier --parser=babel
export const infoIconTooltip: (x: { title: 'Views' | 'Downloads' }) => string = (x) =>
  `Part of your ${(() => {
    switch (x.title) {
      case 'Views':
        return `views`;
      case 'Downloads':
        return `downloads`;
    }
  })()} come from being ${(() => {
    switch (x.title) {
      case 'Views':
        return `viewed in`;
      case 'Downloads':
        return `downloaded from`;
    }
  })()} the following applications.`;

However, if we flatten and then compile, the generated TS has a type error:

$ intlc flatten app/routes/Users/components/UserStatsSubRoute/components/StatsGraph/lang/en-US.translations.json > flattened.json

$ intlc compile --locale en-US flattened.json | prettier --parser=babel
export const infoIconTooltip: (x: { title: 'Views' | 'Downloads' }) => string = (x) =>
  `${(() => {
    switch (x.title) {
      case 'Views':
        return `${(() => {
          switch (x.title) {
            case 'Views':
              return `Part of your views come from being viewed in the following applications.`;
            case 'Downloads':
              return `Part of your views come from being downloaded from the following applications.`;
          }
        })()}`;
      case 'Downloads':
        return `${(() => {
          switch (x.title) {
            case 'Views':
              return `Part of your downloads come from being viewed in the following applications.`;
            case 'Downloads':
              return `Part of your downloads come from being downloaded from the following applications.`;
          }
        })()}`;
    }
  })()}`;
image
samhh commented 2 years ago

tsc is narrowing the type of the object property in the outer switch case when we don't want it to:

type A = 'a1' | 'a2'
declare const x: { y: A }

switch (x.y) {
    case 'a1': { // narrows
        switch (x.y) {
            case 'a2': {} // errors
        }
    }
}

We can't do something like as typeof x.y as that's similarly impacted by this narrowing.

Is it semantically viable to unflatten the AST? That'd workaround this issue and give terser output, and it'd avoid having to concern the JS compiler with a fundamentally TS issue.

OliverJAsh commented 2 years ago

Is it semantically viable to unflatten the AST?

I have no idea. 😬

samhh commented 2 years ago

Here's a hacky way we could solve this. Change all switch conditions from x.y to (() => x.y)() (or id(x.y) - all that matters is that it enters a function which TS won't follow). :laughing:

samhh commented 2 years ago

Alternatively we could try to keep track of what we've matched upon so far but... I'm a fan of how we've avoided essentially doing our own typechecking until now.

OliverJAsh commented 2 years ago

One other way to solve this might be to use a type assertion everywhere we use switch, to undo any narrowing from the parent scope.

type A = 'a1' | 'a2'
declare const x: { y: A }

switch (x.y as A) {
    case 'a1': { // narrows
        switch (x.y as A) {
            case 'a2': {} // no error
        }
    }
}

(I suggested doing it for all switches so we don't have to keep track of which ones are nested.)

Also, we don't have to solve this if there's no elegant solution. I don't think it's going to come up very often.