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

init deletes previously loaded resources on completion #181

Closed
lennym opened this issue Apr 2, 2015 · 11 comments
Closed

init deletes previously loaded resources on completion #181

lennym opened this issue Apr 2, 2015 · 11 comments

Comments

@lennym
Copy link
Contributor

lennym commented Apr 2, 2015

I've been working on refactoring an app to split it into a number of modules, and trying to get the i18n resources for each of those modules contained within them. This has meant that I have been using i18n.addResouceBundle to load the module's resources into their own namespace.

However, since i18n.init is async and destroys any previously loaded resources in the resStore as part of its callback then these modules' loaded resources are lost once init completes.

The usage information in the README and code examples don't make any reference to this fact so I had to spend quite some time debugging to find this.

I'd suggest merging the store at https://github.com/i18next/i18next-node/blob/master/lib/dep/i18next.js#L968 instead of overwriting it completely so that resources which have been added during initialisation are not deleted.

Alternatively, simply have init load its resources synchronously (or at least have the option to do so)

@lennym
Copy link
Contributor Author

lennym commented Apr 2, 2015

The naming conventions are slightly misleading as well - in particular filesync since it does not use sync fs methods.

@lennym
Copy link
Contributor Author

lennym commented Apr 2, 2015

This is all compounded by the fact that there's no external way of checking if initialisation has completed, or waiting until it has completed so my modules have know way of determining if it is safe to add resources.

@jamuhl
Copy link
Member

jamuhl commented Apr 2, 2015

naming of filesync comes from syncing state from filesystem to inmemory (or for other backends from eg. mongodb to inmem). but agree might be a little missleading.

init function has a optional callback which is called when loading is done.

writing a own implementation for a synchronous file'sync' is rather easy to do.

but agree, we should change that for the node.js variant of i18next so that resources are not deleted.

@lennym
Copy link
Contributor Author

lennym commented Apr 2, 2015

Yes, I've ended up taking the "build my own backend" approach to this basically taking fileync and swapping to *Sync methods (watch https://github.com/lennym/i18next.fssync - the code is done but I forgot to push before I left the office earlier).

All I really need is for modules to be able to safely call addResourceBundle safely.

@lennym
Copy link
Contributor Author

lennym commented Apr 7, 2015

I've been thinking more on this topic, the issue at hand, and best solutions to resolve.

The Problem

In short - i18next as it currently stands is not suitable for use inside node modules. The nature of the code base as a singleton with a single config and resource store prevents a module from being able to reliably create its own i18n scope. If I call i18n.init in my module with its settings and resource files then this will be destroyed if any other module also calls i18n.init on the same instance of i18next since configuration and resources can only exist in a singe place.

This is a hangover I suspect of the fact that i18next started life as a browser script, and so there was no notion of modules, and there would only ever be a single scope - a singleton was fine, and namespaces could satisfy all of the use cases.

My workaround for this is to a) add a backend to synchronously load default namespaces, and b) have my modules subsequently add their own namespaces to the global scope. This is undesirable for a number of reasons:

  • modules have to have knowledge of the environment in which they exist - I have to assume that init has been called somewhere up the stack.
  • I still need to make sure that the modules add their own resources after init has been called since even a synchronous init will still delete them otherwise
  • I lose the auto-traversal of langauges and namespaces that init gives me - I have to manually add each namespace and langauge with i18n.addResourceBundle.

Proposed Solution

Export a constructor/factory which can create isolated instances of resource stores and associated configurations. Then a module can create its own scope and resources with its own config, which cannot be influenced or affected by any other external sources.

The issue with this is that it effectively requires a total re-write.

I will continue to think on this, and see if any more ideas come to me as I get to know the internals of the project better.

@jamuhl
Copy link
Member

jamuhl commented Apr 8, 2015

you're correct with the assumption that i18next started as a browser project. In our project we never run in bigger issues as we just loaded all namespaces and languages (rather limited) on serverside.

for one bigger project with 'dynamic' modules we overloaded the way i18next-node resolves the resource path, so loading a prefixed namespace would land inside a module to pick up the files for that module.

the proposed solution is something on the roadmap for a version 2.0.0, which is in my head for a long time (basically keeping most of the existing API, having instances, have it more modularized so people could choose if they need eg. plural support, the shims,...). But this would need a bigger time investment that right now i can not commit to.

@lennym
Copy link
Contributor Author

lennym commented Apr 8, 2015

Do you have an idea of what you might imagine the API looking like for the modular stuff in 2.0? I spent a few hours looking at it yesterday, and I think if I could find a spare few days or a weekend then I might be able to have a good go at putting something together.

I'm trying to convince my employers that since we're making heavy use of this package that it's worth letting me spend some proper time working on it.

@jamuhl
Copy link
Member

jamuhl commented Apr 8, 2015

not yet thought to much about that.

but one thing i would do for sure is moving out the jquery stuff (it's time to let it go). all the jquery stuff could be in a extra file depending on the i18next-client. in same step the built in ajax stuff could be simplified and reduced in size - i guess.

same for the shims i added eg. for ie8 support - they could be provided in an extra file.

pluralrules might be loaded separate (but actually they are not that big anymore...so we might provide them without extra configuration/loading).

must be a cool place you're working at!

@lennym
Copy link
Contributor Author

lennym commented Apr 8, 2015

No worries - I've had a crack this morning at implementing something based on the existing unit tests at https://github.com/lennym/i18next/blob/spike/constructor/src/i18next.js

It's not going too badly...

@jamuhl
Copy link
Member

jamuhl commented Nov 5, 2015

might be interested in https://github.com/jamuhl/i18next-playground

upcoming v2 of i18next -> will support instances and solve a few other issues ;)

@jamuhl
Copy link
Member

jamuhl commented Nov 29, 2015

will be in v2 http://i18next.github.io/i18next.com/

@jamuhl jamuhl closed this as completed Nov 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants