PolicyEngine / policyengine-app

PolicyEngine's free web app for computing the impact of public policy.
GNU Affero General Public License v3.0
37 stars 101 forks source link

Internationalize `equivalised` #1092

Closed MaxGhenis closed 8 months ago

MaxGhenis commented 8 months ago

To use a z in the US wherever we show it

abhcs commented 8 months ago

The quick "fix" would be to just do this for equivalized in the decile charts. A better fix would be to use an internationalization framework such as formatjs or i18next. This issue is related to #1070.

abhcs commented 8 months ago

I have been investigating how an internationalization framework such as formatjs can be added to the code. The idea is to not display any string directly to the user but to pipe it through the framework. Here are some benefits and costs to consider:

Benefits

Costs

Question

anth-volk commented 8 months ago

Thanks for delving into this, @abhcs.

While I think such a framework makes sense for a large application where this issue might emerge many times, especially if we were to also use other languages, I wonder if this isn't too much overhead for what we're trying to do.

I'm curious if there isn't some middle ground where we merely defined a local internationalization function that we passed a country_id and a word in American English to, that then supplied the correct spelling from a data object similar to the following:

{
  equivalized: {
    uk: equivalised
  },
  center: {
    uk: centre,
    ca: centre
  }
}

We could then wrap the respective words in this internationalization function within a template literal, cutting down on the conditional React code akin to what you describe under "Benefits."

The drawback of this, obviously, is that it would require manually defining each internationalized version, but wouldn't formatjs require doing similarly? Even so, we'd avoid having to implement an internationalization framework app-wide. That said, curious to hear your thoughts: is this proposal too similar to how formatjs works to warrant treating it separately?

abhcs commented 8 months ago

I'm curious if there isn't some middle ground where we merely defined a local internationalization function...

The main idea in frameworks such as formatjs is that the main code looks very clean with a default message -- there are no conditionals based on locale. The localization logic moves away from the code and isn't local.

As an example, consider the way Plotly is used in policy output charts. We pass the locale name to Plot but not the currency symbol extracted from metadata. The locales are defined away from the code in plotly_locales. This is how issues #148 and #1068 were fixed.

As another example, consider an older version of AverageImpactByDecile. If we were using formatjs here instead, we could express the conditional structure of the message by using the select construct. The bits of code that say

${formatVariableValue(metadata.variables.household_net_income, change, 0)}

would get replaced by {change, number, precision-integer currency/${metadata.currency}}. If you want the sign to always be visible and compact the number by using suffixes such as k, bn, etc, then you can further modify the message to {change, number, sign-always compact-short currency/${metadata.currency}}. You can read more about it here. My point is that formatjs provides a powerful language for specifying messages. Using it will make PolicyEngine more compact and error-free. This aspect of formatjs was not properly emphasized in my previous comment.

If the transition to formatjs is done properly and we consistently use it when adding new features, bugs such as this one will not get introduced.

Even so, we'd avoid having to implement an internationalization framework app-wide.

The complexity of the PolicyEngine is constant. Frameworks such as formatjs can help with offloading some of its complexity in creating localized messages. This has a setup cost -- in a sense we would have to rebase the app on formatjs. However, if you expect more messages of this type to be added to PolicyEngine in the future, transitioning to a framework like formatjs would be recommended.