Open discussion about i18next

daniloster commented 4 years ago

From where it came: https://github.com/daniloster/svelte-i18n/pull/8

daniloster commented 4 years ago

I have copied the initial comments in the PR above, so we can follow within context.

Quoting @adrai

How would an i18next example look like when loading translations asynchronously i.e. With i18next-http-backend ? Is there something like hasNamespaceLoaded? https://github.com/i18next/react-i18next/blob/master/src/utils.js#L43 not to render (or render a fallback ui) until the namespace is loaded?

for example, when using an i18next config like this:

I think something like this should be done somewhere: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L32 but I have no idea how... 🤷‍♂️

daniloster commented 4 years ago

I am going to add a better example. But, roughly it should be like.

i18nextState.js

import i18n from 'i18next'
import HttpApi from 'i18next-http-backend'
import { writable } from 'svelte/store'
import { factoryI18nextState } from '../src'

export const i18nAsyncStatus = writable('Loading')

i18n.init({
  fallbackLng: 'en',
  languages: ['en', 'pt-BR'],
  lng: 'en',
  backend: {
    loadPath: '/assets/locales/{{lng}}.json',
  },
}).then(() => {
  // initialized and ready to go!
  i18nAsyncStatus.set('Success')
}).catch(() => {
  i18nAsyncStatus.set('Error')
})

const i18nState = factoryI18nextState({
  i18n,
  persistence: {
    get: () => localStorage.getItem('locale'),
    set: (locale) => localStorage.getItem('locale', locale),
  },
})

export default i18nState

Then, we could use the i18nAsyncStatus in the App to conditionally display what is expected.

daniloster commented 4 years ago

As I said, I am going to add a better example explaining it in the GETTING_STARTED.md. I am writing that without an editor (IDE), so I could be possibly miss something. Just wanted to give an idea how roughly the scenario could be handled.

I am really thankful for all the feedback. cc @adrai

adrai commented 4 years ago

This would only handle the loading of preloading languages and namespces... In i18next languages and namespces can be loded on request (after the init)

daniloster commented 4 years ago

Yeah, I will need to investigate it better to understand if we can intercept the requests to infer when a loading started and when it finished.

daniloster commented 4 years ago

I think we can use the custom request. What do you think?

  // define a custom request function
  // can be used to support XDomainRequest in IE 8 and 9
  //
  // 'options' will be this entire options object
  // 'url' will be passed the value of 'loadPath'
  // 'payload' will be a key:value object used when saving missing translations
  // 'callback' is a function that takes two parameters, 'err' and 'res'.
  //            'err' should be an error
  //            'res' should be an object with a 'status' property and a 'data' property the key:value translation pairs for the
  //            requested language and namespace, or null in case of an error.
  request: function (options, url, payload, callback) {},

adrai commented 4 years ago

I don’t know how this could help... and btw: this wouldn’t be useful if using a different backend... like i18next-fs-backend or i18next-locize-backend...?

daniloster commented 4 years ago

I understand your point.

If i18next allows to different plugins to load asynchronously the resources (which is awesome), it should provide an interface that allow to intercept/listen events. I could not find one yet.

I don't believe this lib should be doing some similar to https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L32

This goes beyond the scope of the library, I believe. However, I will still look up the i18next to understand if it is possible to have these events/interceptors.

daniloster commented 4 years ago

I might have found a way. https://github.com/i18next/i18next-chained-backend/blob/master/src/index.js#L26-L45 This library chain execution of backends. I can try to add a PR to this to allow listen events on read (loading, success, error).

adrai commented 4 years ago

There is a onLoaded event in i18next... https://www.i18next.com/overview/api#onloaded But I don’t think this is the right way... might ask a fried 😉 @jamuhl

daniloster commented 4 years ago

There is a onLoaded event in i18next... https://www.i18next.com/overview/api#onloaded But I don’t think this is the right way... might ask a fried 😉 @jamuhl

Awesome, it seems that is exactly that.

i18next.on('languageChanged', function(lng) {}) i18next.on('loaded', function(loaded) {}) i18next.on('failedLoading', function(lng, ns, msg) {})

These 3 events should sort it out. Thank you for helping out.

adrai commented 4 years ago

