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

Internationalization (I18n) #610

Closed
jodosha opened this Issue Jul 12, 2016 · 37 comments

Comments

Projects
None yet
8 participants
@jodosha
Member

jodosha commented Jul 12, 2016

Introduce I18n features:

  • Add i18n gem as dependency to hanami.gemspec
  • Generate new projects with default translation file that uses :en locale (this is useful for lib/). It could be placed under config/locales/en.yml.
  • Generate new apps with default translation file that uses :en locale. It could be placed under apps/web/config/locales/en.yml.
  • Hanami boot process should include these files in I18n load paths (I18n.load_path.concat)
  • Within full-stack Hanami projects validations (from hanami-validations) should use :i18n messages engine instead of the default :yaml. The files generated above should be used by developers to indicate validation failures.
  • Introduce Hanami::Helpers::I18n to ship with a public method #translate, (aliased as #t), to be available in views and templates. This should be a shortcut to I18n.t.

@jodosha jodosha added this to the v0.9.0 milestone Jul 12, 2016

@joneslee85

This comment has been minimized.

Member

joneslee85 commented Jul 13, 2016

@jodosha have you ever thought about letting the browser (frontend side) to handle i18n? It is not that your proposal is not good, in fact it is quite good and proven (in Rails and elsewhere). I just want to explore new ways of achieving it that is closer to the frontend-heavy paradigm nowaday. I am thinking of a translation middleware that server translation API and frontend would consume the API endpoints and apply translation on the fly. Ofcourse there are certain caveats in caching and security. I guess it's worth to bring it up anyway.

@cllns

This comment has been minimized.

Member

cllns commented Jul 13, 2016

Glad to see internationalization making it into hanami pre-1.0! Happy to help out.

I'm a little concerned about using the i18n gem, since it doesn't seem terribly well maintained. Also I remember @pascalbetz saying using it was a bit rough.

It also monkey patches Hash. How will that be resolved?

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Jul 13, 2016

@joneslee85 interesting approach. It would involve JS on the frontend, right? What if you have just an API which serves i18n (error) messages to the client (not necessarily a browser)?

@cllns Probably I meant using i18n is a bit rough in the sense of using it with Hanami (Lotus, back then) is difficult because it does not integrate that well. If I remember correctly:-)

How about we start with the i18n gem but make sure our own interface is used?

Also localization (i10n) should also be considered. Date/Time/NUmber formats, Currencies...

@jodosha

This comment has been minimized.

Member

jodosha commented Jul 13, 2016

@joneslee85

@jodosha have you ever thought about letting the browser (frontend side) to handle i18n?

I haven't considered this scenario, but I'd discard it for now. This front-end scenario is an advanced use case which doesn't make sense to implement if we first don't have basic use case in place. In other words, server side first.

Please remember that until now we don't have any JS support out of the box. Using JS for that, would mean to implement that JS support first, by delaying I18n.

Betting on front-end for I18n makes the assumption that a JS engine is always available. This isn't true for several cases, think of browsers with disabled JS, or error messages sent over HTTP JSON API.


I am thinking of a translation middleware that server translation API and frontend would consume the API endpoints and apply translation on the fly. Ofcourse there are certain caveats in caching and security.

This sounds like a third party gem. For instance, I wrote something similar for Rails a few years ago: https://github.com/jodosha/rails-i18n-js

@jodosha

This comment has been minimized.

Member

jodosha commented Jul 13, 2016

@cllns

I'm a little concerned about using the i18n gem, since it doesn't seem terribly well maintained.

It's a standard in Ruby world, there aren't better alternatives (as far as I know), and we already support it with hanami-validations (master), because of dry-validation. 😉

@jodosha

This comment has been minimized.

Member

jodosha commented Jul 13, 2016

@pascalbetz

How about we start with the i18n gem but make sure our own interface is used?

Where, specifically? In views/templates we should use the new helpers mentioned in the description. For all the other contexts, I think that referencing directly I18n.t isn't that bad. Or we can ship Hanami::I18n, but I want to avoid verbosity 😉 .

