Allow translate() to take an array of keys to test, and use the first one that exists. #150

Merged
merged 4 commits into from Sep 23, 2013

Projects

None yet

2 participants

@dpehrson
Contributor

This pull request is a result of the issue #125 regarding defaultValue with empty string keys. I found the "bug" because I was really abusing defaultValue to accomplish the goal of prioritized translation keys.

This pull request implements what I was attempting to do in a proper way.

Before I go into the details, some caveats:

  1. I've never contributed to an open source project that I didn't manage before.
  2. I cloned the repo and figured out how to get the tests running by trial and error. If I've edited things in less-than-ideal ways, let me know.
  3. The way I implemented my feature is by modifying the handling of translate()'s key argument. I realize that if there is a sacred part of the i18next code, translate() is probably it, so please let me know if this is a bad idea. Originally I was going to do this in a separate function, but as far as I can tell, it fits cleanly and transparently into the existing API.

Onto the why and how...

The Problem

You work for a large national company in a highly regulated industry. As a result they have two requirements:

  1. Code has to be deployed very carefully and very slowly through obscene amounts of red tape.
  2. Content has to be updated very quickly in response to regulatory and legal changes.

In my particular case, text that appears in the web application has to be customized on a state-by-state basis and may need to be changed within days of regulatory decisions or legal proceedings. These changes may not fall within normal code deployment windows so I need a way to allow the application to provide default and state-specific text without being aware in advance of which states are special snowflakes in advance (hint: 99% of the time it is California.)

The Old Solution

Nest translate() calls with defaultValue configurations:

// Note that this code is over-exaggerated to prove the point.
i18n.translate(
  ["key", state, city].join("."), // City-level override, if present.
  {
    arg: "value",
    defaultValue: i18n.translate(
      ["key", state].join("."), // State-level override, if present.
      {
        arg: "value",
        defaultValue: i18n.translate("key", {arg: "value"}) // No overrides.
      }
    )
  }
);

While this works as a result of #125 being merged, it was obvious from the conversation with @jamuhl that this was not the intended usage so I promised I'd provide a proper solution, which leads us to...

The New Solution

Instead of abusing the hell out of defaultValue, we simply rename translate()'s key parameter to potentialKeys and accept either a String or an Array<String> as the value.

  • If potentialKeys is a String, i18next behaves as we all know and love.
  • If potentialKeys is an Array<String>, i18next uses the first key that exists.

Now, our code looks like this:

i18n.translate(["key", state, city].join("."), ["key", state].join("."), "key"], {arg: "value"});

This is way cleaner.

How It Works
  1. translate() now officially expects that the key argument is now actually a potentialKeys argument that is an Array<String>. If potentialKeys is actually a String according to legacy behavior, it just converts it to a single-entry Array of the String that was provided.
  2. After processing the options, but before processing anything else, it loops through the array setting key to the one currently being tested. If that key exists we break and translate() continues on as normal. In the case that none of the potentialKeys are found, the loop ends with key being set to the last potentialKey we tried. In either scenario i18next behaves the way it always did, it just either uses the first key that exists, or the last key when none exist.
Risks & Potential Problems
  1. I might have broke something.
  2. Right now I have no test coverage for if an empty array is provided. The behavior is the same as if a null/undefined key is provided, however.
  3. This implementation assumes that the same options apply to every potentialKey. Because of this, you must supply every option that any potentialKey may want to use, even if some of them don't expect/want it. This works fine for interpolation, but would not work nicely if one key wants to be returned as an object tree and another doesn't. As far as I am concerned this is a completely acceptable tradeoff, if you are doing something like that you are probably being a jerk and should write some custom code.

Thanks for considering this pull request and please let me know if anything about it is incorrect or stupid.

@jamuhl
Member
jamuhl commented Sep 23, 2013

let me merge this and see what happens ;)

@jamuhl jamuhl merged commit 9f38c8f into i18next:master Sep 23, 2013
@jamuhl
Member
jamuhl commented Sep 23, 2013

up in v1.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment