Allow to prevent calls outside of I18n context by setting locale to false

[casperisfine]

Context

We recently discovered a common error pattern in our application. I18n.t and similar API would get called during the boot phase, and the result stored in constant or similar class level variables. e.g.:

class Foo < ActiveRecord::Base
  validates :bar, presence: { message: I18n.t('foo.bar.missing') }
end

Of course this is a simple programming mistake, but I believe it could be prevented with a relatively simple change.

Solution

By allowing frameworks like Rails to set I18n.locale = false early in the boot phase, and whenever they load code, it could catch this type of mistake early.

That safety is totally opt-in, so it wouldn't break existing usage.

Also if for some reason you need to access a translation during those phases, you can explicitly specify the locale you want:

I18n.with_locale(false) { I18n.t('foo.bar', locale: 'en') }

Alternative solution

For now we've implemented a similar feature by simply setting a fake I18n backend. However since the locale selection is done inside I18n itself, the backend can't know wether the locale was passed explicitly or not, so it's hard to opt-out of that safety in the few case you need it.

cc @rafaelfranca

@radar what do you think?

[rafaelfranca]

I like the plan. I believe those kind of mistakes are easy to do and having protection around it in the framework.

[radar]

Activerecord already does validation messages without having to specify the message like this. See section 4.5 here: https://guides.rubyonrails.org/i18n.html

[rafaelfranca]

This is not about Active Record. It is about any code that calls I18n.t at load time and in really are just mistakes.

Things like:

class Bar
  SOME_ERROR_MESSAGE = I18n.t('something')

class Foo
  class_attribute :name, default: I18n.t('name')

[radar]

Define these as class methods instead so they’re evaluated on call rather than on compilation.

I am loathe to further complicate or add features to the i18n codebase.

[casperisfine]

Define these as class methods instead so they’re evaluated on call rather than on compilation.

Yes, there is way to do this properly. The point here is not to allow something that was impossible before, but to eagerly catch programmers mistakes.

[rafaelfranca]

This is not a code to allow strange usage of I18n. It is a feature to direct people with less experience on how Ruby and I18n work to the right direction.

This came out from one real application where people called I18n.t on compilation. Code review didn't catch. The feature stayed on production only in english for days. All of this could be avoided if I18n had a way to disallow calls on compile time.

I understand and agree with the hesitation to add new feature to this codebase, but I believe this one will help many experienced and non-experienced developers to catch real problems earlier.

@radar can you please reconsider in this optic?

[radar]

@rafaelfranca I can definitely see the advantages of that. I think setting I18n.locale = false is a decent enough workaround for this issue and, from what I can tell, it will raise exceptions in the case where people are incorrectly using it.

One last thing: I do wonder if the MissingLocale error message is useful enough to developers who encounter this issue. Think about a junior developer who might see this error message for their first-ever time. Does "No locale specified" really convey enough information to them? How can they discover how to remedy this exception?

Maybe we could make that exception's default error message clearer? Something like "I18n.locale is set to false, but must be configured to be a locale symbol. <list available locales here?>".

I am not sure entirely. What do you think would be useful error message to display to a junior developer? I think "No locale specified" is a good start, but we could probably do a better "Developer UX" here.

[rafaelfranca]

This was the image we used in our custom backend:

        Can't call I18n.#{method} before the application is fully initialized.
        I18n.#{method} is meant to display text in the user locale, so by invoking it during the boot phase
        you risk leaking english strings to non-english speaking users.

The context on our implementation is different so I don't think this message fully apply to this patch.

When you do I18n.locale = false is not as obvious that we want to disable translations on boot, so maybe we can have something like:

        Can't call I18n.#{method} while I18n is disabled. It is usually disabled when loading your Ruby
        code.

        I18n.#{method} is meant to display text in the user locale, so by invoking it during the code
        loading phase you risk leaking english strings to non-english speaking users.

We can complete the message with explanation on how to solve some of those cases and how to opt-out the protection in case you really want to use translated text when loading the code.

[casperisfine]

I indeed didn't port the exception message as is, because it makes some assuptions about the default locale etc, that might not hold in raw I18n.

I'll try to come up with something better though.

[casperisfine]

I just updated the exception name and error message, let me know how it looks.

[casperisfine]

I also added this. While crafting the error message I figured that exists? was inconsistent with the others, as it would take a locale as second param instead of a named one.

Having all methods accepting it as named param allows for a more insightful error message.

[paracycle]

Suggested change

	I18n.#{method} is currently disabled, likely because your application is still in it's loading phase.
	I18n.#{method} is currently disabled, likely because your application is still in its loading phase.

[casperisfine]

Changed. Thanks!

[radar]

Error message looks great. Thanks for taking the time to update that. I will merge this now, and do a new release tomorrow AM (Eastern Australian time)