Probably this will not work, when lazy loading namespaces.... (but I didn't check) I suspect to solve this you need to intercept if the passed namespace is already loaded (https://www.i18next.com/overview/api#hasresourcebundle) and if not load it with the help of https://www.i18next.com/overview/api#loadnamespaces

and for new languages just calling i18n.changeLanguage(lng, () => /* rerender */) (https://www.i18next.com/overview/api#changelanguage) should work...

so with this ⬆️, there should not be the need to listen for those loaded events

@jamuhl, I hope this is correct what I'm saying ;-)

adrai commented 4 years ago

So I would suggest to first handle the lazy loading of a new language (changeLanguage) And secondly, optionally lazy loading the namespace... there are i18next libraries out there not offering this too... i.e. jquery-i18next

daniloster commented 4 years ago

I am still thinking about the concerns of each part (this lib and the i18next). Honestly, I think that should have events/interceptors for async calls happening inside the library i18next.

I need to think better about it to not introduce an unnecessary breaking change.

I really appreciate all the help ❤️ .

adrai commented 4 years ago

As there is no automatic lazy loading of namespaces in i18next (only languages), theese are all things you may need:

https://www.i18next.com/overview/api#changelanguage https://www.i18next.com/overview/api#onlanguagechanged

https://www.i18next.com/overview/api#hasresourcebundle https://www.i18next.com/overview/api#loadnamespaces

https://www.i18next.com/overview/api#oninitialized

fyi: react-i18next offers a lazy loading of namespaces, i18next not.

jamuhl commented 4 years ago

what we do in react is basically (in a hook or HOC):

check if demanded namespaces are loaded: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L32 -> if ok --> ready

if not loaded loadNamespaces: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L52 -> callback --> ready

both functions are in i18next:

jamuhl commented 4 years ago

And bind rerender on (languageChanged, store events optional via config):

https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L62

daniloster commented 4 years ago

Thank you for the help. I will find some time to add these capabilities. Really appreciate all the help. 🥰

I am maturing the idea to make this changes decoupled from the core of the library. I don't want to add specificity unless it can be injected or used as decorator.

adrai commented 4 years ago

I’m curious to see how it will look like 👍

daniloster commented 4 years ago

@adrai , I found some time to create this PoC. I think it attends. But, I would like to understand a bit more about the namespaces. I am not sure if I got the concept correct. Also, the loading mechanism with the http plugin seems a bit off, It is loading for all the languages previously selected the new namespace set. I thought it would be on-demand, wouldn't it?

Thanks for helping again. And I hope to be on the right track.

daniloster commented 4 years ago

I still see it coupled. I will need to work a bit harder on it.

adrai commented 4 years ago

But, I would like to understand a bit more about the namespaces

Hehe, namespaces... Namespaces can be anything: https://www.i18next.com/principles/namespaces That's why I would not recommend setting it as default here: https://github.com/daniloster/svelte-i18n/pull/11/files#diff-968ac2f3714a9bb1a81b15286576127eR21

It is loading for all the languages previously selected the new namespace set.

Should only load for the current language and the fallbacklng... (all you need to properly resolve a key) If you remove i.e. the mobile namespace here: you should see it. fyi: when changing the language all namespaces defined in the init, will be loaded for that language

btw: I don't understand what you mean by: "it should not break the language which contains -. Once it is retried, it works."

I still see it coupled

Finally you have multiple options:

put everything in svelte-i18n (your current logic + i18next stuff)
create an additional module, which extends svelte-i18n with i18next stuff
create 2 completely different i18n modules: svelte-i18n and svelte-i18next
svelte-i18n being completely i18next based and be an official i18next module for svelte listed here ;-) I think if doing so, offering a good svelte i18next capability, It could be someone (probably locize) could offer to help you with a Patreon sponsoring or similar...

Good job so far 👍

daniloster commented 4 years ago

Thank @adrai for quickly feedback.

I could not find a way to set which namespace will be used to resolve keys when changing language.

I don't understand what you mean by: "it should not break the language which contains -. Once it is retried, it works.

When I change language to 'pt-BR' having the URL pattern as assets/locales/{{lng}}/{{ns}}.json. The http plugin try to fetch for 'pt-BR' and 'pt'. But, I don't have resources for only 'pt'. Then, the fetch call fails with 404. I was expecting only to fetch for the language O have set 'pt-BR'.

adrai commented 4 years ago

try to call the t function with the namespace as prefix like in the link of the i18next docs I added in my previous comment.

regarding the loading... this is a normal behaviour. To tweak it use the load option: currentOnly

jamuhl commented 4 years ago

no overriding of load will load locale, language (pt-BR will load pt-BR and pt -> pt-BR can only contain what is different from regular pt) load: 'languageOnly' will load the language part of locale only (pt-BR -> it will load pt) load: 'currentOnly' will load just the one it got (pt-BR -> will load that)

daniloster commented 4 years ago

I completely understand if you don't have time, @adrai . Would you have 10 min to sync with me on a video call next Saturday (10am Dublin time)? My hangout is my <github_handle>@gmail.com

10:00 Saturday, in Dublin, County Dublin is 11:00 Saturday, in Switzerland

I am super confused about lazy-loading with namespace that should not be defined on the config, and at the same time is loaded asynchronously.

How the tool (this lib + i18next) would know what to load? Would we have predefined lifecycle events? Things start to get too much coupled and difficult to keep certain scalability. It is probably due to my naive knowledge on internationalization.

Really, I completely understand if it is not suitable.

Thank you so much.

adrai commented 4 years ago

Good morning...

Let's try to clarify the confusion... Let's have an example: You have an app built in several parts, to simplify let's say each part is a page. So you will organize your namespaces like this:

first-page.json // content for first page
second-page.json // content for second page
common.json // common content for i.e. save, cancel button, footer, etc...

In the app code, now you will configure i18next to load the namespace 'common' already while initializing i18next (because this content is used everywhere) Then somewhere where "rendering" the first-page you need to instrument the code to tell i18next, for this page I want the keys of namespace 'first-page', so behind the scene call hasLoadedNamespace and if not loadNamespaces and wait for rendering until the namespace is loaded. The same needs to be done for the second-page. To not be forced to prefix the keys in the code always with the appropriate namespace name i.e. t('first-page:hello') you could call getFixedT like here in react-i18next, this way you just need t('hello')

With such a setup, as soon as you would "navigate or render" the first-page or second-page, the appropriate namespace would be loaded.

I hope this clarifies a bit the lazy-loading topic.

If possible, I would keep the discussion here... this way we can all "work" on it in an asynchronous way ;-) If there is still a need to have a hangout session, we can organize it, but this Saturday is probably not the best one.

