Add beginnings of i18n support to UI, and update version of node and eslint and friends

[jonathaningram]

• At this stage translations have only been applied to the main
  dashboard page.
• Rename CF_SKIN to SKIN_NAME since the CF prefix seems odd and
  redundant.
• Add SKIN_PROVIDES_TRANSLATIONS so that skins (or more correctly,
  clients that are building the app with a skin) can indicate that their
  skin does include translation files.
• The default cg skin still works as before and no translation files
  are required for it to work. This is because the translation keys are
  in fact just pieces of text.
• There is some crossover with skinning and translating and there may be
  cases we should make some things configurable by the current skin JS
  properties, but there is also the ability to only use the
  translation files for certain things. For example a "contact us" link
  could just default to the cloud.gov one and skins could override it by
  providing their own translation. Probably need some discretion here.
• There may be cases where a text key is not suitable because the skin
  wants to render two different things where the default skin wants to
  render the same thing.
• Because the default (cg) skin does not need to provide translation
  files, the SKIN_NAME (formerly CF_SKIN) variable has been passed into
  process.env. This is deliberate because now the i18n.js module can
  remove dead code at build time. Particularly, the i18next-xhr-backend
  module can be entirely removed for cloud.gov.
• The i18next module allows us to actually change the language but I
  don't see a need for this. For example, cloud.gov is unlikely to want
  to allow changing the language to en-NZ within the interface - thus
  there is no need for the main app to provide arbitrary translation
  files (e.g. en-GB). Skins will probably be the only way to provide
  translation files.

Relevant files from our skin for context:

• See how we're setting our skin lang: https://github.com/govau/ci-console/blob/master/skin/index.js#L9
• And our current translation: https://github.com/govau/ci-console/blob/master/skin/locales/en-AU/translations.json

How much does this add to the build? Well, there's something suspicious going on with the build as is (unless I'm mistaken) because it seems to be about 7MB...I haven't dived into it yet to see if there's an issue. Anyway, here's a rough idea how much these modules add to the build:

Of course, for cloud.gov, the Backend is currently excluded from the build because of dead code elimination.

Looking forward to hearing your thoughts on this!

Regards,
Jon

[el-mapache]

So, if the i18n library can't find a key, does it default to showing the provided string? I'm slightly confused about what the I18n component is doing here. Is it intended to be illustrative?

[jonathaningram]

Exactly - if the key does not exist it just shows the text per https://www.i18next.com/principles/fallback.html#key-not-found

WRT the I18n component: this is a component that provides a "render prop" per https://react.i18next.com/components/i18n-render-prop.html#i18n-render-prop

I'm not sure if you've followed Ryan Florence or Michael Jackson's recent discussions on HOCs versus render props, but what I've done here is used the render prop version instead of the HOC version. Here's their blog article and inside it is also the video on the talk that Michael Jackson gave: https://cdb.reacttraining.com/use-a-render-prop-50de598f11ce.

I'll let you dig into those links as you please, but essentially what I'm doing is including this I18n component which gives me access to t so that I can translate things. You will have noticed that I've used <I18n /> deep within the render tree in certain places instead of the first node in the tree: this is because at this stage we're only translating certain specific strings (the "organization" -> "organisation" stuff). If we were actually providing 100% coverage of translating all text, then usually the I18n component would be the first node in the render tree because then all subsequent nodes would have access to t. Within a particular component you'd usually not have more than one rendering of <I18n /> in a tree because you'd just refactor the multiple uses of <I18n /> out to be at the top of the tree.

I hope that clears it up, let me know if you need more info.

[el-mapache]

Re: large file size, the production build is something like 350kb, so I don't think there is anything to worry about yet 😄

[jonathaningram]

Re: large file size, the production build is something like 350kb, so I don't think there is anything to worry about yet 😄

Hmmm odd, I'm seeing much larger when building it with npm run build-prod, but I'll investigate more on my end.

Edit: also, I don't think we're doing code splitting yet, so we probably have lots of room to get that 350kb down into smaller chunks 😎

[jonathaningram]

OK, I just pushed a change to this PR to basically update node and update eslint. Why? The build was failing because the former eslint-airbnb version that we were enforcing was complaining that my line was longer than 100 chars. In order to pass the linter I had to split the JS string into parts and compose it with + (e.g. 'a' + 'b' + 'c') but then that violates Airbnb's rule to not split strings.

