Closed radar closed 2 years ago
@jeffblake I've just confirmed in that test application that:
gem 'i18n', github: "ruby-i18n/i18n", branch: "617"
No longer makes this issue appear.
As it's currently the weekend (or soon to be) in most places of the world, and I am cautious about releasing a new version and disrupting people's weekends, I will be holding off on committing + releasing this one until Tuesday morning Australian Eastern Time.
It appears that the issue is with defaults
here being a single object, and not being duplicated. So when defaults.shift
is called, the defaults
in the options
hash is being changed also. If it was a duplicated object, we wouldn't have this problem.
This change to ActiveModel would also fix this issue, and allow for #591 to be added back into I18n:
diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb
index 72a3e43e71..2b7f02fd0b 100644
--- a/activemodel/lib/active_model/naming.rb
+++ b/activemodel/lib/active_model/naming.rb
@@ -205,7 +205,7 @@ def human(options = {})
defaults << options[:default] if options[:default]
defaults << @human
- options = { scope: [@klass.i18n_scope, :models], count: 1, default: defaults }.merge!(options.except(:default))
+ options = { scope: [@klass.i18n_scope, :models], count: 1, default: defaults.dup }.merge!(options.except(:default))
I18n.translate(defaults.shift, **options)
end
However, I no longer make contributions to Rails code itself, after bad interactions with a particular person on the core team.
Thanks @radar ! That branch fixes it.
However, I no longer make contributions to Rails code itself, after bad interactions with a particular person on the core team.
@radar Understandable. Sorry you had to go through that. I'll test out the fix and open a PR against Rails with that change.
@radar To be clear before I say other things: I'm fine with the revert, but disagree with that regression test as written. I've added a suggestion to fix it.
I reviewed the proposed fix to Rails, and while I agree that works as a workaround, I disagree that's the solution to the problem.
I think it makes sense that they don't provide the original Symbol
as part of the defaults
(i.e., the shift
modifying options['defaults']
is desired; even though their code could better express their intent. Indeed, they have since refactored that code and it's somewhat clearer now). They don't want to do the lookup of the initial value twice, since they shouldn't expect it to be any different the second time around.
resolve
is called in three different cases:Symbol
(Code)
defaults
are resolved. (Code)
defaults
before falling back through fallbacks
locales, keeping the behaviour as it has always been.Proc
(Code)
Proc
returns a Symbol
(current locale or the original locale)Prior to #591, there was no distinction between the cases. #591 should have introduced that distinction.
I'll look into what can be done and open a replacement to #591 (Update: replacement PR is here)
But it won't require a change in the caller's defaults
parameters.
@radar I've opened an alternative PR to this revert: https://github.com/ruby-i18n/i18n/pull/622
Looks like #622 is the go here. I'll be closing this one, and doing a release tomorrow AM including #622.
After #591, and with I18n v1.9.x versions, the fallbacks logic has broken. In this PR, I include a simple test to reproduce the issue as well as reverting #591.
cc @movermeyer / @jeffblake