Also localization (i10n) should also be considered. Date/Time/NUmber formats, Currencies...

I intentionally didn't included l10n, because the i18n gem requires configuration files (YAML) to lookup for date, time, numbers labels. See en.yml from rails-i18n as an example: https://github.com/svenfuchs/rails-i18n/blob/master/rails/locale/en.yml

This task will require a huge effort to organize that data. This is the biggest challenge for l10n for us. When we will have data in place it should be easy to write an API around it.

I'm for shipping i18n first and then l10n.

@sakuro

This comment has been minimized.

Contributor

sakuro commented Jul 13, 2016

Did you consider gettext?

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Jul 13, 2016

@sakuro I don't know about the current state. The last time I used gettext it was horrible to get it working.

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Jul 13, 2016

To clarify: I used gettext maybe 7 years ago in a Rails project. Three different gems were required. I don't know anything about gettext of 2016.

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 5, 2016

@hanami/core-team any feedback?

Started implementing i18n. Some questions popped up:

Hierarchy of YAML files in container architecture
Keys are not isolated/scoped to the app they are defined in.

# apps/app_a/config/locales/en.yml
en:
  welcome: "Welcome to App A"

# apps/app_b/config/locals/en.yml
en:
  welcome: "Welcome to App B"

# Somewhere in a template:

<%= I18n.t('welcome') %>

Unless we implement some sort of hierarchy the load order will define what translation will be shown. Last one wins.
What is the desired/intended behavior?

Configuration

Do you intend (or does it already exist) to provide a configuration of I18n via Hanami or should developers use standard I18n config?

Sidenote
My usual rant about architecture: in order to implement this, we need to touch both architecture generators: app and container. I don't see any added value in app architecture.

@cllns

This comment has been minimized.

Member

cllns commented Aug 5, 2016

For the Hierarchy question, I think the files should be isloated/scoped to the app their defined in. Is there some reason this isn't feasible?
So <%= I18n.t('welcome') %> inside app_a will output "Welcome to App A" and
And <%= I18n.t('welcome') %> inside app_b will output "Welcome to App B".

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 5, 2016

@cllns not sure if this kind of scoping is supported out of the box by i18n. Since we work with static/self methods, I don't think there is a way to switch out sources depending on app context.

@cllns

This comment has been minimized.

Member

cllns commented Aug 5, 2016

Can we fake it then? Like make "Welcome to App A" equal to ::I18n.t('__app_a.welcome'), but within app_a, can we provide the API as Hanami::Helpers::I18n.t('welcome')? Just an idea.

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 5, 2016

probably this would work. Custom Backend perhaps. Let's see....

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 8, 2016

@pascalbetz Good catch!

My suggestion is to do something similar to Web::Routes: we create this constant at the runtime to reference the routes from an application named Web. Then the helper that we use for the views and templates references that constant.

So:

def routes
  Web::Routes
end

Can we think to have namespaced translation files:

# apps/web/config/locales/en.yml
en:
  web:
    welcome: "Welcome"
# apps/admin/config/locales/en.yml
en:
  admin:
    welcome: "Welcome to the admin panel"

Then at the runtime we create Web::I18n and Admin::I18n, which respond to .translate (aliased as .t).

Web::I18n.t('welcome') # => "Welcome"
# equivalent to
I18n.t('web.welcome') # => "Welcome"
# equivalent to
I18n.t('welcome', scope: 'web') # => "Welcome"

Then, Web::View should include a module that references the corresponding Web::I18n:

def t(*args)
  Web::View.t(*args)
end

Please have a look at:

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 8, 2016

Hello @jodosha

Thanks for the feedback. Having scoped YML files is probably the easiest way. But i'm not sure I like it.

  • Might confuse users. They are not used to scoping
  • You can still put translations for web into admin, as long as they contain the right scope
  • Classes created at runtime are harder to debug (where does it come from, where is the source code?)
  • You can access translations from other apps

