i18n icon indicating copy to clipboard operation
i18n copied to clipboard

Introduce support for deprecating translation keys

Open aldesantis opened this issue 6 years ago • 1 comments

This was born out of the discussion on https://github.com/solidusio/solidus_auth_devise/pull/173. It was originally started by @kennyadsl and completed by me.

The rationale and logic are explained in the commit message for future reference.

Please note that it's very experimental and we understand it may be too complex and/or opinionated to be introduced into I18n directly. We're happy to accept any change requests or even to make it a separate gem if you feel like I18n is not a good place for it.

It's still a bit rough at the edges. If this sparks enough interest, we'd like to:

  • Move the translate patches to Deprecator. This would be much cleaner, but it has some performance drawbacks, mainly that we'd need to call lookup in Deprecator just to check if the keys are defined, but then still delegate to super for the rest of the logic, and super would call lookup again.
  • Improve the deprecation warnings by using a proper logger and/or a more configurable approach.
  • Add API and wiki documentation.
  • Maybe allow people to turn the behavior on/off entirely? Not everyone may like the idea of their translation keys being overridden automatically. However, this might create more problems than it solves, so we're not sure at this point.

Hopefully, the current state of it is enough for you to get an idea!

aldesantis avatar Sep 25 '19 12:09 aldesantis

Hi

I like the idea to handle deprecation on translations. But I think we can have a simpler approach. Instead of adding another layer in the inheritance stack, I think we can take benefit of actual I18n functionality. We can have several backend using I18n::Backend::Chain, and I18n will fallback from I18n::Backend::Deprecator to I18n::Backend::Simple or anything else.

We can do it with this kind of implementation:

module I18n
  module Backend
    class Deprecator

      def store_translations(locale, data, options = EMPTY_HASH)
        ...
      end

      def available_locales
        ...
      end

      def lookup(locale, key, scope = [], options = EMPTY_HASH)
        key = find_the_key_in_the_dedicated_store
        Warning.warn("Deprecated key #{key}") if key
        return nil # Always return nil to get the fallback on I18n::Backend::Simple or other backend in the chain.
      end
    end
  end
end

then I18n.backend = I18n::Backend::Chain.new(I18n::Backend::Deprecator.new, I18n.backend)

This way, we won't change anything in I18n::Backend::Base. What do you think about it?

papa-cool avatar May 21 '21 10:05 papa-cool