Web-Component read strings from i18n/messages.*.json

[joanise]

Instead of coding the strings right in returnTranslation(), read and use the messages in i18n/messages.*.json in a format that we can later integrate into POEditor for ease of translation and maintenance.

Fixes #193

[joanise]

Doh, CI was failing on my own computer and I failed to notice it. Fixing that.

[joanise]

So, this PR is submitted now, and ready for review. I'm not seeking to merge it before ICLDC is over, but I would like feedback on it.

Of particular importance to me, since I'm still learning how to do this stuff:

• did I use anything that might break on old browsers we still want to support? (e.g., is it really OK to use Object.entries()?)
• is my import style OK, importing from the three files and hard-coding the building of an object from them?

At some point we might want to generalize the lang field to infer its allowed values from the existence of messages files, but that's going to be for another day.

[dhdaines]

Yes, Object.entries is the right thing to use, and should be fine for browser compatibility: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries#browser_compatibility

In older browsers we'll have more important problems!

That said, you could also define substitutions with an index signature to avoid having to call it any: https://www.typescriptlang.org/docs/handbook/2/objects.html#index-signatures

[dhdaines]

(my comment didn't show up but you also won't have to cast the substitution with as string if you use an index signature)

[joanise]

Nice, I like the index signature thing. Is this quick solution OK, or do I need to actually define a type?

  getI18nString(key: string, substitutions: { [index: string]: string } = {}): string {
    let result: string = this.getRawI18nString(key);
    for (const [sub_key, value] of Object.entries(substitutions)) {
      result = result.replace("<" + sub_key + ">", value);
    }
    return result;
  }

It works, in any case, and inlining the type definition, for something this simple, seems like the right thing to do to me.

[dhdaines]

Just need to make sure that angular extract-i18n will leave these alone, I presume you did!

[joanise]

We only run extract-i18n in Studio-Web, so it should not touch anything in here. And indeed, nx extract-i18n studio-web does not modify any files outside packages/studio-web so it's OK.
I'm sure, however, that this would be incompatible with any i18n bundle, it's strictly my own manual i18n process! I like how it's fully self-contained in our own code, though, with no (additional) external dependencies.

[dhdaines]

Make sure that the type definition for InterfaceLanguage includes spa...

[joanise]

That conveniently got done automatically when we updated line 36 to

language: "eng" | "fra" | "spa" = "eng";

in demo.component.ts. Or maybe that's line you're saying I should make sure to have modified, in the first place?

In any case, I love how you can't miss this one when working with TS. If I revert it to just "eng" | "fra", then the code this.language = "spa"; just a few lines lower is considered an error by TS. Although, wait, that's a kind of optional fluff line that trips us, but if I remove it, it all compiles, and language="spa" works even if it's not allowed by the type. I guess that's a consequence of the runtime happening with the transpiled JS, setting language to an illegal value in the HTML is invisible to the TS transpiler.

Anyway, long story short, it's good to check this!

[roedoejet]

This is awesome @joanise - clean, clear, efficient solution that will make adapting to future langauges much easier. Thanks!