Is this ok for you?

PS: in this next-i18next example you'll see a similar namespace setup: the namespaces: https://github.com/isaachinman/next-i18next/tree/master/examples/simple/public/static/locales/en the app code: https://github.com/isaachinman/next-i18next/tree/master/examples/simple/pages

daniloster commented 4 years ago

Hey @adrai thanks for taking the time to explain it. I understand it.

There is a very important conflicting between svelte-i18n and i18next.

namespace

svelte-i18n uses namespace to not repeat itself to resolve a key, whereas i18next uses namespace to scope a different set of dictionary. Both have its own and correct meaning independently.

Resolution

The svelte-i18n will accept namespace prefixed (but optional) with i18nextNamespace:app.container.PageOne.

Where

i18nextNamespace is the namespace defined by i18next
: is the separator defined by i18next (the library will auto-detect which symbol to split)
The suffix part is the namespace for key to not repeat itself

I will push as soon as I get the tests passing.

I will take a bit more to write proper documentation.

Once more, thank you for the patient and help along this integration.

daniloster commented 4 years ago

Updated the PR with the changes. https://github.com/daniloster/svelte-i18n/pull/11

adrai commented 4 years ago

Thank you for the clarification about the namespace conflict... Little advice: try to set the i18next option "debug" to true, to check if there are "unwanted" warnings... It may be you call hasLoadedNamespace function before i18next is finished to initialize. Btw: Did you already try to use also the https://github.com/i18next/i18next-browser-languageDetector ? I assume this could still be a problem, because you pick the state.locale from i18n.options.language => i18n.language has always the detected language => https://www.i18next.com/overview/api#language (btw. there's also i18n.languages)

daniloster commented 4 years ago

Pushed a few updates replacing the commits. Also, left some comments to highlight some points.

I will try to use the debug.

Regarding the language, you believe I should try one and fallback to the other e.g. const locale = [].concat(i18n.options.language || i18n.options.languages).shift(). Is it what you meant? Or the opposite? const locale = [].concat(i18n.options.languages || i18n.options.language).shift().

Note: Integrating the i18next, this library does not need to know the others only the primary.

i18next provide so many options that is difficult know what are the priorities (at least for me): language, fallbackLng, languages. Sames for namespaces. It would be nice to have less options and maybe plural e.g. only languages where the order defines the primary (which should also be default/fallback language). Does it make sense?

daniloster commented 4 years ago

I get it now about the i18n.options.language and the i18n.language. Thanks for the heads up.

daniloster commented 4 years ago

I have pushed the changes without console warnings. Happy in a certain way and a bit sad in the other hand.

Honestly, I feel as it became something overcomplicated (integration). Integration seems really brittle and couple. Architecturally, i18next should (and I believe it can) have better API, besides reactive event hooks.

I really appreciate all the time invested help with this. Thank you again.

adrai commented 4 years ago

Nice, I just tried also with the language detector... seems to work. Good job!

i18next should (and I believe it can) have better API, besides reactive event hooks.

Regarding the API, feel free to suggest any optimization/simplification while keeping the existing functionality... i18next is always happy to evolve in a better way ;-)

daniloster commented 4 years ago

Nice, I just tried also with the language detector... seems to work. Good job!

i18next should (and I believe it can) have better API, besides reactive event hooks.

Regarding the API, feel free to suggest any optimization/simplification while keeping the existing functionality... i18next is always happy to evolve in a better way ;-)

Thank you. You helped a lot. 🎉

For API, my concern is that would be good to have a major release planned simplifying it.

I totally understand the purpose of not adding a breaking changes when evolving a library/system. However, still believe that a breaking changes should be considered to make things simpler and consistent.

All our communication and integration motivated me to start a new project as PoC. I really, really, appreciate the time you invested in it.

Thank you again.

adrai commented 4 years ago

You’re welcome.

For API, my concern is that would be good to have a major release planned simplifying it.

Curious to see your suggestions 😉

👍

daniloster commented 4 years ago

I will put something together. I have merged the PR, but, not release yet. I want to write documentation before releasing.

daniloster commented 4 years ago

Closing as integration is working. 🎉

daniloster / svelte-i18n

Open discussion about i18next #10