Providing client-defined locale string validation and re-writing

[mesr]

I would like to propose and, if approved, offer to implement client-defined locale string validation and re-writing.

With the current implementation, the active locale can be set to any string ­— even an empty string — with no regard to whether or not that string is valid and the corresponding locale defined. Furthermore, when the active locale is set based on e.g. the value of a URL segment (such as the lang query-string parameter), this gives end-users direct control over the internal state of the I18n module with no opportunity for client code to intercept and modify that behaviour. Ultimately, validation of locale strings entirely relies on client code outside of the I18n module.

While it makes sense to perform client-defined validation, the I18n module should provide an opportunity to perform such validation before switching locale. This would provide client code with the ability to customize the validation of language strings to their need and implement behaviours such as language lookup.

This update would involve:

• Adding a hook point that would call function I18n.whenSwitchingLocale(locale) prior to switching locale and, in a fashion analogous to the new client-customizable locale detection facility, support as return values locale strings as well as promises that resolve to a locale string;
• [optional, see comment below] Adding functions I18n.addLocale(), I18n.removeLocale() and I18n.getDefinedLocales() for dynamically updating and obtaining the list of defined locales;
• [optional, see comment below] Renaming the I18n.pluralizationRules instance property to I18n.definedLocales to reflect its now wider responsibility (where defined locales with no pluralization rules would be listed with an empty object for value).

Compatibility with the current API would be entirely preserved, as the I18n.whenSwitchingLocale(locale) default implementation would simply return the new locale string as-is, thereby performing no validation and allowing switching to undefined locales.

Types, tests and documentation would be updated accordingly.

What would be the maintainer's position on such a feature? Any additional suggestions?

[mesr]

On second thought, the second and third point above could be left out of the I18n module and easily implemented in the client code. This would keep the I18n module's API lighter.

[ducksoupdev]

Implemented with #54

[mesr]

Actually, this proposal is still up for discussion; proposal #53 was implemented with PR #54.

[mesr]

This is what the proposed changes would look like (file src/i18n.mjs):

class I18n {
  /* ... */

  // The current locale would now remain the same when passed a non-string value
  setLocale (locale) {
    const newLocale = this?.whenSwitchingLocale(locale) || locale
    if (typeof newLocale === 'string') {
      this.currentLocale = newLocale
    }
    return this
  }

  // A default implementation of this hook is not required but might be
  // desirable for documentation purposes
  whenSwitchingLocale (locale) {
    return locale
  }

  /* ... */
}

There is one notable point of departure in the code from the proposal above: the setLocale(locale) function would now prevent switching the current locale to a non-string values. This is for better integration with the locale detection system, so that custom implementations of the proposed whenSwitchingLocale(locale) hook would have the opportunity to conditionally prevent switching.

Yet, the question from proposal #53 remains: should switching locale be prevented on non-string values or on falsy values?

Please let me know what you think.

[mlevy-liftoff]

I like your proposal @mesr and would prefer that preventing locale switching can be done by returning a falsely value.

[mesr]

On second thought, I think that I too prefer that switching be prevented on falsely value. But then, for consistency, I should probably update the locale detection system to make it skip the user-defined rule on falsely values as well (it's currently skipping it on non-string values).

[mesr]

@ducksoupdev, I'd like to discuss further the above question, if you don't mind. Even though it might seem like a very small detail to keep poking at, I think it's important to get it right from the start because it is part of the public API.

Preventing locale updates on falsely values is neat from the perspective of internal programming because, obviously, it makes for very simple test conditions (i.e. if (newLocale) /* switch */ else /* do nothing */). The problem that I see, now that I'm implementing the tests, is that it ends up creating non-intuitive discontinuities in the acceptable locale values, for instance:

• all numbers are accepted and converted to a string, except 0
• true is accepted and converted to a string, but not false
• all strings are accepted but not '' (although this one already feels more intuitive)

There is a third option that I now believe would keep the API simpler and more intuitive, and that is preventing switching only on null and undefined.

What would be your thoughts on that?

[ducksoupdev]

I can see the problem. Returning a null or undefined would be a simple but good solution to the API 👍

[mesr]

Submitted PR #57