In short, i'd really like if we could have the translations isolated. What do you think?

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 9, 2016

Some more thoughts:

  • what if users want to use scope for something internal to the application?
  • Should it be possible to use translations from admin in web ? Or should common translations rather be duplicated or go into the projects translations?
@jodosha

This comment has been minimized.

Member

jodosha commented Aug 9, 2016

Hi @pascalbetz !

Might confuse users. They are not used to scoping

If they can still use t('welcome') in a template, scoping is transparent.

Web::I18n.t('welcome') is required only outside views/templates (eg. an action).

You can still put translations for web into admin, as long as they contain the right scope

???

Classes created at runtime are harder to debug (where does it come from, where is the source code?)

Right, and I think we should remove this practice after 1.0, but until then it's consistent with the other constants (eg. routes)

You can access translations from other apps

Sure. Is this bad or good for you?

In short, i'd really like if we could have the translations isolated. What do you think?

That is great. What's your idea? 😄

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 9, 2016

@pascalbetz I have the impression that I poorly explained my idea.

My proposal is to scope the translations only in the YAML file. The API should make the scope transparent. For instance in views/templates/helpers we should make this to work: t('welcome'). As you can see there is not scope here.

Outside of that context (eg. actions, interactors), they should use Web::I18n.t('welcome'). Here there is the Ruby namespace Web that acts as kind of scope, but developers aren't using the scope: 'web' option, so they are free to specify it in case of need.

Does this clarify the intent?

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 9, 2016

@jodosha

Now I understand:-)

It will still allow users to do:

Web::I18n.t('welcome', scope: 'admin')

right? Not saying that this is bad. Just pointing out to understand the expectations.

For the implementation Web::I18n would need to specify the scope, right? Or do you want to switch backends on I18n? This would be possible as far as I can tell from a quick review of the I18n code. The configuration is thread local.

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 9, 2016

It will still allow users to do:

Web::I18n.t('welcome', scope: 'admin')

Yes.

For the implementation Web::I18n would need to specify the scope, right?

Yup, that is an internal information, that Web should store internally and pass down to I18n.t.

@pascalbetz pascalbetz referenced this issue Aug 9, 2016

Closed

I18n #632

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 9, 2016

Ok, i came up with something (yet basic though and WIP but should help for discussions).

Web::I18n would be an instance of this:
https://github.com/pascalbetz/hanami-hanami/blob/1c5983328694111ea93ca9c062b9c7ab68e63c60/lib/hanami/i18n/i18n_with_application_scope.rb

I try scoped to the application first, then unscoped. I think this would be usefule as it provides some sort of fallback:

  • try application translations first
  • try project translations then

If nothing is found, then the error message will report the unscoped key as missing.

@jodosha what do you think?

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 9, 2016

@pascalbetz Yes, I like the idea of the fallback. IIRC I18n should support this kind of scenario. If not, I think it's a good idea to implement our own.

@k-solutions

This comment has been minimized.

k-solutions commented Aug 9, 2016

Could we consider isolating I18n in its own service?

On Aug 9, 2016 13:48, "Luca Guidi" notifications@github.com wrote:

@pascalbetz https://github.com/pascalbetz Yes, I like the idea of the
fallback. IIRC I18n should support this kind of scenario. If not, I think
it's a good idea to implement our own.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#610 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAOrfVQAmJbum_u2IqcH2ioVihVCZurgks5qeFr_gaJpZM4JKQEM
.

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 9, 2016

@k-solutions can you explain a bit more?

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 9, 2016

@jodosha I18n provides a cascading backend, but AFAIK it does not "try with and without scope". Anybody?

@k-solutions

This comment has been minimized.

k-solutions commented Aug 9, 2016

Well, it could live outside of application and could have a common API and
each application could call it from whatever frontend templates or backend
templates and under it could be implemented with I18n or whatever developer
prefer , for example gettext or any upcoming solution.

Regards,
Hristo Kochev

On 9 Aug 2016 6:37 p.m., "Pascal Betz" notifications@github.com wrote:

