Missed case for customLoad #78

Closed
jcampanello opened this Issue Feb 12, 2013 · 6 comments

2 participants

@jcampanello

When sync._fetch is called from inside init (be it at initialization time or when you do i18n.setLng) the global options object (o) is passed to sync._fetch.

But when loadNamespaces is called (line 638), it builds it's own local opts object, pulling in attributes from the global options (o). In this case, customLoad is not pulled into this local opts so _fetch will use the regular load mechanism.

Also, i've noticed that customLoad is only used for single lang/namespace fetches (as opposed to setting dynamicLoad to true). In this later case, customLoad will not be used.

I'm burning right now with a project, when i'm finished if you want i can make a pull request for the changes (if it's easier for you).

Using 1.6.0

Thanks!

@jamuhl
i18next member

Would be happy to get a pull request for this. No need to hurry as next release is not yet planned - but there will be one for sure ;)

@jcampanello

Another related issue seems to be that neither loadNamespaces nor addResourceBundle update the o.ns.namespaces list (append new namespaces).

My scenario is as follows:

1) at startup time, i have many modules (files) loading and i want each file to dynamically add it's namespace to i18n in the default language (en) using addResourceBundle. The goal is that just including a file in the html will add functionality to the app (no central place to configure, etc).

2) at a later point, the application sets the user's desired language, triggering the load of the new language texts for each registered module (each module file brings with it all the messages, that's why i have a customLoad).

When 2 happens, only the default namespace is loaded for the new language. I think this is because neither loadNamespaces nor addResourceBundle adds the namespace to the options.ns.namespaces array.

Should the add to options.ns.namespaces array be done manually or is it reasonable that these methods do it automatically?

@jamuhl
i18next member

Just missed this. Yes it should be added if not already in the list of namespaces. Got to add that for next release.

@jamuhl
i18next member

this last commit 360c49c should close this.

please check it out - reopen the issue if i missed something.

@jamuhl jamuhl closed this Apr 24, 2013
@jcampanello

Jan,

first: really sorry for not taking care of this as i promised! My expectations of some available time to devote to this issue went down the drain... Sorry again!

second: i (visually) reviewed the diff for the issue commit and compared to the comments i've placed in my working copy and the changes seem to be at exactly the same places, so i think your commit should solve the issue.

Best regards,

Jose Luis

@jamuhl
i18next member

thanks for cross-checking.

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