Project

General

Profile

Actions
Copy link

Patch #31384

closed

Remove custom lazy loading of i18n files

Added by Pavel Rosický over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Jean-Philippe Lang
Category:
Code cleanup/refactoring
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:

Description

i18n gem was updated in #29946 (4.1.0)

this caused a regression with these translations

     numbers:
       '1': One
       '2': Two
       '3': Three

     I18n.t('1', scope: [:numbers])

     Failure/Error: _key = _key.to_sym
     NoMethodError:
       undefined method `to_sym' for 2:Integer
       Did you mean?  to_s
     # ./lib/redmine/i18n.rb:206:in `block in lookup'
     # ./lib/redmine/i18n.rb:205:in `each'
     # ./lib/redmine/i18n.rb:205:in `inject'
     # ./lib/redmine/i18n.rb:205:in `lookup'
     # /usr/local/rvm/gems/ruby-2.5.3-railsexpress/gems/i18n-1.6.0/lib/i18n/backend/base.rb:32:in `translate'

the problem is in redmine, because it uses a custom backend that overrides the original behaviour and it wasn't updated accordingly

see the original PR https://github.com/ruby-i18n/i18n/pull/422
also this change doesn't work https://github.com/ruby-i18n/i18n/pull/469

attached patch fixes the problem by removing most of the custom backend in redmine

Files

i18n.rb.patch (2.85 KB) i18n.rb.patch Pavel Rosický, 2019-05-17 16:38

Related issues

Related to Redmine - Feature #36728: Reintroduce lazy loading of i18n filesClosedMarius BĂLTEANU

Actions
Blocks Redmine - Feature #29946: Update i18n gem (~> 1.6.0)ClosedGo MAEDA

Actions
Actions
Copy link
 #1

Updated by Go MAEDA over 5 years ago

Actions
Copy link
 #2

Updated by Go MAEDA over 5 years ago

  • Target version set to 4.1.0
Actions
Copy link
 #3

Updated by Go MAEDA over 5 years ago

Actions
Copy link
 #4

Updated by Go MAEDA over 5 years ago

Actions
Copy link
 #5

Updated by Go MAEDA over 5 years ago

  • Target version deleted (4.1.0)
Actions
Copy link
 #6

Updated by Marius BĂLTEANU over 5 years ago

  • Assignee set to Jean-Philippe Lang
  • Target version set to 4.1.0

Jean-Philippe, please take a look at the patch proposed by Pavel as a fix to a problem that was introduced by me when we updated the i8n gem to 1.6.0 (#29946).

For me, the change looks good, especially because we drop quite a lot of custom code.

Actions
Copy link
 #7

Updated by Jean-Philippe Lang over 5 years ago

Marius BALTEANU wrote:

For me, the change looks good, especially because we drop quite a lot of custom code.

Sure but it also removes the feature that this custom code provides :-)

Actions
Copy link
 #8

Updated by Jean-Philippe Lang over 5 years ago

  • Subject changed from fix type-casted translation keys to Remove custom lazy loading of i18n files
  • Category set to Code cleanup/refactoring
  • Status changed from New to Closed
  • Target version deleted (4.1.0)

Patch committed.

Actions
Copy link
 #9

Updated by Pavel Rosický over 5 years ago

thanks for the feedback. You're right, i18n gem provides lazy loading for translations, but it loads all available translations at once.

the original redmine backend loaded only the current locale and this optimization was removed.

here's a benchmark on a clean redmine instance (without plugins)

  1.859000   1.515000   3.374000 (  3.379579) - load all locales
  0.079000   0.063000   0.142000 (  0.141866) - load the current locale (english)

on production all translations should be eager loaded, but on development the first request time is now significantly worse. Do you think it's worth to restore the optimization or improve it in the i18n gem?

Actions
Copy link
 #10

Updated by Go MAEDA over 2 years ago

  • Related to Feature #36728: Reintroduce lazy loading of i18n files added
Actions
Copy link

Also available in: Atom PDF