@k-solutions https://github.com/k-solutions can you explain a bit more?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#610 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAOrfSXp3ZsOb3fVi9PfkEdg-pu75viIks5qeJ6ggaJpZM4JKQEM
.

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 10, 2016

@k-solutions To clarify, are you suggesting a microservice for I18n, to live on its own process?

@k-solutions

This comment has been minimized.

k-solutions commented Aug 10, 2016

From any application point I18n is outside there business domain, so it's
make perfect sense to me. Obviously if it need to be front-end accessible
we need to expose it as microservice, yes. For the back end template engine
it may be as simple as Rack Middleware, but my main point is to define a
clear API, which will hide actual implementation and then make this
implementation configurable and versatile as possible.

On Aug 10, 2016 9:52 AM, "Luca Guidi" notifications@github.com wrote:

@k-solutions https://github.com/k-solutions To clarify, are you
suggesting a microservice for I18n, to live on its own process?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#610 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAOrfaVDOCeW3iNhMHEoBNzXVULHU_nXks5qeXUtgaJpZM4JKQEM
.

@pascalbetz

This comment has been minimized.

Contributor

pascalbetz commented Aug 10, 2016

@k-solutions

The intent is to use I18n gem. It provides different backends (YAML, Gettext PO Files, DB, ...) and will be hidden through

Web::I18n.translate # or just .t, for short
Admin::I18n.translate
# and so on, one for each app in your hanami project

What do you think?

Can you detail how the Rack middleware would look like/work?

Thanks for your input!

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 10, 2016

@k-solutions A microservice is out of scope: it complicates deployment and introduces performances penalties because translations should be communicated down to the wire rather than accessed from memory.

A Rack middleware is not viable too, because translations would only be available in web contexts, while we aim to make them accessible from all the parts of an application (eg. an entity).

@k-solutions

This comment has been minimized.

k-solutions commented Aug 11, 2016

Well then we left with Rails approach to integrate I18n deep into framework
...at least consider make it optimal ....

Side note ...not sure it make sense for business use case to have I18n on
entities ...

On Aug 10, 2016 1:24 PM, "Luca Guidi" notifications@github.com wrote:

@k-solutions https://github.com/k-solutions A microservice is out of
scope: it complicates deployment and introduces performances penalties
because translations should be communicated down to the wire rather than
accessed from memory.

A Rack middleware is not viable too, because translations would only be
available in web contexts, while we aim to make them accessible from all
the parts of an application (eg. an entity).


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#610 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAOrfd5GiyvbryFIc8i2tJ5kBVwSQ5grks5qeabtgaJpZM4JKQEM
.

@jodosha jodosha removed this from the v0.9.0 milestone Oct 24, 2016

@jodosha

This comment has been minimized.

Member

jodosha commented Nov 29, 2016

I'm closing this for now, but I'd like to resume the discussion with all of you after 1.0.

@jodosha jodosha closed this Nov 29, 2016

@pmichna

This comment has been minimized.

pmichna commented Aug 16, 2017

@jodosha

I'm closing this for now, but I'd like to resume the discussion with all of you after 1.0.

I guess the time has come. ;)

@jc00ke

This comment has been minimized.

Contributor

jc00ke commented Aug 9, 2018

@jodosha Now that we're getting close to v2.0 is it time to revisit this? I'm trying to get I18n to work in a Hanami app and I'm running into issues with dry-validations rules and error messages. This seems like a good time to revisit i18n if it hasn't already been addressed (maybe it has and I missed it?)

@jodosha

This comment has been minimized.

Member

jodosha commented Aug 10, 2018

@jc00ke Which specific errors did you get in for validation error messages? Is it worth to open a ticket in hanami-validations or dry-validation?

Is there anything else you think we should consider?

@jc00ke

This comment has been minimized.

Contributor

jc00ke commented Aug 10, 2018

Sure, I can open an issue in hanami-validations but I still think it's worth shoring up i18n in Hanami apps in general. It could be as simple as docs in the guide. I found it to be trickier than expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment