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

Default i18n messages should not replace pre-existing keys #31

Closed
timriley opened this issue Dec 8, 2015 · 3 comments
Closed

Default i18n messages should not replace pre-existing keys #31

timriley opened this issue Dec 8, 2015 · 3 comments

Comments

@timriley
Copy link
Member

timriley commented Dec 8, 2015

If you have an app with i18n configured, and some local i18n keys to replace dry-validation message defaults, then later requiring dry-validation will overwrite those keys.

For example, if I set up i18n like this, in the boot sequence of my app:

require "i18n"
I18n.load_path += Dir["#{MyApp::Container.root}/apps/*/locales/**/*.yml"]
I18n.backend.load_translations

And where I have a .yml file like this:

en:
  errors:
    filled?: "Fill this thing, yo"

And then later on require a file that requires dry-validation, the errors.filled? key gets replaced with the default:

[1] pry(main)> I18n.t("errors.filled?")
=> "%{name} must be filled"

If, instead, I do a little hoop-jumping and force dry-v's i18n integration to load before I suppose my own locale files, then things work as I'd want:

require "i18n"

# Load dry-validation's i18n support first, so we can override if we want
require "dry/validation/messages/i18n"

I18n.load_path += Dir["#{Alpinist::Container.root}/apps/*/locales/**/*.yml"]

I18n.backend.load_translations
[1] pry(main)> I18n.t("errors.filled?")
=> "Fill this thing, yo"

You can see these examples in action in https://github.com/icelab/alpinist/tree/sign-in-and-users-crud.

I think the preferred behaviour here would be for dry-v not to do anything that might result in pre-existing keys being overwritten.

@solnic
Copy link
Member

solnic commented Dec 8, 2015

I'm not sure how to solve this to be honest. Does it work as expected when you unshift load path rather than append?

@solnic solnic closed this as completed in 0bde77d Dec 8, 2015
solnic added a commit that referenced this issue Dec 8, 2015
…I18n#merge

It makes no sense to have the merge interface since it mutates global
I18 state anyway.

This means the only way to override built-in errors is to:

  require "i18n"
  require "dry-validation"

  I18n.load_path << path_to_your_custom_translations

Refs #31
@solnic
Copy link
Member

solnic commented Dec 8, 2015

Sorry Tim but there's no way around that.

@timriley
Copy link
Member Author

timriley commented Dec 8, 2015

@solnic Fair enough! Thanks for looking into it.

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

No branches or pull requests

2 participants