Airbnb have since removed this 100 char limit for JS strings, so I have updated it (along with other eslint packages). However, the current version of node/npm does not work properly with shrinkwrapping (well, it's frustratingly flaky and painful to get working) so I also updated it. I also had trouble adding the original i18next* packages where I was doing a rm -rf node_modules && npm install && npm install --save this and that dance to get the shrinkwrap file to update.

The avid reader will notice that the build still fails. This is because of all the new/changed rules in the latest version of Airbnb's styleguide (incidentally lots of these are formatting things that would probably just go away with prettier 😎). So we were actually enforcing an old version of the styleguide, even though when referring to Airbnb's repository README docs, we were looking at and using the latest rules.

The next step is to go through and fix all these failing lint issues so the build passes, however I don't want to touch the codebase until #1250 is merged since that touches most of the code.

I didn't want this PR to stray away from being just about i18n, but I think it's for the best.

[el-mapache]

Whoops, I was basing that number off the production network requests, which are gzipped. The prod bundle is probably bigger than it needs to be, but at least it's significantly less that 7mb!

[jcscottiii]

@jonathaningram first off, thank you soo much for this! this is awesome.
i'm trying to imagine the worst case scenario where if there was a non-Latin Alphabet translation, how would we implement that. The partial sentence translation might not work out cleanly. I'm a little in favor of the key-value files like here: https://github.com/i18next/react-i18next/blob/master/example/react_renderProps/public/locales/en/translations.json

instead of have the json file with only certain words.

This way, if we change our phrasing slightly, the key will be the same and translations will still work.

But would love to hear your thoughts for the other way.

[jonathaningram]

But would love to hear your thoughts for the other way.

To be fair, I don't have a huge amount of experience with translation keys being the real words, but I do have experience with the translation keys being name.spaced.keys and so I can talk about some of the down sides of that method.

1. It's so verbose. Example from https://help.shopify.com/themes/development/internationalizing/translation-filter. It's arguably very unreadable:

Further, your code now looks like this:

There's just a large cognitive load to follow that: "ok, in layout, then under header, there is a key called support_link". Aside: in this case I feel like I would have preferred there be a "header.json" file containing the translations because the prefix layout. is completely redundant and does not match component-driven design (where <Header /> should probably not know of some concept called layout).

2. Searching for keys is hard. Given a key article.comment_form.submit_button_text - I want to find the translation file so I can change button_text to something else. I can't just search for button_text because no doubt there are lots of buttons.

3. Translation files get out of hand because of the nesting. Often you end up with a useless prefix or root namespace which is completely redundant (even at times it's the project name - I've had this previously on a project). Using the words/phrases directly gives you a completely flat translation file.

4. Because you've taken out the real words/phrases and replaced them with keys everywhere, your component ends up looking like a really abstract thing with the meaning taken away.

5. Maybe it's easier for actual translators to convert a word or phrase into the target language? E.g. given both article.comment_form.submit_button_text and Submit comment, maybe the second is easier to translate?

In terms of backwards compatibility, you make a good point that if cloud.gov changes some wording "Here's something" into "Here is something", all skins that relied on the old key will no longer work because the new key does not exist. For cloud.gov.au this would be OK because the fallback "Here is something" will just show. For cloud.gov.fr though, you'd end up with a random English phrase in your application. Perhaps in this case the project would benefit from tagged versions and a CHANGELOG.md (semver things). Anyway, see my final thought for why this may not even matter for now.

Final thought:

Correct me if I'm wrong on this point. But if we want to be pragmatic, at the moment, I believe cloud.gov and cloud.gov.au are the only users of this project. We don't have a need to translate all the things. For the most part our goals and messaging align pretty well and also there are lots of places where we'll start refactoring out particular content sections that the skin must now provide. Thus, these core content pieces won't even need translating.

[jonathaningram]

As a case study, I just had a look at the kubernetes UI for how they do i18n, and looks like they are doing word/phrase-based keys: https://github.com/kubernetes/dashboard/blob/9c9b2a3aa18dcc029f9899d08ab3bfd1f995d101/src/app/frontend/deploy/deployanyway_dialog.js#L34

[jcscottiii]

But if we want to be pragmatic, at the moment, I believe cloud.gov and cloud.gov.au are the only users of this project

Currently, this is true but I don't want to keep the barrier high for future languages

The K8s case study uses a common key and has translation files that use that common key.
For example: MSG_DEPLOY_ANYWAY_DIALOG_TITLE

They then have .xtb files that make use of that key
https://github.com/kubernetes/dashboard/search?utf8=%E2%9C%93&q=MSG_DEPLOY_ANYWAY_DIALOG_TITLE&type=

Currently, this PR doesn't have that common key. We could do something like that but that's similar to what I offered above with our own translation files. The only difference is that you as the programmer know what the default translation will be so that you have an idea without switching files.

We could combine that idea too.

https://www.smashingmagazine.com/2017/01/internationalizing-react-apps/#pluralization-and-templates

[jonathaningram]

I'm going to close this for now and revisit later. It needs to be rebased anyway, plus we might not want to take the approach that I used in this PR.