Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude MissingTranslation options that are not used by the instance #581

Merged
merged 1 commit into from Jan 26, 2022

Conversation

sundling
Copy link

@sundling sundling commented Oct 11, 2021

Fixes #580

Instances of MissingTranslation are currently initialized with the options originally passed to I18n.translate.
When using ActiveRecord those options will include a reference to the instance.
And when combined with a (Rails) cache store, that MissingTranslation instance is now expected to work with Marshal.dump.

The only MissingTranslation option being used appears to be :scope; when converting the instance to a string, so that key is kept.

@radar
Copy link
Collaborator

radar commented Oct 11, 2021

Thanks for this PR @sundling. Could you please add a description to show what drove this change?

@radar radar merged commit 92cef5e into ruby-i18n:master Jan 26, 2022
@simonlevasseur
Copy link

This PR recently introduced an issue for us in a scenario where we were catching MissingTranslation errors and re-invoking I18n.translate with a different locale but reusing the key and options. Doing that now results in a translation without the proper variables for interpolation.

I think from a convention POV, it makes sense to pass the options back via the exception. The bug that we are trying to fix in this PR is pretty specific to Rails/ActiveRecord but the change was made globally which is unexpected.

Can we revert and apply a more narrow fix in ActionRecord?

@radar
Copy link
Collaborator

radar commented Feb 3, 2022

@simonlevasseur Yes, I think that would be a sensible approach here.

@simonlevasseur
Copy link

Just to clarify, do you want me to revert this PR or do you plan on doing it?

@radar
Copy link
Collaborator

radar commented Feb 13, 2022

@simonlevasseur could you please look into it?

@xtreme-shane-lattanzio
Copy link

I think we may be running into issues with this as well. Since 1.9.0 we are getting I18n::InvalidLocaleData: can not load translations from /usr/share/rvm/gems/ruby-2.7.1/gems/activesupport-6.0.4.4/lib/active_support/locale/en.yml: #<ArgumentError: wrong number of arguments (given 2, expected 1)>

@radar
Copy link
Collaborator

radar commented Feb 24, 2022

@xtreme-shane-lattanzio please open a new issue here with a link to a repo that reproduces this issue

@xtreme-shane-lattanzio
Copy link

I think something was weird with including an of safe_yaml gem. We removed it and the error went away since we should be using YAML.safe_load instead of SafeYAML.load anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility with Rails 6.1+ and validations containing procs/lambdas
4 participants